fix: avoid unhandled rejection from orphan .finally() in gallery extension download dedup (fixes #323530) - #323537
Open
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
…nsion download dedup (fixes #323530) createInstallExtensionTask memoizes in-flight gallery downloads in extractingGalleryExtensions, but called promise.finally(cleanup) as a standalone statement and returned the raw promise. When the download/ verify rejected (e.g. signature verification UnknownError), the returned promise was handled by the caller while the discarded .finally() promise rejected with no handler, producing an unhandled rejection in the shared process that surfaced in error telemetry. Fold .finally() into the stored/returned promise so the single handled promise carries the cleanup, matching the existing idiom used by scanExtensions in the same file. No error is suppressed; the real caller still receives and handles the rejection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated error-fix analysis for an error-telemetry issue. Fix at the producer of the unhandled rejection — the download-dedup memoization — not at the signature-verification crash site.
Summary
The error bucket
SignatureVerificationInternal: 署名の検証が 'UnknownError' エラーで失敗しました。("Signature verification failed with 'UnknownError' error") is reported as an unhandled promise rejection in the shared process, with a 2-frame stack:ExtensionManagementService.downloadExtension(extensionManagementService.ts:381)async ExtensionManagementService.downloadAndExtractGalleryExtension(extensionManagementService.ts:300)The throw itself is legitimate: when the
@vscode/vsce-signmodule throws during verification,ExtensionSignatureVerificationService.verify()returnsverificationStatus = UnknownError;downloadExtension'sswitchhas noUnknownErrorcase, so it correctly fails closed by throwingExtensionManagementError(..., SignatureVerificationInternal). The UI path already handles this error gracefully (dialog with Learn More / Report Issue / Install Anyway).The actual defect is in
createInstallExtensionTask, which memoizes in-flight gallery downloads in theextractingGalleryExtensionsmap. It stored/returned the raw download promise and then calledpromise.finally(cleanup)as a standalone, discarded statement.promise.finally()returns a new promise; when the download rejects, the returned promise is handled by the caller, but the discarded.finally()promise rejects with no handler → unhandled rejection → error telemetry. This leaks every download/extract rejection, with the signatureUnknownErrorcase being the most frequent.Fixes #323530
Recommended reviewer:
@sandy081Culprit Commit
The unhandled-rejection defect is a latent bug in the
extractingGalleryExtensionsdedup block ofcreateInstallExtensionTask. The orphanpromise.finally(...)statement is present unchanged at commit7f503b81(2026-04-09) with identical line numbers, and the most recent fix-window edit to this file —88e21a04("handle edge cases while updating built in extensions",@sandy081, 2026-04-10) — added unrelated lines and did not touch the dedup block. The exact commit that first introduced the orphan.finally()was not pinned within the time budget.The
recent-regression/stable-anomalysignal correlates with the auto-update for built-in extensions feature series by@sandy081(e.g.143b631d"Enable auto-update for built-in extensions", and follow-upsaa0db967,7f503b81,88e21a04, late March–April 2026), which added a background gallery download/verify code path. This greatly increased background download/verification frequency, amplifying the pre-existing unhandled-rejection leak into a visible telemetry spike for version 1.126.0 (shipped commit7e7950df89d055b5a378379db9ee14290772148a).Code Flow
flowchart TD A[Install / background auto-update of gallery extension] --> B[createInstallExtensionTask] B --> C{extractingGalleryExtensions<br/>has in-flight promise for key?} C -- yes --> R[return cached promise] C -- no --> D[downloadAndExtractGalleryExtension] D --> E[downloadExtension → verify signature] E --> F[vsce-sign module.verify throws] F --> G[verificationStatus = UnknownError] G --> H["downloadExtension switch has no UnknownError case →<br/>throw ExtensionManagementError(SignatureVerificationInternal)"] H --> I[returned/stored promise rejects → awaited by task → HANDLED] H --> J["orphan promise.finally(...) promise rejects → NO handler"] J --> K[Unhandled rejection in shared process → error telemetry bucket]Affected Files
src/vs/platform/extensionManagement/node/extensionManagementService.ts— increateInstallExtensionTask, fold the.finally()cleanup into the promise that is stored inextractingGalleryExtensionsand returned, eliminating the discarded orphan promise branch.Repro Steps
@vscode/vsce-signmodule throws, soExtensionSignatureVerificationService.verify()yieldsverificationStatus = UnknownError. In practice this is reached frequently by the background auto-update path for built-in extensions, which performs many background downloads.downloadExtensionhas noUnknownErrorcasein its verificationswitch, so it throwsExtensionManagementError(..., SignatureVerificationInternal)(correct fail-closed behavior).createInstallExtensionTask, the returned/stored dedup promise rejects and is handled by the awaiting task, but the discardedpromise.finally(...)branch rejects with no handler.SignatureVerificationInternal: 署名の検証が 'UnknownError' エラーで失敗しました。.How the Fix Works
Chosen approach —
src/vs/platform/extensionManagement/node/extensionManagementService.ts,createInstallExtensionTask:Before:
After:
.finally()is now chained into the single promise that is both stored in the map and returned to the caller, so that one handled promise carries the cleanup. The discarded second promise — the only branch without a rejection handler — no longer exists, so a rejecting download can no longer produce an unhandled rejection. The fix is placed at the producer of the unhandled rejection (the dedup memoization), not at the crash site (the signature-verification throw) — following the principle of fixing where the unhandled value is produced rather than where it surfaces. Notry/catchis added and nologService.error/throwis removed: the awaiting caller still receives and handles the rejection exactly as before; only the duplicate, unobserved promise branch is removed. The change mirrors the already-correct idiom used byscanExtensionsin the same file (its scan dedup chains.finally()into the stored/awaited promise), so it restores consistency rather than inventing a new pattern.After this change, the
createInstallExtensionTaskdedup block cannot produce an unhandled rejection on a failed download, because the only promise derived from the download (the stored/returned one) now includes the.finally()cleanup and is awaited by the caller — there is no longer a separate, unobserved.finally()promise to reject.Alternatives considered:
UnknownErrorcaseat thedownloadExtensionswitchso it stops throwingSignatureVerificationInternal— rejected: failing closed on an unverifiable signature is correct security behavior, and the UI already handles this error gracefully; the throw is not the bug, the orphan rejection is..catch()to the orphan.finally()promise — rejected: this swallows/duplicates handling and is strictly worse than removing the orphan branch and reusing the existing, verified idiom.try/catch— rejected: that hides the error from the telemetry pipeline instead of fixing the unhandled-rejection producer.Recommended Owner
@sandy081(Sandeep Somavarapu). Rationale: declared GDPRownerof the signature-verification telemetry event inextensionSignatureVerificationService.ts; author of the extension-management auto-update series that introduced/amplified the affected background download path; long-standing owner of the extension-management area. Active in the repository within the last days (e.g.88e21a04, 2026-04-10).