fix: tolerate invalid tool-call JSON in language model wrapper (fixes #322123)#322127
Open
vs-code-engineering[bot] wants to merge 1 commit into
Open
fix: tolerate invalid tool-call JSON in language model wrapper (fixes #322123)#322127vs-code-engineering[bot] wants to merge 1 commit into
vs-code-engineering[bot] wants to merge 1 commit into
Conversation
…322123) The model can stream malformed JSON for tool-call arguments. The language model wrapper threw a bare Error from a fire-and-forget finished callback, producing an unhandled rejection in error telemetry without cleanly aborting the response. Align with the other tool-call consumers (langModelServer, messagesApi) by logging the diagnostic and falling back to empty parameters so the tool call is still surfaced to the consuming extension.
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
The bundled Copilot Chat extension throws
Error('Invalid JSON for tool call')from the language-model response wrapper when an Anthropic model streams malformed JSON for a tool call'sarguments. The throw happens inside anasync"finished" callback that the streaming layer invokes fire-and-forget, so it surfaces as an unhandled promise rejection in error telemetry rather than cleanly aborting the request. Impact: noisyunhandlederrorreports and a dropped tool call whenever the model emits invalid argument JSON, even though every sibling consumer of the same data already tolerates it.Fixes #322123
Recommended reviewer:
@bhavyausCulprit Commit
Code Flow
sequenceDiagram participant Model as Anthropic model participant Stream as messagesApi streaming participant Wrapper as languageModelAccess callback participant Telemetry as Error telemetry Model->>Stream: streams partial_json deltas - malformed Note over Stream: Concatenates deltas faithfully -<br/>reconstruction is correct, not buggy Stream->>Wrapper: tool call arguments = invalid JSON Note over Wrapper: ⚠️ Root cause: JSON.parse throws inside<br/>a fire-and-forget finished callback Wrapper-->>Telemetry: 💥 Unhandled rejection -<br/>Invalid JSON for tool callAffected Files
extensions/copilot/src/extension/conversation/vscode-node/languageModelAccess.tscatchclause logged the error thenthrow new Error('Invalid JSON for tool call')inside the fire-and-forget finished callbackextensions/copilot/src/platform/endpoint/node/messagesApi.tsinput_json_delta.partial_jsonintocopilotToolCalls[].arguments; the assembled bytes match exactly what the model sent. The outbound path at L287-L292 already tolerates the same condition ("Keep empty object if parse fails")extensions/copilot/src/extension/agents/node/langModelServer.tstry { input = call.arguments ? JSON.parse(call.arguments) : {}; } catch { input = {}; }extensions/copilot/src/extension/intents/node/toolCallingLoop.ts'{}'and defers parsing rather than throwingRepro Steps
This is non-deterministic — it requires an Anthropic model to stream syntactically invalid JSON in a tool call's
arguments. To reproduce/observe:input_json_delta.partial_jsonis not valid JSON (a truncated or garbled argument stream). This happens intermittently under real traffic.provideLanguageModelResponse's finished callback hits theJSON.parsecatch branch andthrows.unhandlederror, and the tool call is dropped.To force it deterministically in a test, make the endpoint emit a tool call with
argumentsset to a malformed string such as'{"a":'.How the Fix Works
Chosen approach —
languageModelAccess.ts(here the crash site is also the correct fix site, because the bad data originates outside our code):The
argumentsstring is produced by the model, an external/untrusted boundary — there is no internal producer to correct (themessagesApiaccumulator faithfully reconstructs exactly what the model sent). The established convention across every other consumer of this same field is to tolerate invalid JSON and fall back to empty parameters. The change touches only the existingcatchclause:this._logService.error(err, ...)diagnostic, so the malformed-JSON condition is still surfaced to logging/telemetry — the violation is not silenced.throw new Error('Invalid JSON for tool call'). That throw lived inside anasyncfire-and-forget finished callback, so it never cleanly aborted the response — it only produced an unhandled rejection. Removing it loses no real error handling.parameters = {}and still callsprogress.report(new LanguageModelToolCallPart(...)), so the tool call is surfaced to the consuming extension and the tool_use/tool_result pairing is preserved (silently dropping the tool call would desync the Anthropic conversation contract).This follows the fix-at-the-trust-boundary principle: handle untrusted external input where it crosses into typed code, while keeping the diagnostic log rather than masking it. It mirrors three sibling consumers of the same data —
langModelServer.ts(catch { input = {} }),messagesApi.tsoutbound ("Keep empty object if parse fails"), andtoolCallingLoop.ts(normalizes/defers instead of throwing).After this change, the finished callback can no longer emit an unhandled
Error('Invalid JSON for tool call'): theJSON.parsefailure path logs the error and assignsparameters = {}instead of throwing, soprogress.reportalways receives a valid object.Alternatives considered:
tool_usewithout a matchingtool_resultdesyncs the Anthropic message contract and can wedge the conversation; reporting{}keeps the pairing intact.Recommended Owner
@bhavyaus— feature-area owner for Anthropic model integration (feature-area fallback, used because the bug is pre-existing with no culprit commit). Liveness gate: PASS (recent commits through mid-June 2026, including Anthropic messages-API work). Team-membership gate: PASS (named area owner who authors changes into core and is on the team).@bryanchen-dis the secondary owner for this area and the current issue assignee. The prior area owner was excluded because that working-areas entry is intentionally empty pending handover.