diff --git a/ext/vscode/CHANGELOG.md b/ext/vscode/CHANGELOG.md index 8b46381f634..a4a366e6e40 100644 --- a/ext/vscode/CHANGELOG.md +++ b/ext/vscode/CHANGELOG.md @@ -8,7 +8,7 @@ ### Bugs Fixed -### Other Changes +- [[#9136]](https://github.com/Azure/azure-dev/pull/9136) Fixed the workspace resource view surfacing an opaque "Process exited with code 1" error (with a Report Issue prompt) when `azd show` fails for expected reasons such as not being logged in or having no environment. The underlying `azd` error is now included for diagnosis and these background failures no longer prompt users to file a bug. ## 0.10.0 (2025-09-22) diff --git a/ext/vscode/src/test/suite/unit/azureDevCliApplication.test.ts b/ext/vscode/src/test/suite/unit/azureDevCliApplication.test.ts new file mode 100644 index 00000000000..f0e63b88cc0 --- /dev/null +++ b/ext/vscode/src/test/suite/unit/azureDevCliApplication.test.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { WorkspaceResource } from '@microsoft/vscode-azureresources-api'; +import { IActionContext } from '@microsoft/vscode-azext-utils'; +import { expect } from 'chai'; +import { AzureDevCliApplication } from '../../../views/workspace/AzureDevCliApplication'; +import { AzDevShowResults, AzureDevShowProvider } from '../../../services/AzureDevShowProvider'; +import { AzureDevEnvListProvider } from '../../../services/AzureDevEnvListProvider'; +import { AzureDevEnvValuesProvider } from '../../../services/AzureDevEnvValuesProvider'; + +suite('AzureDevCliApplication Tests', () => { + // Captures the errorHandling context that AzureDevCliApplication passes into getShowResults so the + // test can assert the passive tree-resolution path opts out of the modal "Report Issue" dialog. + function buildApplication(showProvider: AzureDevShowProvider): AzureDevCliApplication { + const resource = { id: '/test/azure.yaml', name: 'test-app' } as unknown as WorkspaceResource; + const envListProvider = {} as unknown as AzureDevEnvListProvider; + const envValuesProvider = {} as unknown as AzureDevEnvValuesProvider; + + return new AzureDevCliApplication( + resource, + () => { /* no-op refresh */ }, + showProvider, + envListProvider, + envValuesProvider, + new Set(), + () => { /* no-op toggle */ }, + /* includeEnvironments */ false + ); + } + + test('Suppresses the error dialog when show fails during passive tree resolution', async () => { + let capturedSuppressDisplay: boolean | undefined; + + const failingShowProvider: AzureDevShowProvider = { + getShowResults: (context: IActionContext): Promise => { + // Record the flag at call time - AzureDevCliApplication must set it before invoking show. + capturedSuppressDisplay = context.errorHandling.suppressDisplay; + return Promise.reject(new Error('Process exited with code 1')); + }, + }; + + const application = buildApplication(failingShowProvider); + + // Resolving children triggers the show call. The failure is swallowed (suppressDisplay + no + // rethrow), so this must not throw and must not surface a modal dialog. + const children = await application.getChildren(); + + expect(capturedSuppressDisplay, 'getShowResults must be invoked with suppressDisplay enabled').to.equal(true); + // Even though show failed, the Services node is still returned (with no services). + expect(children).to.have.lengthOf(1); + }); +}); diff --git a/ext/vscode/src/test/suite/unit/execAsync.test.ts b/ext/vscode/src/test/suite/unit/execAsync.test.ts new file mode 100644 index 00000000000..dc837f50109 --- /dev/null +++ b/ext/vscode/src/test/suite/unit/execAsync.test.ts @@ -0,0 +1,119 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { ChildProcessError, StreamSpawnOptions } from '@microsoft/vscode-processutils'; +import { expect } from 'chai'; +import { execAsync, SpawnStreamAsync } from '../../../utils/execAsync'; + +// Builds a fake spawnStreamAsync that emits the given stdout/stderr into the accumulator pipes that +// execAsync wires up, then either resolves or rejects to simulate the process exit. The pipes must be +// ended so that AccumulatorStream.getString() (which awaits the stream 'close' event) resolves - this +// mirrors how a real child process closes its stdio streams on exit. +function fakeSpawn(stdout: string, stderr: string, rejectWith?: Error): SpawnStreamAsync { + return (_command: string, _args, options?: StreamSpawnOptions) => { + if (stdout) { + options?.stdOutPipe?.write(Buffer.from(stdout)); + } + options?.stdOutPipe?.end(); + + if (stderr) { + options?.stdErrPipe?.write(Buffer.from(stderr)); + } + options?.stdErrPipe?.end(); + + return rejectWith ? Promise.reject(rejectWith) : Promise.resolve(); + }; +} + +// Simulates a rejection that happens *before* a child process is spawned (e.g. an already-cancelled +// token or a Windows executable-resolution failure). Crucially, the pipes are never ended, so any code +// that awaits stderrFinal.getString() would hang forever. +function fakePreSpawnFailure(rejectWith: Error): SpawnStreamAsync { + return (_command: string, _args, _options?: StreamSpawnOptions) => { + return Promise.reject(rejectWith); + }; +} + +// Reproduces the real ordering behind this bug: spawnStreamAsync rejects on the process 'exit' event +// *before* the stderr stream has closed. The rejection is returned immediately while the stderr +// write/end is deferred to a later tick, so this only passes if execAsync waits for stderr to close +// after catching the rejection (rather than reading it too early and missing the message). +function fakeSpawnStderrAfterRejection(stderr: string, rejectWith: Error): SpawnStreamAsync { + return (_command: string, _args, options?: StreamSpawnOptions) => { + options?.stdOutPipe?.end(); + + setTimeout(() => { + if (stderr) { + options?.stdErrPipe?.write(Buffer.from(stderr)); + } + options?.stdErrPipe?.end(); + }, 10); + + return Promise.reject(rejectWith); + }; +} + +suite('execAsync Tests', () => { + test('Returns stdout and stderr on success', async () => { + const { stdout, stderr } = await execAsync('cmd', [], undefined, fakeSpawn('out', 'err')); + + expect(stdout).to.equal('out'); + expect(stderr).to.equal('err'); + }); + + test('Includes process stderr in the error message on non-zero exit', async () => { + // stderr closes *after* the spawn promise rejects - the real ordering - so this proves execAsync + // waits for the stderr pipe to close before appending it, instead of reading it too early. + const spawn = fakeSpawnStderrAfterRejection( + 'ERROR: parsing project file: File is empty.', + new ChildProcessError('Process exited with code 1', 1, null) + ); + + try { + await execAsync('cmd', [], undefined, spawn); + expect.fail('Should have thrown an error'); + } catch (error) { + expect(error).to.be.instanceOf(Error); + // Without this behavior, the message would only be the generic "Process exited with code 1", + // discarding the real reason emitted by the child process on stderr. + expect((error as Error).message, 'Error should retain the exit-code context').to.include( + 'Process exited with code 1' + ); + expect((error as Error).message, 'Error should surface the underlying stderr').to.include( + 'parsing project file: File is empty.' + ); + } + }); + + test('Rethrows the original error unchanged when stderr is empty', async () => { + const originalError = new ChildProcessError('Process exited with code 1', 1, null); + const spawn = fakeSpawn('', '', originalError); + + try { + await execAsync('cmd', [], undefined, spawn); + expect.fail('Should have thrown an error'); + } catch (error) { + expect(error).to.equal(originalError); + expect((error as Error).message).to.equal('Process exited with code 1'); + } + }); + + test('Rethrows pre-spawn failures immediately without waiting on stderr', async () => { + const originalError = new Error('Operation cancelled'); + const spawn = fakePreSpawnFailure(originalError); + + // If execAsync awaited stderr for non-ChildProcessError rejections, this would hang because the + // fake never ends the stderr pipe. A short timeout guards against a regression that reintroduces + // the hang. + const timeout = new Promise((_resolve, reject) => + setTimeout(() => reject(new Error('execAsync hung waiting on an un-ended stderr pipe')), 1000) + ); + + try { + await Promise.race([execAsync('cmd', [], undefined, spawn), timeout]); + expect.fail('Should have thrown an error'); + } catch (error) { + expect(error, 'Pre-spawn failure should be rethrown unchanged').to.equal(originalError); + } + }); +}); diff --git a/ext/vscode/src/utils/execAsync.ts b/ext/vscode/src/utils/execAsync.ts index 7552a841d20..2198dfc6eef 100644 --- a/ext/vscode/src/utils/execAsync.ts +++ b/ext/vscode/src/utils/execAsync.ts @@ -3,9 +3,12 @@ // This file was lifted and adapted from https://github.com/microsoft/vscode-containers/blob/6a8643df8d42033fb776170dfd0ffe92f316f5b5/src/utils/execAsync.ts -import { AccumulatorStream, CommandLineArgs, NoShell, spawnStreamAsync, StreamSpawnOptions } from '@microsoft/vscode-processutils'; +import { AccumulatorStream, CommandLineArgs, isChildProcessError, NoShell, spawnStreamAsync, StreamSpawnOptions } from '@microsoft/vscode-processutils'; -export async function execAsync(command: string, args: CommandLineArgs, options?: Partial): Promise<{ stdout: string, stderr: string }> { +// Alias for the spawn implementation so it can be overridden in tests without spawning a real process. +export type SpawnStreamAsync = typeof spawnStreamAsync; + +export async function execAsync(command: string, args: CommandLineArgs, options?: Partial, spawnStreamAsyncFunction: SpawnStreamAsync = spawnStreamAsync): Promise<{ stdout: string, stderr: string }> { const stdoutFinal = new AccumulatorStream(); const stderrFinal = new AccumulatorStream(); @@ -16,7 +19,28 @@ export async function execAsync(command: string, args: CommandLineArgs, options? stdErrPipe: stderrFinal, }; - await spawnStreamAsync(command, args, spawnOptions); + try { + await spawnStreamAsyncFunction(command, args, spawnOptions); + } catch (error) { + // Only a ChildProcessError means the process actually started and exited non-zero, so its + // stderr pipe was ended and can be read. spawnStreamAsync can also reject *before* spawning + // (e.g. an already-cancelled token or a Windows executable-resolution failure); in that case + // stdErrPipe is never ended and getString() would hang forever. Rethrow those immediately. + if (!isChildProcessError(error)) { + throw error; + } + + // On a non-zero exit, spawnStreamAsync rejects with a generic message (e.g. "Process exited + // with code 1") from within a ChildProcess handler, before the caller ever reads stderr. Append + // the process's stderr to the error so callers can classify and report the real failure instead + // of an opaque exit code. + const stderr = (await stderrFinal.getString()).trim(); + if (stderr && error instanceof Error) { + error.message = `${error.message}\n${stderr}`.trim(); + } + + throw error; + } return { stdout: await stdoutFinal.getString(), diff --git a/ext/vscode/src/views/workspace/AzureDevCliApplication.ts b/ext/vscode/src/views/workspace/AzureDevCliApplication.ts index 98ab92e2fd9..c15a7202046 100644 --- a/ext/vscode/src/views/workspace/AzureDevCliApplication.ts +++ b/ext/vscode/src/views/workspace/AzureDevCliApplication.ts @@ -70,6 +70,12 @@ export class AzureDevCliApplication implements AzureDevCliModel { return callWithTelemetryAndErrorHandling( TelemetryId.WorkspaceViewApplicationResolve, async actionContext => { + // This runs automatically whenever the workspace tree resolves. A non-zero `azd show` + // exit here is almost always an expected/environmental state (not logged in, no + // environment yet, invalid azure.yaml) rather than an extension bug, so don't surface a + // modal error with a "Report Issue" button. The failure is still captured in telemetry, + // and azure.yaml problems are reported independently via the Problems panel. + actionContext.errorHandling.suppressDisplay = true; return await this.showProvider.getShowResults(actionContext, this.context.configurationFile); } ) as Promise;