fix: return graceful quit-early result when diff worker returns null (fixes #317338)#317341
Open
vs-code-engineering[bot] wants to merge 1 commit into
Open
fix: return graceful quit-early result when diff worker returns null (fixes #317338)#317341vs-code-engineering[bot] wants to merge 1 commit into
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
…ixes #317338) When the editor worker service cannot compute a diff (worker crash, resource unavailability), it returns null. Previously this null was converted into a thrown error 'no diff result available' that reached error telemetry as an unhandled rejection (4,679 users affected on 1.120.0 stable). Instead of throwing, return a quitEarly diff result matching the existing cancellation handling pattern. The diff editor will show the 'quit early' state rather than propagating an unhandled error. 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.
🔧 Error Fix
Summary
The error
no diff result availableis thrown indiffProviderFactoryService.ts:151wheneditorWorkerService.computeDiff()returnsnull. This happens when the web worker cannot compute the diff (worker crash, resource unavailability, or model disposal on the worker side). The error propagates as an unhandled rejection through the diff editor view model, reaching error telemetry.This is a stable-release anomaly affecting 4,679 users with 10,672 hits on version 1.120.0 across all platforms.
Fixes #317338
Recommended reviewer:
@hedietCulprit Commit
Commit range narrowing was inconclusive per the regression scan comment. The error existed in the earliest 1.120.0-insider build under a different bucket ID (
8ddfa303) with low volume. The spike on stable suggests increased diff editor usage rather than a specific code change amplifying the failure path. The throw statement has existed since the original implementation.Code Flow
sequenceDiagram participant VM as DiffEditorViewModel participant DP as WorkerBasedDocumentDiffProvider participant WS as EditorWorkerService participant W as Worker VM->>DP: computeDiff(original, modified, options, token) DP->>WS: computeDiff(uri, uri, options, algorithm) WS->>W: $computeDiff(...) W-->>WS: null (worker failed) WS-->>DP: null Note over DP: cancellationToken NOT cancelled DP->>DP: throw Error("no diff result available") DP-->>VM: rejected promise VM->>VM: .catch(rejectIfNotCanceled) re-throws Note over VM: Unhandled rejection reaches telemetryAffected Files
src/vs/editor/browser/widget/diffEditor/diffProviderFactoryService.tssrc/vs/editor/browser/widget/diffEditor/diffEditorViewModel.tsrejectIfNotCanceledsrc/vs/editor/browser/services/editorWorkerService.tsnullwhen worker can't computeRepro Steps
How the Fix Works
Chosen approach (
diffProviderFactoryService.ts:150-157): Instead of throwing an error whenresultis null, return a gracefulquitEarlydiff result with empty changes — the same structure already used for the cancellation case at lines 140-148. This fixes the error at the data producer (where the error is constructed) rather than at downstream consumers. ThequitEarly: trueflag signals to the diff editor UI that the computation did not complete normally, which is the correct semantic for "worker couldn't compute the diff."Alternatives considered: Adding a try/catch in
diffEditorViewModel.tsaround the.computeDiff()call was rejected because it would silence the error at a consumer site rather than fixing the producer, violating the data-producer principle.Recommended Owner
@hediet— owner annotation in the telemetry declaration at line 127 of diffProviderFactoryService.ts, and author of the diff editor infrastructure.