Repository navigation
Conversation
a2aproject#1274 labelled 1.0 HTTP+JSON success responses application/a2a+json but left errors on application/json, because rest_error_handler is shared with the v0.3 REST adapter. Spec Section 11.1 asks for the A2A media type on responses, and the Section 11.6 error example uses it. Give rest_error_handler and rest_stream_error_handler a keyword-only media_type that defaults to application/json, and have the 1.0 RestDispatcher pass constants.A2A_JSON_MEDIA_TYPE. The v0.3 adapter keeps application/json. Co-Authored-By: Claude Opus 5.5 <[email protected]>
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/cluster/database_event_stream.py | 92.73% | 93.64% | 🟢 +0.91% |
| src/a2a/server/routes/rest_dispatcher.py | 95.08% | 95.14% | 🟢 +0.05% |
| src/a2a/utils/error_handlers.py | 96.67% | 96.70% | 🟢 +0.04% |
| Total | 93.48% | 93.49% | 🟢 +0.01% |
Generated by coverage-comment.yml
1AbdullahButt
left a comment
1AbdullahButt
left a comment
Contributor
There was a problem hiding this comment.
Looks correct. Here's what I checked.
- Every 1.0 REST handler is covered. All eleven handlers in
RestDispatcher(rest_dispatcher.py:183-392) now use_a2a_rest_error_handleror_a2a_rest_stream_error_handler. None is left on the bare decorator. The v0.3rest_adapter.py:71/81still uses the plain decorators, so it keepsapplication/jsonas intended. - The
functools.partialdecorator works.partial(rest_error_handler, media_type=...)(func)callsrest_error_handler(func, media_type=...), andfunctools.wrapsinside still copies the handler's name and docstring. The keyword-onlymedia_typedefault leaves every existing call site unchanged. - The stream path matches the success path.
_handle_streamingpulls the first item eagerly (:150-163), so a pre-stream error such as theA2A-Version: 9.9case in the new test reaches the decorator'sexceptand gets the new media type. An error after the first event is still sent as an SSEevent: error, which this PR doesn't change. - The SDK client is unaffected.
send_http_stream_request(client/transports/http_helpers.py:137-142) callsraise_for_status()before it looks atcontent-type. The status error handlers read the body withresponse.json(), which doesn't check the media type. So a 4xx/5xx labelledapplication/a2a+jsontakes the same path as before. - Headers match the success path. Starlette appends
charsetonly totext/*types, so errors are sent as plainapplication/a2a+json, the same asA2AJSONResponseon success. git merge-treeagainstmainis clean.
Out of scope, for completeness: responses Starlette generates itself for the 1.0 REST mount (unknown path 404, wrong-method 405) are still text/plain, so a TCK check on "every error" could still flag those.
I didn't run the tests.
Reviewed with help from Claude Code.
This branch has not been deployed
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.
Description
#1274 made 1.0 HTTP+JSON success responses
application/a2a+jsonand deliberately left the error path onapplication/json, sincerest_error_handleris shared with the v0.3 REST adapter, calling it worth a follow-up. This is that follow-up.Spec 11.1 says
application/a2a+jsonSHOULD be used for responses, and the error example in 11.6 is served asContent-Type: application/a2a+json.Changes:
rest_error_handlerandrest_stream_error_handlertake a keyword-onlymedia_type, defaulting toapplication/json, so every existing caller is unchanged.RestDispatcher(1.0) decorates its handlers withfunctools.partial(..., media_type=constants.A2A_JSON_MEDIA_TYPE).application/json;test_v03_message_stream_malformed_json_returns_400already pins that.Content-Type, so SDK clients are unaffected.Testing:
./scripts/lint.shpasses (ruff check, ruff format, ty).uv run pytest --cov=src: 2562 passed, 207 skipped; total coverage 94%.tests/server/routes/test_rest_routes.pycover a handler error (404) and an error raised before the first stream event (400 for an unsupportedA2A-Version). Both fail onmainand pass with this change.HTTP_JSON-SVC-001(error Content-Type SHOULD beapplication/a2a+json) now passes.One thing to know: a2a-tck
maincurrently has a check underHTTP_JSON-ERR-001that rejects any error Content-Type other thanapplication/json, which contradicts its ownHTTP_JSON-SVC-001and the 11.6 example. Open a2a-tck PR a2aproject/a2a-tck#238 relaxes it. Until that lands, a TCK run from a2a-tckmainreportsHTTP_JSON-ERR-001instead ofHTTP_JSON-SVC-001for this SDK. This repo'srun-tck.yamlpins TCK0.3.0.beta3on JSON-RPC, so CI here is unaffected.CONTRIBUTINGGuide.bash scripts/format.shfrom the repository root to format)AI assistance: developed with help from Claude Code. I reviewed the change, and the checks listed under Testing were run locally.
Follow-up to #1274 🦕