Skip to content

issue missing root cause of error #845 solution #933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gauravsaini04 wants to merge 2 commits into from
Closed

issue missing root cause of error #845 solution #933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77fca8d
issue missing root cause of error #845 solution
gauravsaini04 Nov 18, 2024
e040bec
changes necessary for check to pass
gauravsaini04 Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions src/spec-common/injectHeadless.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,18 +503,26 @@ async function runLifecycleCommand({ lifecycleHook }: ResolverParameters, contai
// doesn't get interleaved with the output of other commands.
const printMode = name ? 'off' : 'continuous';
const env = { ...(await remoteEnv), ...(await secrets) };
const { cmdOutput } = await runRemoteCommand({ ...lifecycleHook, output: infoOutput }, containerProperties, typeof postCommand === 'string' ? ['/bin/sh', '-c', postCommand] : postCommand, remoteCwd, { remoteEnv: env, pty: true, print: printMode });

// 'name' is set when parallel execution syntax is used.
if (name) {
infoOutput.raw(`\x1b[1mRunning ${name} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`);
}
try {
const { cmdOutput } = await runRemoteCommand({ ...lifecycleHook, output: infoOutput }, containerProperties, typeof postCommand === 'string' ? ['/bin/sh', '-c', postCommand] : postCommand, remoteCwd, { remoteEnv: env, pty: true, print: printMode });

infoOutput.event({
type: 'progress',
name: progressName,
status: 'succeeded',
});
// 'name' is set when parallel execution syntax is used.
if (name) {
infoOutput.raw(`\x1b[1mRunning ${name} of ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`);
}
} catch (err) {
Copy link
Contributor Author

@gauravsaini04 gauravsaini04 Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so basically what i understood is that you'd like me to push this code of catching the error inside runSingleCommand method to be after https://github.com/devcontainers/cli/blob/main/src/spec-common/injectHeadless.ts#L531 i.e. await Promise.all(commands), in the catch block that's written after it
( in your comment here )

Copy link
Contributor

@chrmarti chrmarti Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are very close in #852. I suggest we continue there since that has all the history of the discussion. I have pushed the suggested fixes there, please take a look and leave a review. Thanks!

Copy link
Contributor Author

@gauravsaini04 gauravsaini04 Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

closing this as #852 was already merged.

if (printMode === 'off' && err?.cmdOutput) {
infoOutput.raw(`\r\n\x1b[1m${err.cmdOutput}\x1b[0m\r\n\r\n`);
if (err?.code) {
infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`));
}
throw new ContainerError({
description: `${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} failed.`,
originalError: err
});
}
}
}

infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`);
Expand Down
14 changes: 14 additions & 0 deletions src/test/configs/poetry-example/.devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Example devcontainer.json configuration,
// wired into the vscode launch task (.vscode/launch.json)
{
"image": "mcr.microsoft.com/devcontainers/base:ubuntu",
"features": {
"ghcr.io/devcontainers/features/python:1": {
"version": "latest",
"installTools": "false"
}
},
"postStartCommand": {
"poetry setup": ["/bin/bash", "-i", "-c", "python3 -m venv $HOME/.local && source $HOME/.local/bin/activate && poetry install"]
}
}
12 changes: 6 additions & 6 deletions src/test/container-features/lifecycleHooks.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,10 +406,10 @@ describe('Feature lifecycle hooks', function () {
assert.match(containerUpStandardError, /Running the postAttachCommand from devcontainer.json/);

assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_1.testMarker/);
assert.match(containerUpStandardError, /Running parallel1 from devcontainer.json.../);
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from devcontainer.json.../);

assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_2.testMarker/);
assert.match(containerUpStandardError, /Running parallel2 from devcontainer.json.../);
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from devcontainer.json.../);

// Since lifecycle scripts are executed relative to the workspace folder,
// to run a script bundled with the Feature, the Feature author needs to copy that script to a persistent directory.
Expand All @@ -429,10 +429,10 @@ describe('Feature lifecycle hooks', function () {
assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/rabbit'/);

assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_1.testMarker/);
assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/rabbit'/);
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from Feature '\.\/rabbit'/);

assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_2.testMarker/);
assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/rabbit'/);
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from Feature '\.\/rabbit'/);


// -- 'Otter' Feature
Expand All @@ -449,10 +449,10 @@ describe('Feature lifecycle hooks', function () {
assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/otter'/);

assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_1.testMarker/);
assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/otter'/);
assert.match(containerUpStandardError, /Running parallel1 of postCreateCommand from Feature '\.\/otter'/);

assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_2.testMarker/);
assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/otter'/);
assert.match(containerUpStandardError, /Running parallel2 of postCreateCommand from Feature '\.\/otter'/);

// -- Assert that at no point did logging the lifecycle hook fail.
assert.notMatch(containerUpStandardError, /Running the (.*) from \?\?\?/);
Expand Down