fix(grpc): finish aio client spans on abandoned calls - #19716
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28383fe535
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| _handle_rpc_error(span, rpc_error) | ||
| raise | ||
| except asyncio.CancelledError: | ||
| span.finish() |
There was a problem hiding this comment.
When a unary-unary or stream-unary operation is cancelled, including the new asyncio.wait_for timeout case, this branch flushes the span without setting grpc.status.code, the error flag, or error details, so the failed RPC is reported as a successful span. Streaming cancellations already use _handle_cancelled_error and are asserted as StatusCode.CANCELLED with error == 1; apply equivalent cancellation metadata here before finishing.
Useful? React with 👍 / 👎.
| client_call_details, | ||
| ) | ||
| call = await continuation(client_call_details, request) | ||
| _handle_add_callback(call, _done_callback_stream(span)) |
There was a problem hiding this comment.
If the returned unary-stream or stream-stream wrapper is never iterated and the RPC terminates with cancellation, a deadline, or a server error, this newly registered callback reaches _done_callback_stream but returns for every non-OK code because it assumes _wrap_stream_response will run an awaited handler. With no consumer that handler never runs, leaving the span unfinished, so the abandoned-call fix currently covers only successful streams; the callback path needs completion ownership for non-OK abandoned calls too, and its associated AIDEV lifecycle note should be updated accordingly.
AGENTS.md reference: AGENTS.md:L67-L70
Useful? React with 👍 / 👎.
1 participant
Description
Fixes #19600.
gRPC asyncio client spans could remain unfinished along two paths: a unary call cancelled before its done callback was registered, and a streaming call whose lazy response wrapper was never iterated.
This change makes completion ownership explicit once an interceptor creates a span:
CancelledError;continuationreturns the call, before returning the lazy wrapper.The existing stream iteration and RPC error-handling paths are unchanged.
Testing
grpcio~=1.59.0: 58 passed, 13 skipped.grpcio~=1.59.0: 67 passed, 4 skipped.scripts/lint checksgit diff --checkRisks
Low. Cancellation still propagates to the caller, and callback timing changes only for streaming calls that already have a concrete gRPC call object. Existing success, error, and cancellation tests continue to pass.
Additional Notes
A local attempt to run the oldest Python 3.9 /
grpcio==1.34.1environment did not reach pytest because grpcio's isolated source build failed while importingpkg_resources. No project configuration was changed to bypass that build failure.