Repository navigation
Conversation
The unary wrapper only called after_handler_run from except Exception, so a cancelled call got before_handler_run with no matching after call. The stream wrapper did call it from finally, but reported error=None, which reads as a clean end of the stream. Both now pass the CancelledError through to the after hook and re-raise it.
🧪 Code Coverage (vs
|
mykytanetipa
self-requested a review
November 5, 2026 14:15
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
Follow-up to #1402, as asked in the review there.
server_plugin_hookshandled cancellation differently for unary and streaming methods, and neither told the plugin about it.after_handler_runfromexcept Exception. A cancelled call (a client timeout, a disconnect,asyncio.wait_foraround the handler) gotbefore_handler_runand then nothing, so a plugin that opens a span or takes a slot inbefore_handler_runnever got to close it.finally, but witherror=None, so a cancelled stream looked the same as one that finished normally.Both wrappers now catch
asyncio.CancelledErroralongsideException, pass it toafter_handler_runaserror, and re-raise it.ServerResponse.erroris already typedBaseException | None. A stream closed withaclose()still reportserror=None, sotest_stream_closed_early_still_gets_final_callis unchanged.The two new tests use a task store whose
getblocks, so they cancel insideon_get_taskandon_subscribe_to_taskwithout going throughActiveTask.subscribeand don't depend on #1402. Without thesrcchange the unary test gets noafter_handler_runcall at all and the stream test getserror=None.Full suite on 3.10: 2544 passed, 207 skipped, 4 xfailed, 1 xpassed.
./scripts/lint.shpasses (the three ty warnings it prints are the existing unusedtype: ignorein the client transports).I left the client side alone.
BaseClient._execute_with_interceptorshas the sameexcept Exceptionshape forafter_client_request, andPluginAgentExecutor.executepassesNonetoafter_executionwhen the execution is cancelled. I can send those separately if you want them.CONTRIBUTINGGuide.bash scripts/format.shfrom the repository root to format)