Repository navigation
internal/jsonrpc2: plumb cancellation cause through req.ctx (resolve TODO #51365) #1100
Description
Background
handleAsync in internal/jsonrpc2/conn.go checks whether an incoming request was already canceled before dispatching it to the handler. When it was, it tries to report why. Today it can only recover the reason by inspecting the connection-level s.writeErr, guarding it behind a TODO:
go-sdk/internal/jsonrpc2/conn.go
Lines 668 to 678 in ce393ac
| // Only deliver to the Handler if not already canceled. | |
| if err := req.ctx.Err(); err != nil { | |
| c.updateInFlight(func(s *inFlightState) { | |
| if s.writeErr != nil { | |
| // Assume that req.ctx was canceled due to s.writeErr. | |
| // TODO(#51365): use a Context API to plumb this through req.ctx. | |
| err = fmt.Errorf("%w: %v", ErrServerClosing, s.writeErr) | |
| } | |
| }) | |
| c.processResult("handleAsync", req, nil, err) | |
| continue |
The referenced Go issue (golang/go#51365) shipped as context.WithCancelCause / context.Cause in Go 1.21, and this module already targets Go 1.25, so the API the TODO was waiting for is now available.
Problem
Beyond being a stale TODO, the current approach has a latent mis-attribution: s.writeErr is connection-level state, not per-request. Any request canceled for an unrelated reason (e.g. a peer cancel notification via Cancel, or a read-side teardown) will be reported as ErrServerClosing as long as s.writeErr happens to be non-nil at that moment. The cancellation reason is not tied to the specific request that was canceled.
Proposal
Carry the cancellation cause on each request's context instead of inferring it from global state:
- Change
incomingRequest.cancelfromcontext.CancelFunctocontext.CancelCauseFunc(build it viacontext.WithCancelCause). - At the write-failure teardown, cancel each in-flight request with the cause:
r.cancel(fmt.Errorf("%w: %v", ErrServerClosing, err)). - Other cancel sites (
Cancel, read-side teardown, normal completion) passnil, preserving the existingcontext.Canceledbehavior. - In
handleAsync, replace thes.writeErrlookup withcontext.Cause(req.ctx).
ctx.Err() still returns context.Canceled in all cases (unchanged), so external behavior is preserved; only the reported cause becomes accurate and per-request. Net effect on conn.go is a small diff across ~6 sites and removes the guessing/global-state read from the hot path.
Questions before I open a PR
- Would you welcome a PR for this, or is the current behavior intentional for now?
- Any preference on scope — cause-plumbing only for the write-failure path (the one the TODO calls out), or should read-side teardown also carry a descriptive cause rather than bare
context.Canceled?
I have a working change (all internal/... and mcp tests pass) and am happy to send it once you confirm the direction.
Environment
- go-sdk: current
main - Go: 1.25
Activity
guglielmo-san commentedon Jul 17, 2026
Hi @aha-jansen, thank you for your detailed breakdown. For me you can proceed with the proposed PR, including the cause at Read-side teardown
internal/jsonrpc2: plumb cancellation cause through request context (m…
mcp: carry a cancelled notification's reason as the request's cancel …