fix: honor cancellation when the result relay is blocked - #758
root-Manas wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed changes address cancellation handling and include comprehensive regression coverage.
Review effort: Lite
Findings: None
What changed in this PR
Updates Service.Execute to honor context cancellation when result delivery is blocked.
Changes:
- Uses
sources.SendResultfor cancellation-aware relay delivery. - Adds regression tests for buffered and unbuffered cancellation, ordering, timestamps, and closure.
| File | Description |
|---|---|
uncover.go |
Makes relay output cancellation-aware. |
uncover_test.go |
Adds cancellation and delivery behavior tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
When an SDK caller stops reading
Service.Executeresults and cancels its context, a relay blocked on the full output channel cannot exit. Its unconditional send also prevents the waiter from closing the result channel. Agents already use cancellation-aware sends, but the relay still needs the same handling.Use the existing
sources.SendResulthelper for the relay. Regression tests exercise cancellation with both an unbuffered output and the default 32-result buffer, and verify that normal delivery preserves order, timestamps, and channel closure. Both cancellation cases fail on the unmodifieddevimplementation and pass with this change.Related to #724; this addresses cancellation in the service relay after the producer-side changes.
Validation on Windows/amd64 with Go 1.27.0:
go test . -run TestExecute -count=20go test ./...go vet ./...go build -o ../artifacts/uncover.exe ./cmd/uncoverLocal race testing could not run because this environment has CGO disabled and no C compiler installed.