Skip to content

Silence orphaned unhandledRejections from extension management#314792

Merged
bryanchen-d merged 1 commit into
mainfrom
bryanchen-d/silence-extension-mgmt-unhandled-rejections
May 6, 2026
Merged

Silence orphaned unhandledRejections from extension management#314792
bryanchen-d merged 1 commit into
mainfrom
bryanchen-d/silence-extension-mgmt-unhandled-rejections

Conversation

@bryanchen-d
Copy link
Copy Markdown
Contributor

What

Silence two boot-time / race-condition paths in the shared-process extension management service that produce orphan unhandledRejection events, surfacing as unhandlederror-Cannot read the extension from ... telemetry buckets.

These rejections carry no new diagnostic information — the underlying install failure is already reported by the primary install task (extensionGallery:install event with the real error, stack, and extension id) or logged by the cleanup/migrate flows themselves.

Two paths fixed

1. Duplicate-install wait promise — abstractExtensionManagementService.ts

When a second install request races for an extension that's already being installed (very common at boot for github.copilot-chat because it gets re-requested by auto-update + restore + sync), a sibling promise is created via Event.toPromise(onDidInstallExtensions).then(...) and pushed into alreadyRequestedInstallations.

It's awaited on the success path (line 451), but if the outer try throws first (e.g. another extension in the same batch fails) control jumps to catch and the sibling promise is never observed → Node fires unhandledRejection.

Fix: attach a no-op rejection handler on a separate handle so the rejection is always observed. The original promise is still awaited via joinAllSettled on the happy path, and the underlying failure is already surfaced through the primary task's installExtensionResultsMap entry.

2. Fire-and-forget startup tasks — sharedProcess/contrib/extensions.ts

extensionManagementService.cleanUp();        // not awaited, no .catch
this.migrateUnsupportedExtensions();          // not awaited, no .catch

Any rejection becomes an unhandled rejection at sharedProcess boot. Both functions already log their own internal failures (and migrateUnsupportedExtensions wraps each installFromGallery in its own try/catch), so the only thing missing is a top-level .catch(logService.error) to swallow the orphan.

Why not just fix scanLocalExtension?

The throw inside scanLocalExtension is correct — the caller asked to read an extension that isn't where the caller said it would be (a separate path-construction bug producing //ext-id-style URIs that still needs investigation). Wrapping it would silently corrupt install state. The right fix is at the unawaited consumers.

Impact

Removes the entire unhandlederror-Cannot read the extension from ... family from the top error buckets (~7M machines / ~120M hits in the 14d window) without losing any diagnostic signal — user-initiated install failures still produce notifications and extensionGallery:install telemetry as before.

Two boot-time/race-condition paths in the shared-process extension management service produce `unhandledRejection` events that surface as `unhandlederror-Cannot read the extension from ...` telemetry buckets, even though the underlying install failure is already reported by the primary install task or logged by the cleanup/migrate flows themselves.

1. Duplicate-install wait promise (abstractExtensionManagementService.ts): when a second install request races for an extension that's already being installed, a sibling promise is created via `Event.toPromise(onDidInstallExtensions).then(...)` and pushed into `alreadyRequestedInstallations`. It's awaited on the success path, but if the outer `try` throws first (e.g. another extension in the same batch fails) control jumps to `catch` and the sibling promise is never observed -> Node fires `unhandledRejection`. Attach a no-op rejection handler so the rejection is always observed; the original promise is still awaited via `joinAllSettled` on the happy path, and the underlying failure is already surfaced through the primary task's error result.

2. Fire-and-forget startup tasks (sharedProcess/contrib/extensions.ts): `extensionManagementService.cleanUp()` and `migrateUnsupportedExtensions()` are called without `await` and without a `.catch`. Any rejection becomes an unhandled rejection. Both paths already log their own internal failures, so the only thing missing is a top-level `.catch(logService.error)` to swallow the orphan.

Net effect: removes the entire `unhandlederror-Cannot read the extension from ...` family from top error buckets without losing any real diagnostic signal.
Copilot AI review requested due to automatic review settings May 6, 2026 17:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses boot-time race conditions in extension management that can lead to orphaned unhandledRejection events (notably the unhandlederror-Cannot read the extension from ... telemetry family), without suppressing the underlying install/cleanup error reporting that already exists elsewhere in the pipeline.

Changes:

  • Prevents orphaned rejections when waiting on a duplicate/overlapping extension install by attaching a no-op rejection handler to the “wait for install completion” promise.
  • Ensures shared-process startup tasks (cleanUp, unsupported-extension migration) are not able to surface as unhandled rejections by adding top-level .catch(...) logging.
Show a summary per file
File Description
src/vs/platform/extensionManagement/common/abstractExtensionManagementService.ts Adds a rejection handler to the duplicate-install “wait” promise so failures can’t become orphaned unhandledRejections if the outer flow exits early.
src/vs/code/electron-utility/sharedProcess/contrib/extensions.ts Adds .catch(...) logging to fire-and-forget startup tasks to prevent shared-process boot-time unhandled rejections.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

@bryanchen-d bryanchen-d marked this pull request as ready for review May 6, 2026 17:42
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@deepak1556

Matched files:

  • src/vs/code/electron-utility/sharedProcess/contrib/extensions.ts

@bryanchen-d bryanchen-d enabled auto-merge May 6, 2026 17:47
@bryanchen-d bryanchen-d merged commit b2c6357 into main May 6, 2026
34 checks passed
@bryanchen-d bryanchen-d deleted the bryanchen-d/silence-extension-mgmt-unhandled-rejections branch May 6, 2026 17:55
@vs-code-engineering vs-code-engineering Bot added this to the 1.120.0 milestone May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants