Poetry error message (#852)

* testing the usecase

* error message thrown for the command not found

* semicolon was missing

* unwanted changes reverted and also added command output for the error

* review comments addressed

* moved the cmdOutput to runRemoteCommand function for all the commands

* refactored code and moved outer catch block to inner catch block

* try catch block added for per lifecycleHook

* await promise changes

* test case fixed

* unwanted code removed

* file name changed

* Fixes

---------

Co-authored-by: Christof Marti <[email protected]>

Author: prathameshzarkar9

src/spec-common/injectHeadless.ts

@@ -487,38 +487,48 @@ async function runLifecycleCommand({ lifecycleHook }: ResolverParameters, contai
 			},
 			onDidChangeDimensions: lifecycleHook.output.onDidChangeDimensions,
 		}, LogLevel.Info);
-		try {
-			const remoteCwd = containerProperties.remoteWorkspaceFolder || containerProperties.homeFolder;
-			async function runSingleCommand(postCommand: string | string[], name?: string) {
-				const progressDetail = typeof postCommand === 'string' ? postCommand : postCommand.join(' ');
-				infoOutput.event({
-					type: 'progress',
-					name: progressName,
-					status: 'running',
-					stepDetail: progressDetail
-				});
-
-				// If we have a command name then the command is running in parallel and 
-				// we need to hold output until the command is done so that the output
-				// doesn't get interleaved with the output of other commands.
-				const printMode = name ? 'off' : 'continuous';
-				const env = { ...(await remoteEnv), ...(await secrets) };
+		const remoteCwd = containerProperties.remoteWorkspaceFolder || containerProperties.homeFolder;
+		async function runSingleCommand(postCommand: string | string[], name?: string) {
+			const progressDetails = typeof postCommand === 'string' ? postCommand : postCommand.join(' ');
+			infoOutput.event({
+				type: 'progress',
+				name: progressName,
+				status: 'running',
+				stepDetail: progressDetails
+			});
+			// If we have a command name then the command is running in parallel and 
+			// we need to hold output until the command is done so that the output
+			// doesn't get interleaved with the output of other commands.
+			const printMode = name ? 'off' : 'continuous';
+			const env = { ...(await remoteEnv), ...(await secrets) };
+			try {
 				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`);
+					infoOutput.raw(`\x1b[1mRunning ${name} of ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`);
+				}
+			} catch (err) {
+				if (printMode === 'off' && err?.cmdOutput) {
+					infoOutput.raw(`\r\n\x1b[1m${err.cmdOutput}\x1b[0m\r\n\r\n`);
+				}
+				if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2.
+					infoOutput.raw(`\r\n\x1b[1m${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} interrupted.\x1b[0m\r\n\r\n`);
+				} else {
+					if (err?.code) {
+						infoOutput.write(toErrorText(`${name ? `${name} of ${lifecycleHookName}` : lifecycleHookName} from ${userCommandOrigin} 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.event({
-					type: 'progress',
-					name: progressName,
-					status: 'succeeded',
-				});
 			}
+		}
 
-			infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`);
+		infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`);
 
+		try {
 			let commands;
 			if (typeof userCommand === 'string' || Array.isArray(userCommand)) {
 				commands = [runSingleCommand(userCommand)];
@@ -528,24 +538,24 @@ async function runLifecycleCommand({ lifecycleHook }: ResolverParameters, contai
 					return runSingleCommand(command, name);
 				});
 			}
-			await Promise.all(commands);
+
+			const results = await Promise.allSettled(commands); // Wait for all commands to finish (successfully or not) before continuing.
+			const rejection = results.find(p => p.status === 'rejected');
+			if (rejection) {
+				throw (rejection as PromiseRejectedResult).reason;
+			}
+			infoOutput.event({
+				type: 'progress',
+				name: progressName,
+				status: 'succeeded',
+			});
 		} catch (err) {
 			infoOutput.event({
 				type: 'progress',
 				name: progressName,
 				status: 'failed',
 			});
-			if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2.
-				infoOutput.raw(`\r\n\x1b[1m${lifecycleHookName} interrupted.\x1b[0m\r\n\r\n`);
-			} else {
-				if (err?.code) {
-					infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`));
-				}
-				throw new ContainerError({
-					description: `The ${lifecycleHookName} in the ${userCommandOrigin} failed.`,
-					originalError: err,
-				});
-			}
+			throw err;
 		}
 	}
 }

src/test/cli.up.test.ts

@@ -25,6 +25,7 @@ describe('Dev Containers CLI', function () {
 	});
 
 	describe('Command up', () => {
+
 		it('should execute successfully with valid config', async () => {
 			const res = await shellExec(`${cli} up --workspace-folder ${__dirname}/configs/image --include-configuration --include-merged-configuration`);
 			const response = JSON.parse(res.stdout);

src/test/configs/poetry-example/.devcontainer.json

@@ -0,0 +1,18 @@
+// Example devcontainer.json configuration, 
+// wired into the vscode launch task (.vscode/launch.json)
+{
+	"image": "mcr.microsoft.com/devcontainers/base:latest",
+	"features": {
+		"ghcr.io/devcontainers/features/python:1": {
+			"version": "latest"
+		}
+	},
+	"postStartCommand": {
+		"poetry setup": [
+			"/bin/bash",
+			"-i",
+			"-c",
+			"python3 -m venv $HOME/.local && source $HOME/.local/bin/activate && poetry install"
+		]
+	}
+}

src/test/container-features/lifecycleHooks.test.ts

@@ -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.
@@ -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
@@ -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 \?\?\?/);