Skip to content

Stop aborted request bodies from logging duplicate unhandled rejections in @astrojs/node - #17971

Merged
ematipico merged 6 commits into
mainfrom
triage-17966
Sep 15, 2026
Merged

ematipico merged 6 commits into
mainfrom
triage-17966

Conversation

@matthewp

@matthewp matthewp commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Changes

When a client disconnected while a custom src/fetch.ts handler was reading request.json(), Node rejected the body read with ECONNRESET. The standalone Node adapter let that error escape, so it was logged as an unhandled rejection—often twice.

createAppHandler() now catches render failures:

  • The exact error emitted by an aborted incoming request is ignored because the client is already gone.
  • Other errors are logged once with the request URL and return a 500 response.

Each app handler owns its request context and aborted-error tracking. A handler only reports detached rejections created in its own request context, so other handlers do not log the same rejection.

This prevents aborted body reads from reaching unhandledRejection, without hiding unrelated ECONNRESET errors from user code.

Fixes #17966

Testing

  • Adds one regression test that sends a partial JSON POST over raw TCP, waits until src/fetch.ts starts reading the body, disconnects, then verifies no ECONNRESET or unhandled-rejection output is logged and the server remains healthy.

Docs

No docs update needed because this fixes error handling without changing a public API or configuration.

…ne mode

When a client disconnects mid-request, Node destroys the incoming message
with an ECONNRESET error that propagates through the request body stream.
With a custom src/fetch.ts handler, request.json() rejects and the error
escapes app.render() into the standalone handler's floating promise,
surfacing as an unhandled rejection. The same rejection was logged twice
because createAppHandler registered a process-wide unhandledRejection
listener on every call (the standalone entrypoint calls it once for the
exported handler and again when starting the server).

The unhandledRejection listener is now registered once at module level and
tracks the request URL through an AsyncLocalStorage context. Errors caused
by a client disconnect are tagged on the exact error object (WeakSet) and
dropped silently; genuine rejections are still logged once with the request
URL.
…ntries

Revision addressing adversarial review findings:

- A rejected src/fetch.ts handler is now handled at the adapter boundary
  where the request URL and response are still available: it is logged once
  via the adapter logger with the URL and completes with a 500 (or destroys
  the connection once headers were sent), instead of floating as a process
  unhandled rejection that could not be attributed and left the client
  waiting. The process-level unhandledRejection listener is now reserved for
  genuinely detached promises.
- The unhandledRejection listener, request context, and aborted-error
  registry are shared process-wide through globalThis so that multiple built
  standalone entries loaded into one process register a single listener and
  route rejections to the app that handled the request, instead of logging
  each rejection once per entry.
- Tests: the abort test now synchronizes on the fixture signal that body
  reading started and asserts output stability over an interval; a new test
  covers a fetch handler that rejects directly (one URL-bearing log + 500);
  a new suite starts two built entries in one child process and verifies
  aborts stay silent and each unhandled rejection is logged exactly once.
The duplicate log from a faulty multi-listener implementation could arrive in
a later event-loop turn than the first line, so the two-entry assertions now
wait for the child output to stop growing before slicing, matching the abort
tests.
@changeset-bot

changeset-bot Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e3c2eea

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the pkg: integration Related to any renderer integration (scope) label Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

e18e dependency analysis

No dependency warnings found.

@matthewp
matthewp marked this pull request as ready for review September 14, 2026 23:17

@ematipico ematipico 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.

Looks good

@ematipico
ematipico merged commit 0037c1a into main Sep 15, 2026
25 checks passed
@ematipico
ematipico deleted the triage-17966 branch September 15, 2026 11:55
@astrobot-houston astrobot-houston mentioned this pull request Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg: integration Related to any renderer integration (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aborted request bodies cause duplicate unhandled rejection logs in @astrojs/node

2 participants