Skip to content

subscriptions: cancel_and_drain returns a failed cancel write as Err - #1187

Merged
wboayue merged 3 commits into
mainfrom
drain-cancel-write-error
Nov 5, 2026
Merged

wboayue merged 3 commits into
mainfrom
drain-cancel-write-error

Conversation

@wboayue

@wboayue wboayue commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Part of #1167. The reporter's worker uses cancel_and_drain to settle a request before replacing it, so it needs to see whether the cancel was actually written.

Gap. Both cancel_and_drains logged a failed cancel write and kept draining. The caller saw the failure only indirectly, as a later reset or Unconfirmed. When the stream had already ended with an error on our side (a decode error, BufferLimitExceeded), the call returned Unconfirmed right away and left the cancel to Drop, which also only logs. A local error doesn't mean TWS has ended the request. In async, the write was awaited before the deadline-bounded drain, so a stuck write (the case #1179 fixed for reconnect) could hold the call well past its deadline.

Change (sync and async):

  • A failed cancel write returns Err(e). cancelled is set before the write, so Drop never writes a second cancel.
  • A rejection or local error already read before the call still settles the outcome (Rejected / Unconfirmed), but the cancel is written first wherever one is due, so a write failure reaches the caller.
  • Async: the write runs in its own task as before, and its result comes back over a oneshot under timeout_at(deadline). If the deadline passes first, the call returns Unconfirmed and the task finishes the write. When no receiver is left (deadline passed, or future dropped), the task logs the error.
  • Sync: the blocking write isn't bounded. The docs say so.
  • Docs: the outcome table adds the write error and says a successful write is not an acknowledgement or a freed pacing slot. cancel() still returns ().
  • MessageBusStub gains with_failing_sends() and, async-only, with_stalled_sends(). Both are used by the new tests.

6.0 behavior change: docs/migration-6.0.md §19 (15, 16 and 18 are taken by open PRs #1170, #1183 and #1169), plus a CHANGELOG Changed entry.

Verified

  • New tests: drain_returns_cancel_write_error and drain_after_error_returns_cancel_write_error (sync and async); async drain_deadline_bounds_cancel_write. On the old code the four write-error tests fail and the stalled-write test hangs; with the change, all pass. drain_after_error_is_unconfirmed_and_cancels_on_drop is now ..._writes_cancel.
  • cargo fmt; clippy, rustdoc (-D warnings) and cargo build --examples on all three feature configs; just test (7152 passed, re-run after rebasing onto 6.0: remove Order::fa_profile, basis_points and basis_points_type #1184); integration crates build and pass clippy.

A failed cancel write was logged and the drain went on, so the caller saw
it only indirectly. Return it. After a local error (decode, buffer limit)
the cancel was left to Drop, which only logs; write it inline instead. In
async, the deadline also bounds the write; a write still pending then
finishes in its own task and logs. Sync and async.

MessageBusStub gains with_failing_sends and with_stalled_sends (async).
@wboayue
wboayue merged commit 2958bb9 into main Nov 5, 2026
7 checks passed
@wboayue
wboayue deleted the drain-cancel-write-error branch November 5, 2026 21:23
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.

1 participant