Skip to content

fix(jsonrpc): lenient decode of error.data to preserve server errors - #386

Open
dongjiang1989 wants to merge 1 commit into
a2aproject:mainfrom
dongjiang1989:fix-json-decode
Open

fix(jsonrpc): lenient decode of error.data to preserve server errors#386
dongjiang1989 wants to merge 1 commit into
a2aproject:mainfrom
dongjiang1989:fix-json-decode

Conversation

@dongjiang1989

Copy link
Copy Markdown
Contributor

Problem

The a2av0 compat JSON-RPC transport (and the v2 JSON-RPC client, which shares the same code path) decodes responses into jsonrpc.ClientResponse, whose Error.Data field is typed []*errordetails.Typed (internal/jsonrpc/jsonrpc.go). JSON-RPC 2.0 / A2A v0.3 define error.data as any JSON value, and v0.3 servers commonly return a plain object there (e.g. {"reason":"detail"}). When they do, json.Decode(&resp) fails with cannot unmarshal object into Go struct field ..., so the caller gets failed to decode response instead of the server's actual error — the real code and message are lost.

Repro: point an a2av0 JSON-RPC client at a v0.3 server that returns:

{"jsonrpc":"2.0","id":"1","error":{"code":-32603,"message":"real error message","data":{"reason":"detail"}}}

The client surfaces failed to decode response; the real message never reaches the caller.

Same bug class as #318 (REST streaming error events), applied to the JSON-RPC path.

Closes #385.

Fix

Change Error.Data from []*errordetails.Typed to json.RawMessage so the initial decode always succeeds, regardless of what shape error.data has. FromJSONRPCError then parses it leniently:

  1. Try to unmarshal as []*errordetails.Typed (the standard typed array).
  2. Otherwise, if the data is a JSON object, wrap it as a single struct-typed detail so its contents are not silently dropped.
  3. Otherwise, still return the error with code + message preserved.

ToJSONRPCError is updated to marshal the typed slice into json.RawMessage on the way out. The ordering of TypedDetails (ErrorInfo first, then struct details) is preserved to keep existing round-trip behaviour.

Because the fix lives in the shared internal/jsonrpc/jsonrpc.go, both the v2 client (a2aclient/jsonrpc.go) and the v0 compat client (a2acompat/a2av0/jsonrpc_client.go) benefit.

Tests

  • internal/jsonrpc/jsonrpc_test.go: added TestFromJSONRPCError_NonTypedData with 5 sub-cases — plain object, string, typed array, null, absent — verifying code/message are always preserved and object data is surfaced as a struct detail.
  • a2aclient/jsonrpc_test.go: added TestJSONRPCTransport_ErrorWithNonTypedData (end-to-end transport test with a server returning a plain-object data) and TestJSONRPCTransport_ErrorWithStringData (string data). Updated the existing TestJSONRPCTransport_ErrorDetails to marshal typed details into json.RawMessage to match the new field type.

Verification

  • go vet ./... — clean
  • golangci-lint run ./... — 0 issues
  • go test -count=1 ./... — all 29 packages pass

Change Error.Data from []*errordetails.Typed to json.RawMessage so that
v0.3 servers returning error.data as a plain JSON object (or any non-
array value) no longer cause the whole response decode to fail. The
client now always preserves code+message and only converts to typed
details when the data actually parses as such. Fixes a2aproject#385.

Signed-off-by: dongjiang <dongjiang1989@126.com>
@chopmob-cloud

Copy link
Copy Markdown
Contributor

Verified the root cause and the fix against current main. On main, Error.Data is typed []*errordetails.Typed, so a v0.3 server that returns error.data as a JSON object (which JSON-RPC 2.0 and A2A allow, since data is any JSON value) fails the whole Decode, and the real code and message are lost. Retyping Data to json.RawMessage, building the result from code and message first, and only then parsing data leniently is the right shape, and keeping it in internal/ means the field-type change is not an external break. The array path still decodes and ToJSONRPCError re-marshals correctly.

One correctness gap in the single-object branch. It uses errordetails.NewFromStruct(singleDetail), and NewFromStruct is NewTyped(StructType, details), so it hard-codes TypeURL to StructType and leaves any @type key inside Value. But Typed has a custom UnmarshalJSON (errordetails.go:67) that reads @type, sets TypeURL to it, and deletes it from Value. So a server that returns a single, non-array typed detail as an object, for example {"@type":"type.googleapis.com/google.rpc.ErrorInfo","reason":"...","metadata":{...}}, is mis-decoded: the ErrorInfo type is not recognized, the ErrorInfo metadata-extraction branch in FromJSONRPCError (the d.TypeURL == errordetails.ErrorInfoType check) is skipped, and @type leaks into Value.

Suggested fix: route the object branch through *errordetails.Typed rather than NewFromStruct, so the same @type handling as the array path applies. Something like unmarshalling e.Data into a var d errordetails.Typed and appending &d when it succeeds. That keeps a single typed object consistent with a one-element typed array.

I confirmed this against your branch locally: a test that feeds a bare {"@type": ErrorInfoType, ...} object into FromJSONRPCError fails as-is (the ErrorInfo is not recognized and @type leaks into Value), and passes once the object branch routes through *errordetails.Typed, with go test still green across internal/jsonrpc, errordetails and a2aclient. A bare-string data value and a plain StructType object with no @type both still behave correctly.

Narrow but real, and CI is green otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a2av0 JSON-RPC client fails to decode v0.3 error responses with non-typed data, masking the real error

2 participants