fix(ci): resolve failures from telemetry regression, unhandled rejections, and flaky tests#8680
fix(ci): resolve failures from telemetry regression, unhandled rejections, and flaky tests#8680
Conversation
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
…Formation LSP client
The vscode-languageclient LanguageClient creates unhandled promise
rejections internally when the LSP server process fails to start
during CI. These rejections (connection disposed, client not running)
occur inside the library's handleConnectionClosed/doInitialize and
cannot be caught from our code.
Skip the LSP client entirely during unit tests since no tests depend
on it. Reverts the process.on('unhandledRejection') approach which
was too broad and wouldn't suppress VS Code's own rejection tracking.
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
… timeout tests - SharedCredentialsProvider: check no *new* messages instead of absolute count (previous test's messages leak via getTestWindow) - ToolkitLogger: retry reading log file content instead of single read (Windows file I/O flush race condition) - editorContext/recommendationHandler: bump timeout to 60s for slow macOS insiders CI environment
|
… var - CodeLens: log caught errors at debug level instead of silent swallow - SageMaker server: process.exit(1) instead of return when env var missing — server can't function without it, shouldn't linger
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
The SageMaker detached server runs inside the VS Code extension host process. process.exit(1) terminates the entire test runner. Use console.error + return instead.
Problem
Multiple CI checks failing across macOS, Windows, and Linux (CodeBuild). All 5849+ unit tests pass — failures are from test assertion regressions, unhandled promise rejections detected by the
run_and_reportscript, and pre-existing flaky tests.Root Causes & Fixes
1. Lambda URI Handler telemetry — 3 test failures (all platforms)
Introduced by: #8598 (
fix(lambda): add confirmation prompt before initiating console login)What broke: #8598 changed
handleLambdaUriErrorfrom throwing a cancelledToolkitErrorto callingtelemetry.record()+ return. This broke telemetry becauserun()overwrites the span result withSucceededwhen the function completes normally — therecord()call gets clobbered.Fix: Revert the cancellation path to throw
ToolkitError.chain(e, 'User cancelled operation', { cancelled: true }). This letsrun()correctly recordCancelledas the result. Tests updated to assert the throw instead of checking telemetry directly.Files:
uriHandlers.ts(functional),uriHandlers.test.ts(test)2. AppBuilder Walkthrough test — 1 test failure (macOS minimum)
Introduced by: Interaction between the walkthrough test (#8236) and background security scan errors from
startSecurityScan.ts. The test'sonDidShowMessagehandler asserted on every message, but an unrelated security scan error message fired during the test.Fix: Filter the
onDidShowMessagehandler to only assert on the expected overwrite prompt message, ignoring unrelated ones. Added explicit assertion that the prompt was shown.Files:
walkthrough.test.ts(test only)3. Unhandled promise rejections — Linux CodeBuild failures (all 3 variants)
Introduced by:
throwwhen env var missing — feat(sagemaker): Merge SageMaker SSH Kiro integration #8589 (feat(sagemaker): Merge SageMaker SSH Kiro integration)StackActionCodeLensProvidercallingsendRequeston stopped client — feat(cloudformation): Merge CloudFormation LSP integration with toolkit updates #8275 (feat(cloudformation): Merge CloudFormation LSP integration)deactivate()client.stop()rejecting — same PR feat(cloudformation): Merge CloudFormation LSP integration with toolkit updates #8275What broke: The
run_and_reportscript in CodeBuild fails the build if it detects "rejected promise not handled" in stdout. All 5849 tests pass, but these 5 unhandled rejections triggered the check.Fix:
console.error+returninstead ofthrowfor missing env var — avoids unhandled rejection while keeping extension host aliveisRunning()guard + try/catch beforesendRequest, errors logged at debug level.catch()onclient.stop()Files:
server.ts,stackActionCodeLensProvider.ts,extension.ts(all functional — adds resilience)4. CloudFormation LSP client lifecycle rejections — Linux CodeBuild minimum
Introduced by: #8275 (
feat(cloudformation): Merge CloudFormation LSP integration)What broke: The
vscode-languageclientLanguageClient creates unhandled promise rejections internally when the LSP server process fails to start during CI. These rejections (connection got disposed,Client is not running) happen inside the library'shandleConnectionClosed/doInitializeand cannot be caught from our code.Fix: Skip the CloudFormation LSP client activation when
AWS_TOOLKIT_AUTOMATION === 'unit'. No unit tests depend on the LSP client — the CloudFormation unit tests cover template parsing, not the language server. E2E tests are unaffected (they use a different automation value).Files:
extension.ts(functional — conditional activation)5. Pre-existing flaky test fixes
These are not caused by any recent PR but fail intermittently across CI:
SharedCredentialsProvider (
handleInvalidConsoleCredentials — does not prompt reload for non-session errors):getTestWindow().shownMessages— the array isn't cleared between testsToolkitLogger (
logs to a file — logs warn):editorContext + recommendationHandler (30s timeout on macOS insiders):
Files:
sharedCredentialsProvider.test.ts,toolkitLogger.test.ts,editorContext.test.ts,recommendationHandler.test.ts(all test only)Functional vs Test-Only Changes
uriHandlers.tsserver.tsstackActionCodeLensProvider.tsextension.tsuriHandlers.test.tswalkthrough.test.tssharedCredentialsProvider.test.tstoolkitLogger.test.tseditorContext.test.tsrecommendationHandler.test.tsTesting