Skip to content

feat: add onDispose hook#1488

Merged
pi0 merged 9 commits into
mainfrom
feat/event-ondispose
Jul 18, 2026
Merged

feat: add onDispose hook#1488
pi0 merged 9 commits into
mainfrom
feat/event-ondispose

Conversation

@pi0x

@pi0x pi0x commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Resolves #1487 (prototype of the refined proposal). Related: #1478 / #1484.

What

You start a timer, subscribe to something, or open an upstream connection inside a request handler. Where do you clean it up? Today the answer depends on where your app runs: on Node.js you can listen to res.on("close"), on Bun / Deno / Cloudflare Workers there is no reliable place at all — so timers keep ticking after clients are long gone.

onDispose(event, cb) is one answer that works on every runtime:

import { onDispose } from "h3";

app.get("/sse", (event) => {
  const interval = setInterval(() => sse.push("tick"), 1000);

  onDispose(event, () => clearInterval(interval));

  // ... return a streaming response
});

The callback runs exactly once, when the request is truly over — the response finished streaming, the client disconnected, or something errored mid-stream. On disconnect/error, it receives the reason.

Like other h3 utils (getQuery(event), readBody(event)), it's just an importable function — apps that never use it don't ship a single byte of it.

How

  • Node (event.runtime.node.res present): observes res "close" — accurate for streaming and non-streaming bodies, and the body is never wrapped, so srvx's FastResponse _toNodeResponse fast paths stay intact. Premature close disposes with an AbortError DOMException (or res.errored).
  • Web runtimes: a streaming body is piped through an identity TransformStream; the pipeTo promise settles on all three termination paths (normal end → resolve, consumer cancel → reject with the cancel reason, which also propagates upstream, source error → reject). Bodyless responses dispose at response production — once the Response is handed to the runtime, delivery is not observable on web (per the fidelity table in Add event.onDispose(): a portable end-of-event hook for resource cleanup #1487).
  • Opt-in, lazily armed, and tree-shakable: the first onDispose registration installs an observe function on the event; toResponse only keeps a symbol check + observe call at its single final exit (after onResponse resolves, so the hook ordering holds on every path). Because registration carries the machinery, apps that never import onDispose don't bundle any of it — this is why it is a util rather than an H3Event method (a method would anchor ~1 kB min in every core bundle).
  • Hardened once, in core: callbacks run in registration order; sync throws and async rejections are both absorbed (reported via console.error unless silent) — the exact rake found in fix(event-stream): correct stream teardown on close and client disconnect #1484. Pending async callbacks are handed to event.waitUntil() for serverless runtimes. Registering after disposal fires immediately with the recorded reason.

Consumer: createEventStream

The Node-only res.once("close", () => this.close()) autoclose is replaced with onDispose(event, () => this.close()). This fixes two pre-existing web-runtime leaks (verified failing on main, matrix web target):

Note: onClosed's own disconnect-path bug (writer.closed rejection swallowed) remains #1484's scope — this PR intentionally doesn't touch it.

Costs

  • Bundle: +~105 B min / +~42 B gzip on the tracked core bundles (the symbol + arm check in toResponse); the ~1 kB machinery is bundled only where onDispose (or createEventStream) is imported. Limits ratcheted with the same tight headroom as before (main sat 32 B under).
  • Runtime: one symbol check per request when unused; the per-chunk queue hop exists only for events that registered a callback and have a streaming body on a non-Node runtime.

Tests

test/dispose.test.ts (matrix, 8 cases): non-streaming fire, mid-stream non-fire + fire on full consumption, disconnect reason propagation (web: exact cancel reason; node: abort error), onResponse ordering, callback order + sync/async error absorption, late registration, unmodified fast path, status/header preservation on wrapped responses. Plus the two SSE regression tests above. Full suite green (1543 passed), lint/typecheck/build clean.

Caveats / open questions

  • Registration is only guaranteed to observe the end of the event when made during request handling — on web runtimes a first registration made mid-stream can never attach the observer (documented; Node is immune).
  • EventStream autoclose now depends on the event flowing through toResponse (true for any H3 app). A consumer invoking handlers directly without toResponse loses the old Node-only fallback.
  • Body-wrapping perf on the streaming path is unbenchmarked beyond "opt-in only"; happy to add a mitata case if wanted.
  • event.disposed: Promise<unknown> remains a possible alternative/additional shape (see Add event.onDispose(): a portable end-of-event hook for resource cleanup #1487) — trivial to derive from this registry if preferred (though as an H3Event member it would re-anchor the machinery in core bundles).
  • Naming: onDispose(event, cb) sits next to the onRequest/onResponse/onError middleware factories in the public namespace despite having a different shape (per-event util). Open to a different name if that reads as a footgun.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added onDispose to register one-time per-event cleanup callbacks that run when the event/response lifecycle fully ends, including cancellation and errors.
    • Disposal is now integrated with the response flow to ensure the returned response participates in end-of-event cleanup.
  • Documentation
    • Added onDispose docs covering argument semantics, callback timing relative to onResponse, late registration, and failure handling.
  • Bug Fixes
    • Improved end-of-stream handling for event streams and SSE, including streams that are never sent and automatic cleanup on client disconnect.
  • Tests
    • Added disposal and SSE lifecycle test coverage, updated export snapshots, and tightened bundle-size benchmark thresholds.

Resolves #1487

Adds `event.onDispose(cb)`: fires exactly once after global `onResponse`
when the event is fully over — response body finished, client
disconnected, or body errored — on every runtime, not just Node.

- Node: observes `res` "close" (accurate for streaming and
  non-streaming bodies alike; body is never wrapped)
- Other runtimes: streaming bodies are piped through an identity
  TransformStream whose `pipeTo` promise settles on all three
  termination paths; bodyless/non-streaming responses dispose at
  response production
- Opt-in and lazily armed: routes that never register pay one symbol
  check
- Callback receives `undefined` on normal completion or the
  cancel/abort reason; sync throws and async rejections are absorbed
  (reported unless `silent`), pending async work goes to `waitUntil`
- `createEventStream` autoclose now uses `onDispose`, fixing the
  web-runtime leak where a stream that is created but never sent
  never closes

Co-Authored-By: Claude Fable 5 <[email protected]>
@pi0x
pi0x requested a review from pi0 as a code owner July 18, 2026 10:29
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Changes

Adds the public onDispose API, cross-runtime response disposal tracking, EventStream cleanup integration, documentation, lifecycle tests, export snapshots, and tighter bundle-size thresholds.

Event disposal lifecycle

Layer / File(s) Summary
Disposal contract and callback execution
src/utils/internal/dispose.ts, src/utils/response.ts, src/index.ts, docs/..., test/unit/package.test.ts
Defines callback registration, disposal reasons, ordering, error handling, public exports, and documentation.
Response and EventStream disposal wiring
src/response.ts, src/utils/internal/dispose.ts, src/utils/internal/event-stream.ts
Observes response completion, disconnects, and body errors, then closes EventStream instances through disposal callbacks.
Lifecycle validation and size checks
test/dispose.test.ts, test/sse.test.ts, test/bench/bundle.test.ts
Tests response, streaming, disconnect, callback, SSE, and response-preservation behavior while tightening bundle thresholds.

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

Possibly related issues

Possibly related PRs

  • h3js/h3#1411 — Both changes modify EventStream disposal and post-close behavior.
  • h3js/h3#1484 — Both changes address EventStream termination and client-disconnect cleanup.

Suggested reviewers: pi0

Poem

A rabbit watched the streams flow bright,
Then closed each one at event night.
“Dispose!” they cheer, “with callbacks in line,”
Through errors and disconnects, all work will shine.
The hoppy new lifecycle keeps things right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.50% 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 Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: introducing the new onDispose hook.
✨ 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 feat/event-ondispose

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.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/utils/internal/dispose.ts (2)

1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefix this new internal module with _.

Rename dispose.ts to _dispose.ts and update its direct imports.

As per coding guidelines, “Use _ prefix for internal files.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/internal/dispose.ts` around lines 1 - 5, Rename the internal module
from dispose.ts to _dispose.ts and update every direct import or reference to
the kEventDispose symbol to use the new _dispose module path, without changing
the module’s behavior.

Source: Coding guidelines


22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use options objects for these multi-argument functions.

Convert each second and subsequent parameter into an options object, including the private helpers.

As per coding guidelines, “Use options object as second parameter for multi-argument functions.”

Also applies to: 44-44, 84-99

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/internal/dispose.ts` at line 22, Update registerDispose and the
other affected disposal functions and private helpers around the referenced
symbols to accept a single options object after the primary parameter, moving
each existing second and subsequent argument into named properties. Update all
internal call sites and destructuring so behavior remains unchanged, including
the helpers near lines 84–99.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/1.guide/900.api/2.h3event.md`:
- Around line 78-86: Update the onDispose callback documentation in the event
disposal example so a reason of undefined is described only as normal disposal,
not as confirmation that the client received the complete response. Keep the
abort/disconnect or body-error description unchanged and align the comment with
the runtime-specific caveat below.

In `@src/utils/internal/dispose.ts`:
- Around line 72-76: Update the rejection handler in the TransformStream body
pipeTo flow to convert an undefined rejection reason into an AbortError before
passing it to fireDispose, while preserving existing non-undefined reasons. Add
a dispose test assertion covering reader.cancel() without an argument and
verifying the resulting AbortError.

---

Nitpick comments:
In `@src/utils/internal/dispose.ts`:
- Around line 1-5: Rename the internal module from dispose.ts to _dispose.ts and
update every direct import or reference to the kEventDispose symbol to use the
new _dispose module path, without changing the module’s behavior.
- Line 22: Update registerDispose and the other affected disposal functions and
private helpers around the referenced symbols to accept a single options object
after the primary parameter, moving each existing second and subsequent argument
into named properties. Update all internal call sites and destructuring so
behavior remains unchanged, including the helpers near lines 84–99.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3c792328-9c2d-4d5f-b7ac-55a0b4ac9460

📥 Commits

Reviewing files that changed from the base of the PR and between 322fa88 and 9f9e7de.

📒 Files selected for processing (8)
  • docs/1.guide/900.api/2.h3event.md
  • src/event.ts
  • src/response.ts
  • src/utils/internal/dispose.ts
  • src/utils/internal/event-stream.ts
  • test/bench/bundle.test.ts
  • test/dispose.test.ts
  • test/sse.test.ts

Comment thread docs/1.guide/900.api/2.h3event.md Outdated
Comment thread src/utils/internal/dispose.ts
pi0 and others added 2 commits July 18, 2026 10:37
Moves the end-of-event machinery behind the composable util: the first
registration installs the arming function on the event, so core
(`toResponse`) only keeps a symbol check and an `arm` call. Apps that
never import `onDispose` no longer bundle any of it.

Core bundle overhead drops from ~1030 B min / ~400 B gzip to
~105 B min / ~42 B gzip vs main (limits ratcheted accordingly).

Also matches h3's util-first surface (`getQuery(event)`,
`readBody(event)`, ...) instead of adding an `H3Event` method.

Co-Authored-By: Claude Fable 5 <[email protected]>
The public wrapper moves from `utils/event.ts` to `utils/response.ts`
so automd renders it on the response utils page; export relocated to
the Response block in `index.ts` accordingly. No behavior change.

Co-Authored-By: Claude Fable 5 <[email protected]>

@coderabbitai coderabbitai Bot 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.

♻️ Duplicate comments (1)
src/utils/internal/dispose.ts (1)

99-102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Preserve abort reasons from pipeTo().

As noted in a previous review, reader.cancel() with no argument rejects pipeTo() with undefined, which fireDispose() incorrectly treats as normal completion. Normalize the rejected undefined to an AbortError so that a client abort correctly surfaces as a disposal reason rather than a successful completion.

🐛 Proposed fix
   body.pipeTo(writable).then(
     () => fireDispose(event, state, undefined),
-    (reason) => fireDispose(event, state, reason),
+    (reason) => fireDispose(event, state, reason ?? new DOMException("Stream aborted.", "AbortError")),
   );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/internal/dispose.ts` around lines 99 - 102, Update the rejection
handler in the body.pipeTo flow to normalize an undefined rejection reason to an
AbortError before passing it to fireDispose, while preserving any existing
non-undefined reason unchanged. Ensure client aborts are reported as disposal
failures rather than successful completion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/utils/internal/dispose.ts`:
- Around line 99-102: Update the rejection handler in the body.pipeTo flow to
normalize an undefined rejection reason to an AbortError before passing it to
fireDispose, while preserving any existing non-undefined reason unchanged.
Ensure client aborts are reported as disposal failures rather than successful
completion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52e61f24-5fe2-4d6f-a257-37c67ee47988

📥 Commits

Reviewing files that changed from the base of the PR and between 9f9e7de and 0d39bd0.

📒 Files selected for processing (10)
  • docs/1.guide/900.api/2.h3event.md
  • docs/2.utils/9.more.md
  • src/index.ts
  • src/response.ts
  • src/utils/event.ts
  • src/utils/internal/dispose.ts
  • src/utils/internal/event-stream.ts
  • test/bench/bundle.test.ts
  • test/dispose.test.ts
  • test/unit/package.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/utils/internal/event-stream.ts
  • test/bench/bundle.test.ts
  • src/response.ts
  • test/dispose.test.ts

Clearer internal vocabulary: `state.observe(response)` starts observing
the prepared response for end-of-event; `observing` guards against a
second attach. Matches the "body observation" wording used in the docs
and comments. No behavior change.

Co-Authored-By: Claude Fable 5 <[email protected]>
@codecov

codecov Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/utils/internal/dispose.ts 92.45% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

…ompletion

`reader.cancel()` with no argument rejects the observing `pipeTo` with
`undefined` — which is the documented reason for *normal completion*.
Normalize an undefined rejection to an `AbortError` `DOMException`
(strict undefined check, so a deliberate `cancel(null)` reason is
preserved). Extracts the shared `abortError()` helper used by the Node
path.

Co-Authored-By: Claude Fable 5 <[email protected]>

@coderabbitai coderabbitai Bot 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.

♻️ Duplicate comments (1)
src/utils/internal/dispose.ts (1)

100-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Normalize undefined rejection reason to AbortError.

As per past review comments, reader.cancel() with no argument rejects pipeTo() with undefined, which fireDispose treats as a normal completion instead of an abort. Normalize undefined to an AbortError to correctly preserve the cancellation semantics.

🐛 Proposed fix
-  body.pipeTo(writable).then(
-    () => fireDispose(event, state, undefined),
-    (reason) => fireDispose(event, state, reason),
-  );
+  body.pipeTo(writable).then(
+    () => fireDispose(event, state, undefined),
+    (reason) =>
+      fireDispose(
+        event,
+        state,
+        reason ?? new DOMException("Connection closed prematurely.", "AbortError"),
+      ),
+  );
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/internal/dispose.ts` around lines 100 - 103, Update the rejection
handler in the body.pipeTo(...).then flow to convert an undefined reason into an
AbortError before passing it to fireDispose. Preserve existing rejection reasons
unchanged, while ensuring cancellation without a reason is treated as an abort
rather than normal completion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@src/utils/internal/dispose.ts`:
- Around line 100-103: Update the rejection handler in the body.pipeTo(...).then
flow to convert an undefined reason into an AbortError before passing it to
fireDispose. Preserve existing rejection reasons unchanged, while ensuring
cancellation without a reason is treated as an abort rather than normal
completion.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8d349cd6-08b3-4bd3-b6b9-369ae974362e

📥 Commits

Reviewing files that changed from the base of the PR and between ef93154 and f4a3a4c.

📒 Files selected for processing (2)
  • src/response.ts
  • src/utils/internal/dispose.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/response.ts

@pi0x pi0x left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

.

pi0 and others added 2 commits July 18, 2026 10:57
… node

Node emits `res` "close" as soon as the socket dies. When the client
aborts while the handler is still running, the event fires before
`toResponse` attaches the listener, so disposal never fired. Check
`res.closed`/`res.destroyed` at observe time and dispose immediately.

Co-Authored-By: Claude Fable 5 <[email protected]>
On web adapters srvx's `FastResponse` is the native `Response`, so piping
every non-null body through the identity `TransformStream` replaced
buffered sources (string, bytes, JSON) with a JS stream — losing the
runtime's native-source send path and implicit content-length.

`toResponse` now passes the resolved handler value to the observer, which
wraps only live producers (`ReadableStream`, `Blob`, an opaque `Response`,
or an `HTTPResponse` carrying a stream). Buffered bodies dispose at
response production — matching the documented semantics for non-streaming
bodies on non-Node runtimes.

Co-Authored-By: Claude Fable 5 <[email protected]>
Comment thread src/response.ts Outdated
pi0 and others added 2 commits July 18, 2026 13:09
The state lookup stays inside each exit (not hoisted) so a first
registration made inside the `onResponse` hook is still observed.

Co-Authored-By: Claude Fable 5 <[email protected]>
@pi0 pi0 changed the title feat(event): add onDispose portable end-of-event hook feat: add onDispose hook Jul 18, 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.

Add event.onDispose(): a portable end-of-event hook for resource cleanup

2 participants