Skip to content

Add graceful server drain and opt-in resend for refused RPCs - #426

Merged
Armster15 merged 5 commits into
mainfrom
server-drain-resend-refused
Oct 21, 2026
Merged

Armster15 merged 5 commits into
mainfrom
server-drain-resend-refused

Conversation

@Armster15

@Armster15 Armster15 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Why

A shutting-down server must distinguish refused calls from calls that started. A refusal before middleware and handlers run is safe to resend.

What changed

  • Add server.drain({ budgetMs }) to the TypeBox router. New procedures receive SERVER_DRAINING before middleware or handlers run.
  • Wait for admitted RPCs and uploads, then close the whole transport when they finish or the budget expires. Other streams do not delay close, and deleted sessions cannot resume.
  • Register streams before middleware. Use two drain states and the existing promise-resolver helper. Repeated drains share their promise and budget.
  • Add opt-in RPC-only resend with a finite maxResends and per-wait newSessionWaitMs. Each resend needs a connected session with a new ID.
  • An abort while waiting returns the original refusal without resending. Once a resend is dispatched, normal call cancellation applies.
  • Share MAX_TIMER_DELAY_MS across all three timer checks, accept negative resend counts as zero, and add focused behavior coverage and minimal documentation.

This builds on the pending-handshake close fix, which is on main. This PR does not modify the handshake path.

Versioning

  • Breaking protocol change
  • Breaking ts/js API change

No wire envelope change. ReaderErrorSchema gains SERVER_DRAINING, so exhaustive built-in error switches need a new case. No version bump in this PR: package.json is not in the diff against main.

Tests

  • Full suite: 813 passed, 1 skipped, across 37 files. npm run check passes TypeScript, Prettier, and ESLint.
  • Ten drain/resend behavior tests in 281 lines. Every test uses the public API, fake timers, and the existing mock transport fixture.
  • (a) Disable admission: the refusal assertion at drain.test.ts:143 gets Ok(null).
  • (b) Move stream registration after middleware: drain.test.ts:158 finds the transport closed before its admitted RPC finishes.
  • (c) Count subscriptions as pending responses: drain.test.ts:180 finds the transport still open after both RPC and upload finish.
  • (c2) Disable the drain timer callback: drain.test.ts:192 finds the transport open at the deadline.
  • (e) Remove the new-session-ID check: drain.test.ts:204 sees three old-session requests instead of one.
  • (f) Remove the resend limit: drain.test.ts:227 sees four requests instead of three for maxResends: 2.
  • Negative budget: restore the removed rejection guard and setup throws Invalid resendRefusedCalls limits before the first-refusal assertion at drain.test.ts:237.
  • (g) Disable the wait timer callback: the refusal assertion at drain.test.ts:250 remains pending until the test times out.
  • (h) Remove the waiter's abort listener: the refusal assertion at drain.test.ts:264 remains pending until the test times out. With the listener restored, the later-connection checks at lines 270-271 show no resend.
  • (i) Route upload finalization into resend while preserving the upload API: the refusal assertion at drain.test.ts:279 remains pending until the test times out.

All mutations were restored. The negative-budget and waiting-abort proofs were rerun in this fix pass. The other proofs remain valid with updated assertion-line references.

Appendix

Decisions

  • Wait only for RPCs and uploads. Waiting for all streams was rejected because a long subscription keeps clients on the draining server for the whole budget.
  • Refuse all new procedure types through the existing cancellation and tombstone path. No new protocol message or public per-session drain.
  • Keep proxy dispatch synchronous, with named RPC resend and session-wait operations. A generic retry framework was rejected because River has no client middleware.
  • Use one router per draining transport. Call server.close() separately to dispose resources. Handlers must not emit the reserved refusal code. The experimental protobuf router is excluded.
  • Time limits must be finite, non-negative, and at most MAX_TIMER_DELAY_MS. Larger Node timer delays run after 1 ms. maxResends must be a safe integer. Non-positive counts disable resend.
  • Abort while waiting preserves the refusal. After dispatch, normal cancellation applies. No extra scheduling boundary or post-wait abort check.

Facts verified in code

  • router/server.ts:242-256,569-571,805-807: admission, cleanup, and registration before middleware.
  • router/server.ts:1081-1113: RPC/upload-only idle detection, bounded deadline, and promise resolution after transport close.
  • router/client.ts:267-276,281-348,380-382: bounded configuration, fresh connected-session waiting, explicit listener cleanup, and RPC-only dispatch.
  • router/client.ts:412-429,710-723: the RPC overload and the existing return relation.
  • transport/transport.ts:40,139: shared timer limit, also imported by the router server and client.
  • router/streams.ts:236 and transport/sessionStateMachine/common.ts:386: existing resolver-helper precedents.

Review dispositions

  • Timer-bound questions: retain the bound because oversized Node timers become 1 ms. Use one shared constant.
  • Negative-count guard: removed. A negative count already stops the loop before any resend. The new behavior test covers this.
  • README: reduced from 24 added lines to 12.
  • Post-wait abort check: deleted under the worth-it gate. Reconsider only if a consumer needs aborts from a Connected listener.
  • Server shape: RPC/upload-only waiting, registration before middleware, two states, and the canonical resolver helper retained.
  • Client shape: named operations, explicit listener cleanup, and the RPC overload retained. No result cast, new retry framework, or scheduling hop.
  • Fixture: unused return property removed.
  • Final size review: no honest structural cut beyond the identified test cleanup. The remaining size is in type overloads and lifecycle cleanup, not another abstraction.

~ written by ⠕ Replit

@Armster15
Armster15 changed the base branch from main to close-pending-handshakes October 21, 2026 22:24
@Armster15
Armster15 added this pull request to stack #428 October 21, 2026 22:24
Base automatically changed from close-pending-handshakes to main October 21, 2026 22:49
Long-lived procedures must not delay transport shutdown. Track admitted calls before middleware and finish when RPCs and uploads end, with a bounded timer as the deadline.

Assisted-by: Replit
Only refused RPCs can be sent again on a fresh connected session. Keep waiting and cleanup in one named operation, and check cancellation before dispatching each resend.

Assisted-by: Replit
Keep one behavior test for each drain and resend risk. Share the transport fixture and state when cancellation preserves a refusal instead of cancelling an in-flight call.

Assisted-by: Replit
@Armster15
Armster15 force-pushed the server-drain-resend-refused branch from 161fcc0 to 6079d4a Compare October 21, 2026 22:49
Comment thread router/server.ts
Comment thread router/client.ts
Comment thread README.md
Share the timer limit already used by the transport and let negative resend counts disable resend. Keep abort handling in the session waiter, remove unused fixture output, and shorten the README to the public contract.

Assisted-by: Replit
@Armster15
Armster15 marked this pull request as ready for review October 21, 2026 23:01
@Armster15
Armster15 requested a review from a team as a code owner October 21, 2026 23:01
@Armster15
Armster15 requested review from daweifeng-replit and komalagrawal-alt and removed request for a team and komalagrawal-alt October 21, 2026 23:01

@rileyjones-dev rileyjones-dev left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Lfg

@Armster15
Armster15 merged commit 5dc7b41 into main Oct 21, 2026
9 checks passed
@Armster15
Armster15 deleted the server-drain-resend-refused branch October 21, 2026 23:02
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.

2 participants