xtab: replace getFetchFailure polling with stream-level error propagation#309474
Merged
xtab: replace getFetchFailure polling with stream-level error propagation#309474
Conversation
…tion (#308745) Add FetchStreamError class and reject() method to FetchStreamSource so fetch failures propagate through the async iterable stream. Consumers now catch errors naturally via for-await instead of polling a mutable closure at ~7 call sites. - Added FetchStreamError (carries NoNextEditReason) in xtab/common, keeping platform/chat free of inlineEdits dependencies - Added FetchStreamSource.reject() delegating to AsyncIterableSource - Updated _performFetch to call reject()/resolve() instead of setting chatResponseFailure - Removed getFetchFailure parameter from all response format handlers - Added FetchStreamError catch in _streamEditsImpl and xtabCustomDiffPatchResponseHandler Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Changed (1)
|
| Before | After |
|---|---|
 |
 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the XtabProvider NES streaming pipeline to propagate fetch failures through the async line stream itself (instead of relying on a mutable chatResponseFailure + getFetchFailure() polling), aligning failure handling with the stream consumption model.
Changes:
- Added
FetchStreamSource.reject(error)to propagate errors throughAsyncIterableSource. - Introduced
FetchStreamErrorto carry a mappedNoNextEditReasonthrough stream rejection. - Removed
getFetchFailure()polling from xtab response handlers and updated unit tests to inject stream errors directly.
Show a summary per file
| File | Description |
|---|---|
| extensions/copilot/src/platform/chat/common/chatMLFetcher.ts | Adds FetchStreamSource.reject() to forward failures into the stream. |
| extensions/copilot/src/extension/xtab/common/fetchStreamError.ts | Introduces typed FetchStreamError containing NoNextEditReason. |
| extensions/copilot/src/extension/xtab/node/xtabProvider.ts | Switches from polling to rejecting the stream on fetch failure; adds FetchStreamError handling (currently only around the diff loop). |
| extensions/copilot/src/extension/xtab/node/xtabCustomDiffPatchResponseHandler.ts | Removes polling and returns FetchStreamError.reason when the line stream rejects. |
| extensions/copilot/src/extension/xtab/node/responseFormatHandlers.ts | Removes getFetchFailure checks so stream rejection drives termination/errors. |
| extensions/copilot/src/extension/xtab/test/node/responseFormatHandlers.spec.ts | Updates tests to validate stream-error propagation instead of polling callbacks. |
| extensions/copilot/src/extension/xtab/test/node/xtabCustomDiffPatchResponseHandler.spec.ts | Updates tests to reject the input stream with FetchStreamError and assert handler behavior. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 1
The try/catch only covered the diff loop, but handleEditWindowWithEditIntent and handleUnifiedWithXml read from the stream during dispatch and could throw FetchStreamError before the diff phase. Extend the try/catch to wrap both the switch dispatch and the diff loop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
alexr00
approved these changes
Apr 13, 2026
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.
Fixes #308745
Summary
Replaces the fragile
chatResponseFailuremutable variable +getFetchFailure()polling pattern with stream-level error propagation in the XtabProvider's NES streaming pipeline.What changed
Core mechanism:
FetchStreamSourcegains areject(error)method that delegates toAsyncIterableSource.reject(), propagating errors through the async iterable stream. A newFetchStreamErrorclass carries the mappedNoNextEditReasonso catch sites can extract it cleanly._performFetch: Now callsfetchStreamSource.reject()on fetch failure andfetchStreamSource.resolve()on success — replacing the pattern of setting a mutablechatResponseFailurevariable and callingresolve()in.finally().Removed polling: All ~7
getFetchFailure()call sites across_streamEditsImpl,handleEditWindowWithEditIntent,handleUnifiedWithXml,generateInsertEdits,generateEditLines, andXtabCustomDiffPatchResponseHandler.handleResponsehave been removed.Error handling: Two catch sites handle
FetchStreamError:_streamEditsImpl— the main orchestrator loopXtabCustomDiffPatchResponseHandler.handleResponse— has its ownfor awaitloopLayering
FetchStreamErrorlives inextension/xtab/common/(same layer as its consumers), keepingplatform/chatfree ofplatform/inlineEditsdependencies.Testing
responseFormatHandlers.spec.tsandxtabCustomDiffPatchResponseHandler.spec.tsto use stream-based error injection instead of polling callbacksResult
Net −58 lines (97 added, 155 removed). Error propagation is now structural rather than polling-based, making it impossible to miss a failure check.