fix(grpc): correct error code mapping and enforce A2A-Version validation - #1167
fix(grpc): correct error code mapping and enforce A2A-Version validation#1167avilleroy51 wants to merge 2 commits into
Conversation
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/request_handlers/grpc_handler.py | 95.42% | 89.20% | 🔴 -6.22% |
| Total | 93.00% | 92.88% | 🔴 -0.12% |
Generated by coverage-comment.yml
Three gRPC error mappings diverged from specification.md:1178-1192 (the normative error-code table): PushNotificationNotSupportedError, UnsupportedOperationError and VersionNotSupportedError were mapped to FAILED_PRECONDITION instead of UNIMPLEMENTED. Separately, the gRPC transport never validated the A2A-Version header at all — DefaultGrpcServerCallContextBuilder.build() didn't expose invocation metadata to the rest of the request-handling code, unlike the JSON-RPC/REST dispatchers which already enforce this via @validate_version. This adds state['headers'] (mirroring the HTTP route builders) and a single _validate_a2a_version() check applied at the one point all 11 RPC methods funnel through, replicating the semantics of validate_version (missing header treated as '0.3'; major version must match). Fixes a2aproject#1166
f9c0293 to
998b675
Compare
|
Hi maintainers — checking in on this PR. It looks like no CI workflow has run yet on this commit (no check-suites registered), which is expected for a first-time external contributor — GitHub requires a maintainer to approve running workflows on fork PRs. Could someone please approve the workflow run (or take a look) when you have a moment? Happy to address any feedback. Thanks! |
| types.TaskNotCancelableError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.PushNotificationNotSupportedError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.UnsupportedOperationError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.PushNotificationNotSupportedError: grpc.StatusCode.UNIMPLEMENTED, | ||
| types.UnsupportedOperationError: grpc.StatusCode.UNIMPLEMENTED, | ||
| types.ContentTypeNotSupportedError: grpc.StatusCode.INVALID_ARGUMENT, | ||
| types.InvalidAgentResponseError: grpc.StatusCode.INTERNAL, | ||
| types.ExtendedAgentCardNotConfiguredError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.ExtensionSupportRequiredError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.VersionNotSupportedError: grpc.StatusCode.FAILED_PRECONDITION, | ||
| types.VersionNotSupportedError: grpc.StatusCode.UNIMPLEMENTED, |
There was a problem hiding this comment.
The error code remapping (FAILED_PRECONDITION -> UNIMPLEMENTED) contradicts the current spec (v1.0.1 and main), which reverted these to FAILED_PRECONDITION in spec PR #1627 (a2aproject/A2A#1627) which was merged 2026-04-14. The SDK's existing values are correct; this change should be dropped.
| } | ||
|
|
||
|
|
||
| def _validate_a2a_version(server_context: ServerCallContext) -> None: |
There was a problem hiding this comment.
This duplicates existing logic in src/a2a/utils/version_validator.py (_get_actual_version (partially), _is_version_compatible, and the identical VersionNotSupportedError message), which will lead to diverging implementations of the same rule.
Consider extracting version_validator.py helper functions to module scope and reusing them in both places
2 participants
Summary
Fixes #1166 — two bugs in the gRPC transport, found while cross-checking
grpc_handler.pyagainstspecification.md's normative error-code table and against the version-validation behavior already enforced on JSON-RPC/REST.PushNotificationNotSupportedError,UnsupportedOperationErrorandVersionNotSupportedErrorwere mapped toFAILED_PRECONDITIONin_ERROR_CODE_MAP. Perspecification.md:1178-1192(the normative A2A-error → gRPC-status table) all three should map toUNIMPLEMENTED.A2A-Versionnever validated on gRPC. JSON-RPC and REST dispatchers already enforce version compatibility via@validate_version(PROTOCOL_VERSION_1_0). The gRPC transport had no equivalent —DefaultGrpcServerCallContextBuilder.build()never exposedinvocation_metadata()to the rest of the request-handling code, so there was no way for any check to see the caller's declared version. This addsstate['headers'](same shape the HTTP route builders already populate) and a single_validate_a2a_version()check applied where all 11 RPC methods funnel through_build_call_context, replicatingvalidate_version's semantics (missing header ⇒ treated as0.3; major version must match1.0).Test plan
tests/server/request_handlers/test_grpc_handler.pyalone: 43/43 passedtests/integration,tests/install_smoke,tests/compat— out of scope — and modules with pre-existing, unrelatedModuleNotFoundError:respx,opentelemetry, migrations): 997 passed, 0 failedruff checkon both changed files: all checks passedmock_grpc_contextfixture now defaultsinvocation_metadata.return_valuetoa2a-version: 1.0(same conventionTestClient(..., headers={'A2A-Version': '1.0'})already uses for JSON-RPC/REST tests)test_abort_context_error_mappingcases that asserted the oldFAILED_PRECONDITIONmapping updated toUNIMPLEMENTEDTestGrpcExtensionscases that overrideinvocation_metadataupdated to include('a2a-version', '1.0')