Skip to content

client: make caller-side context cancellation/timeout errors match context.Canceled/DeadlineExceeded - #902

Merged
roman-khimov merged 3 commits into
masterfrom
copilot/properly-wrap-context-errors
Oct 30, 2026
Merged

roman-khimov merged 3 commits into
masterfrom
copilot/properly-wrap-context-errors

Conversation

Copilot AI commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

When a caller cancels the context, client methods return a raw gRPC status error (rpc error: code = Canceled ...). errors.Is(err, context.Canceled) doesn't match it, so callers have to string-match the message.

_, err := c.ObjectSearchV2(ctx, cnr, nil, nil, signer, opts)
errors.Is(err, context.Canceled) // now true after cancel(); previously false
  • wrapCtxErr (client/grpc.go)
    • Wraps an error as fmt.Errorf("%w: %w", ctx.Err(), err) when the caller's ctx is done and the error is a gRPC Canceled or DeadlineExceeded status.
    • The original gRPC error stays in the chain, so status.Code(err) still works.
    • Server-originated Canceled/DeadlineExceeded is deliberately left untouched when the local ctx is still live, so a server-side cancellation is never reported as a caller cancellation.
  • Where it's applied
    • Stream opening, send and close-send in client/grpc.go, plus callUnary's RecvMsg.
    • Per-message stream reads and writes in PayloadReader and DefaultObjectWriter, via dowithTimeout.
    • dowithTimeout now takes a ctx as its first (unexported) parameter. Both stream types store the context they were opened with. The stored context is the derived one, so Close()-initiated cancels surface as context.Canceled too.
  • Test: TestWrapCtxErr covers the live ctx, cancelled ctx, nil error and deadline cases.

Review note: the in-stream wrapping is exercised only by the helper's unit test. There's no end-to-end test that cancels mid-stream.

Co-authored-by: roman-khimov <22092804+roman-khimov@users.noreply.github.com>
Copilot AI changed the title [WIP] Wrap context-related NeoFS Client errors client: make caller-side context cancellation/timeout errors match context.Canceled/DeadlineExceeded Oct 27, 2026
Copilot AI requested a review from roman-khimov October 27, 2026 14:18
@roman-khimov
roman-khimov marked this pull request as ready for review October 27, 2026 15:45

@roman-khimov roman-khimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, fix linter. default case in switch should be sufficient for the purpose

Co-authored-by: roman-khimov <22092804+roman-khimov@users.noreply.github.com>

Copilot AI commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, fix linter. default case in switch should be sufficient for the purpose

Added a default case that returns err to the switch in wrapCtxErr (client/grpc.go), replacing the trailing return err. go vet and TestWrapCtxErr pass. I haven't run the linter itself, so please check that the lint job is green.

@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.36364% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.42%. Comparing base (7f44025) to head (3a34950).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
client/grpc.go 75.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #902      +/-   ##
==========================================
+ Coverage   61.38%   61.42%   +0.03%     
==========================================
  Files         177      177              
  Lines       17233    17243      +10     
==========================================
+ Hits        10579    10591      +12     
+ Misses       6654     6652       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roman-khimov
roman-khimov merged commit e883d0c into master Oct 30, 2026
10 checks passed
@roman-khimov
roman-khimov deleted the copilot/properly-wrap-context-errors branch October 30, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Properly wrap context-related NeoFS Client errors

4 participants