fix: don't report NES fetch failures as unexpected errors (fixes #325643)#325649
Open
vs-code-engineering[bot] wants to merge 1 commit into
Open
fix: don't report NES fetch failures as unexpected errors (fixes #325643)#325649vs-code-engineering[bot] wants to merge 1 commit into
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
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.
Summary
Benign, transient network failures from the Copilot Next Edit Suggestions (NES / xtab) fetch are being reported to error telemetry as unexpected errors, generating noise like
unhandlederror-An unexpected error occurred: {"type":"networkError","reason":"...net::ERR_NAME_NOT_RESOLVED."}.When the NES request fails (here a DNS failure,
net::ERR_NAME_NOT_RESOLVED), the fetch error is modeled as aNoNextEditReason.FetchFailure._getNextEditCanThrowthen re-throws that modeled reason as an exception. Because thegetNextEdit()promise is one of the racers passed toraceAndAll(promises, onUnexpectedError)ininlineCompletionProvider.ts,raceAndAllforwards every racer rejection to itserrorHandler— which isonUnexpectedError. So a benign connectivity failure ends up in the unexpected-error telemetry pipeline (surfacing viaonUnexpectedErrorinsrc/vs/base/common/errors.ts, the deepest deminified frame).FetchFailureis a first-class expected reason, not a programming error — a failed fetch simply means "no suggestion right now". Genuine internal errors are modeled separately asNoNextEditReason.Unexpected. The fix stops converting the expectedFetchFailurereason into a thrown exception while keepingUnexpectedthrowing.Fixes #325643
Recommended reviewer:
@ulugbeknaCulprit Commit
Not pinpointed to a single commit within the time budget (shallow clone; no local
git log). The relevant machinery is:raceAndAllerror handling — last touched bya28a677cfd6b7af2fa276d4fc662cff9be0c800f"Fix raceAndAll error handling (C# "Language mode" disappeared from the list #3996)" (@benibenj), which forwards all racer rejections to the suppliederrorHandler.raceAndAll(..., onUnexpectedError)NES call site and theFetchFailure/Unexpectedre-throw innextEditProvider.tspredate that and belong to the NES/inline-edits feature area (@ulugbekna).The bug is the interaction: an expected
FetchFailureis thrown by the NES provider and then classified as unexpected by the race error handler.Code Flow
flowchart TD A[xtab _performFetch: DNS failure net::ERR_NAME_NOT_RESOLVED] --> B[mapChatFetcherErrorToNoNextEditReason] B -->|NetworkError case| C[NoNextEditReason.FetchFailure ErrorUtils.fromUnknown] C --> D[_getNextEditCanThrow] D -->|OLD: throw error.error| E[getNextEdit rejects] E --> F["raceAndAll(promises, onUnexpectedError) racer rejects"] F -->|errorHandler(error)| G[onUnexpectedError] G --> H[error telemetry: 'An unexpected error occurred'] D -->|NEW: FetchFailure handled, return empty result| I[No throw - no suggestion, telemetry status noEdit:fetchFailure]Affected Files
extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.tsNoNextEditReason.Unexpectedre-throws;NoNextEditReason.FetchFailureis now handled as an expected non-result (logged, telemetry status set) instead of thrownSupporting (unchanged) context:
extensions/copilot/src/extension/xtab/node/xtabProvider.ts—mapChatFetcherErrorToNoNextEditReasonmapsNetworkError→FetchFailure.extensions/copilot/src/extension/inlineEdits/vscode-node/inlineCompletionProvider.ts—raceAndAll(..., onUnexpectedError)racer whose rejection was being reported.extensions/copilot/src/extension/inlineEdits/vscode-node/raceAndAll.ts— forwards every racer rejection toerrorHandler.Repro Steps
net::ERR_NAME_NOT_RESOLVED; before this change the resultingFetchFailureis thrown and reported viaonUnexpectedError, producing theunhandlederror-An unexpected error occurred: {"type":"networkError",...}telemetry event. After this change the request resolves with no suggestion and no unexpected-error report.How the Fix Works
Chosen approach —
nextEditProvider.ts: Split the combinedFetchFailure || Unexpectedbranch.NoNextEditReason.Unexpectedcontinues tothrow error.error(genuine programming errors stay observable).NoNextEditReason.FetchFailureis now handled inline as an expected outcome: it logs the failure (logger.trace,logContext.setError), records the diagnostic signal on telemetry (setNextEditProviderError, statusnoEdit:fetchFailure), and falls through to the existing empty-result return. This is the data-producer principle applied at the right layer — the NES provider is the producer of the rejectedgetNextEdit()promise, so preventing it from throwing an expected fetch failure stops the rejection at its source rather than filtering it downstream at the crash site (onUnexpectedError). NologService.error/telemetry call is removed and notry/catchis added; a modeled expected reason simply stops being re-thrown as an exception.Alternatives considered:
raceAndAll(..., onUnexpectedError)call site. Rejected: it fixes the reporting boundary (near the crash site) rather than the producer, and reliably detecting a network error there requires string-matching the flattenedfromUnknownmessage (ErrorUtils.fromUnknowncollapses the structuredChatFetchErrorinto a genericError), which is fragile.NetworkErrortoGotCancelled/NoSuggestionsinmapChatFetcherErrorToNoNextEditReason. Rejected: it mislabels a network failure as a cancellation or an empty-model-response, polluting those telemetry categories and hit-rate metrics.Recommended Owner
@ulugbekna— active owner of the NES / inline-edits feature area (extensions/copilot/src/extension/inlineEdits/**,xtab/**) with recent commits to these exact files (e.g.652b4aaaper-language NES gating,b5f83615NES telemetry). Team member with commits in the last 90 days.