Skip to content

fix: do not await writeEarlyHints callback in Node.js#1387

Open
ruguoba wants to merge 1 commit into
h3js:mainfrom
ruguoba:fix/writeEarlyHints-callback
Open

fix: do not await writeEarlyHints callback in Node.js#1387
ruguoba wants to merge 1 commit into
h3js:mainfrom
ruguoba:fix/writeEarlyHints-callback

Conversation

@ruguoba

@ruguoba ruguoba commented May 15, 2026

Copy link
Copy Markdown

Fixes #1383

Problem

In Node.js v24, does not call the callback when passed an empty hints object (or possibly in other cases). This causes the returned Promise to never resolve, and the request handler hangs indefinitely.

Root Cause

The original code wrapped in a Promise:

If the callback is never called, the Promise never resolves.

Fix

Since early hints are fire-and-forget by nature, we now call without wrapping it in a Promise:

This prevents the handler from hanging while still sending the early hints.

Summary by CodeRabbit

  • Refactor
    • Streamlined early hints handling for Node.js runtime.

Review Change Stack

In Node.js v24, res.writeEarlyHints() does not call the callback when
passed an empty hints object, causing the returned Promise to never
resolve and the request handler to hang indefinitely.

Since early hints are fire-and-forget by nature, we now call
writeEarlyHints without wrapping it in a Promise.

Fixes h3js#1383
@ruguoba ruguoba requested a review from pi0 as a code owner May 15, 2026 01:59
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

writeEarlyHints now directly calls event.runtime.node.res.writeEarlyHints(hints) and returns immediately on Node.js runtimes with native early-hints support, replacing the prior callback-based Promise wrapper that was preventing resolution.

Changes

Node.js Early Hints Fast Path

Layer / File(s) Summary
Node.js native early hints fast path
src/utils/response.ts
The Node.js runtime fast path now directly invokes res.writeEarlyHints without a callback-based Promise wrapper, allowing the function to return immediately when the native API is available instead of hanging indefinitely.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • h3js/h3#1288: Both PRs update src/utils/response.ts's writeEarlyHints to use Node's native res.writeEarlyHints when available and otherwise handle the non-native fallback differently (with/without Promise wrapping and via Link: rel=preload headers), making the main PR changes directly related to #1288.

Suggested reviewers

  • pi0

Poem

🐰 A promise once hung in the dark,
Waiting for hints that would not embark—
No more delays, just swift and bright,
Node's native path now takes flight! ⚡

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: do not await writeEarlyHints callback in Node.js' accurately describes the main change: removing Promise wrapping to avoid awaiting a callback that may not be invoked.
Linked Issues check ✅ Passed The code change directly addresses issue #1383 by removing Promise wrapping around res.writeEarlyHints(), preventing handlers from hanging when the callback is not invoked.
Out of Scope Changes check ✅ Passed All changes in src/utils/response.ts are focused on fixing the writeEarlyHints callback issue reported in #1383 with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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 and usage tips.

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

🧹 Nitpick comments (1)
src/utils/response.ts (1)

111-114: 💤 Low value

Consider simplifying return type to void.

Since both the native and fallback code paths now return void synchronously (no path returns Promise<void> after removing the Promise wrapper), the return type annotation could be simplified from void | Promise<void> to just void for clarity.

♻️ Simplify return type
 export function writeEarlyHints(
   event: H3Event,
   hints: Record<string, string | string[]>,
-): void | Promise<void> {
+): void {
🤖 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/response.ts` around lines 111 - 114, The writeEarlyHints function's
annotated return type (void | Promise<void>) is outdated because both native and
fallback code paths now return synchronously; update the signature to return
just void by changing the type annotation on writeEarlyHints and any related
overloads/usages (referencing writeEarlyHints and H3Event) so the function
signature and callers reflect a synchronous void return.
🤖 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.

Nitpick comments:
In `@src/utils/response.ts`:
- Around line 111-114: The writeEarlyHints function's annotated return type
(void | Promise<void>) is outdated because both native and fallback code paths
now return synchronously; update the signature to return just void by changing
the type annotation on writeEarlyHints and any related overloads/usages
(referencing writeEarlyHints and H3Event) so the function signature and callers
reflect a synchronous void return.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e74757b7-0168-4c56-8f07-e17f4331c326

📥 Commits

Reviewing files that changed from the base of the PR and between 84244b4 and 2920fc6.

📒 Files selected for processing (1)
  • src/utils/response.ts

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.

writeEarlyHints never resolves with node runtime

1 participant