Skip to content

http-transport: fix fixed-length response backpressure and abort handling - #278

Closed
denny-il wants to merge 2 commits into
mainfrom
dev/fix-271-fixed-length-backpressure
Closed

denny-il wants to merge 2 commits into
mainfrom
dev/fix-271-fixed-length-backpressure

Conversation

@denny-il

@denny-il denny-il commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Closes #271

What was broken

The fixed-length response path in packages/transports/src/http-server/node.ts registered onWritable for every source chunk. uWS honors only the first registration, so later backpressure could leave a new waiter unresolved while a stale callback retried with the wrong chunk offset.

The fixed-length reader also lacked the abort-side body cancellation used by the chunked path, leaving a stalled reader.read() and its lock alive after client abort.

Fix

  • Share one lazily registered writable dispatcher with a swappable pending waiter across fixed-length and chunked response paths.
  • Retry each fixed-length chunk from the current uWS write offset after drain events.
  • Cancel fixed-length body reads on abort and cancel the source on zero-length or other early response completion.
  • Preserve the current shared server-host APIs and transport ownership.

Tests

  • Real uWS fixed-length backpressure coverage in packages/transports/tests/neemata/http/node-runtime.spec.ts.
  • Deterministic two-chunk partial-write and offset coverage with one writable registration.
  • Pending-waiter abort, stalled-read abort, zero-length cancellation, and early-completion source cancellation coverage.

Verification

  • pnpm vitest run packages/transports/tests/http-server/fixed-length-stream.spec.ts packages/transports/tests/neemata/http/node-runtime.spec.ts --reporter=agent (11 tests passed)
  • pnpm vitest run --project @nmtjs/transports --reporter=agent (239 tests passed)
  • pnpm run fmt
  • pnpm run check

@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ea0b1d6b-1c68-4881-9a73-4eda75df43ef

📝 Walkthrough

Walkthrough

Node/uWebSockets streaming now shares Promise-based writable waiting across fixed-length and chunked responses. Fixed-length pumping retries partial writes, cancels stalled reads on abort, and cleans up cancellation hooks. Tests cover backpressure, registration reuse, offsets, and abort behavior.

Changes

Writable backpressure coordination

Layer / File(s) Summary
Shared writable waiter and streaming pump
packages/http-transport/src/runtimes/node.ts
Adds centralized writable waiting, integrates it into fixed-length and chunked streaming, retries partial writes, and handles reader cancellation and aborts.
Backpressure and abort coverage
packages/http-transport/tests/node-runtime.spec.ts
Adds integration and dispatcher tests for paused reads, resumed writes, single registration reuse, offset tracking, abort cleanup, and fixed-length edge cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ReadableStream
  participant handleFixedLengthStream
  participant HttpResponse
  ReadableStream->>handleFixedLengthStream: read chunk
  handleFixedLengthStream->>HttpResponse: tryEnd chunk
  HttpResponse-->>handleFixedLengthStream: partial write
  handleFixedLengthStream->>HttpResponse: waitWritable
  HttpResponse-->>handleFixedLengthStream: onWritable wake
  handleFixedLengthStream->>HttpResponse: retry remaining bytes
Loading

Possibly related PRs

  • neematajs/neemata#267: Both changes modify Node streaming backpressure coordination and writable-event handling.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement the shared onWritable dispatcher and fixed-length resume logic needed to address #271.
Out of Scope Changes check ✅ Passed The test additions and node runtime updates stay focused on the fixed-length response backpressure fix.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes to fixed-length response backpressure and abort handling.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev/fix-271-fixed-length-backpressure

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@denny-il
denny-il force-pushed the dev/fix-271-fixed-length-backpressure branch from 350c5bf to d1b114e Compare August 23, 2026 07:08
@denny-il

Copy link
Copy Markdown
Contributor Author

Closing as no longer relevant: the packages this changes are removed in #367 (part of the Effect migration stack).

@denny-il denny-il closed this Sep 23, 2026
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.

http-transport: fixed-length chunk path re-registers onWritable per chunk (uWS honors only the first)

1 participant