feat(node,deno,bun): implement env vars, remove srvx-deprecated ones, fix handler docs #4013
+60
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@wadefletch is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughDocuments environment variables for Node, Bun, and Deno; renames Node handler preset to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
wadefletch
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/2.deploy/10.runtimes/bun.md`:
- Line 21: The document has a heading-level skip (h1 to h3) at the "Environment
Variables" section similar to deno.md which triggers markdownlint MD001; update
the heading hierarchy so levels are consecutive (e.g., change the "Environment
Variables" heading to h2 or adjust surrounding headings) to eliminate the h1→h3
jump and keep consistency with the project style and deno.md; ensure the change
only alters the markdown heading markers (e.g., from ### to ##) in the
docs/2.deploy/10.runtimes/bun.md file.
In `@docs/2.deploy/10.runtimes/deno.md`:
- Line 21: The "Environment Variables" heading currently uses an h3 (###) which
jumps from h1 to h3; change it to h2 (##) so heading levels don't skip, or
alternatively change surrounding headings in this doc to match the project's
existing convention (as seen in "1.node.md") so all runtime docs use consistent
heading levels; update the "Environment Variables" heading text accordingly and
verify the rest of this markdown follows the same h-level pattern.
|
|
||
| :read-more{to="https://bun.sh"} | ||
|
|
||
| ### Environment Variables |
There was a problem hiding this comment.
Same h1 → h3 heading level skip as in deno.md.
Same markdownlint MD001 concern. Consistent with the project pattern but worth addressing if the project decides to fix heading levels.
🤖 Prompt for AI Agents
In `@docs/2.deploy/10.runtimes/bun.md` at line 21, The document has a
heading-level skip (h1 to h3) at the "Environment Variables" section similar to
deno.md which triggers markdownlint MD001; update the heading hierarchy so
levels are consecutive (e.g., change the "Environment Variables" heading to h2
or adjust surrounding headings) to eliminate the h1→h3 jump and keep consistency
with the project style and deno.md; ensure the change only alters the markdown
heading markers (e.g., from ### to ##) in the docs/2.deploy/10.runtimes/bun.md
file.
| deno run --unstable --allow-net --allow-read --allow-env .output/server/index.ts | ||
| ``` | ||
|
|
||
| ### Environment Variables |
There was a problem hiding this comment.
Heading level skips from h1 to h3.
Static analysis (markdownlint MD001) flags this as jumping from # to ###. However, this is consistent with the existing pattern in 1.node.md (Line 28), so this may be intentional for the project. Consider aligning with the project's heading conventions.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 21-21: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
In `@docs/2.deploy/10.runtimes/deno.md` at line 21, The "Environment Variables"
heading currently uses an h3 (###) which jumps from h1 to h3; change it to h2
(##) so heading levels don't skip, or alternatively change surrounding headings
in this doc to match the project's existing convention (as seen in "1.node.md")
so all runtime docs use consistent heading levels; update the "Environment
Variables" heading text accordingly and verify the rest of this markdown follows
the same h-level pattern.
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/2.deploy/10.runtimes/1.node.md`:
- Line 57: Replace the unhyphenated phrase "Node.js compatible handler" with
"Node.js-compatible handler" in the sentence that reads "Nitro also has a more
low-level preset that directly exports a Node.js compatible handler usable for
middleware and custom servers." to apply the correct hyphenation for the
compound modifier; update the string exactly where that text appears in the
docs.
| **Preset:** `node_middleware` | ||
|
|
||
| Nitro also has a more low-level preset that directly exports a function with `(req, res) => {}` signature usable for middleware and custom servers. | ||
| Nitro also has a more low-level preset that directly exports a Node.js compatible handler usable for middleware and custom servers. |
There was a problem hiding this comment.
Hyphenate “Node.js‑compatible”.
Grammar nit: “Node.js compatible” should be “Node.js‑compatible”.
🧰 Tools
🪛 LanguageTool
[grammar] ~57-~57: Use a hyphen to join words.
Context: ...l preset that directly exports a Node.js compatible handler usable for middleware...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@docs/2.deploy/10.runtimes/1.node.md` at line 57, Replace the unhyphenated
phrase "Node.js compatible handler" with "Node.js-compatible handler" in the
sentence that reads "Nitro also has a more low-level preset that directly
exports a Node.js compatible handler usable for middleware and custom servers."
to apply the correct hyphenation for the compound modifier; update the string
exactly where that text appears in the docs.
Wire NITRO_UNIX_SOCKET, NITRO_SHUTDOWN_DISABLED, and NITRO_SHUTDOWN_TIMEOUT through to srvx serve options in the Node, Bun, and Deno server presets. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The Handler (advanced) section referenced preset `node` and export `listener`, but `node` is an alias for `node-server` (not the middleware handler). The actual preset name is `node-middleware` and the export is `middleware`. Co-authored-by: Cursor <cursoragent@cursor.com>
wadefletch
force-pushed
the
wade/implement-env-vars
branch
from
a47c3ae to
3482064
Compare
8 tasks
wadefletch
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/2.deploy/10.runtimes/bun.md`:
- Around line 29-30: Update the docs entry for the environment variable
NITRO_SHUTDOWN_TIMEOUT in the Bun runtime docs to use the correct default of
5000 milliseconds (to match Node.js docs and srvx's gracefulTimeout), i.e.,
change the documented default from `3000` to `5000` and ensure the description
remains consistent with the existing NITRO_SHUTDOWN_DISABLED line and the
Node.js `NITRO_SHUTDOWN_TIMEOUT` documentation.
In `@docs/2.deploy/10.runtimes/deno.md`:
- Around line 28-29: Update the NITRO_SHUTDOWN_TIMEOUT default in
docs/2.deploy/10.runtimes/deno.md from 3000 to 5000 to match the Node.js docs
and srvx's default gracefulTimeout (5 seconds); specifically change the
`NITRO_SHUTDOWN_TIMEOUT` line so it states the default is `5000` milliseconds
(and verify the `NITRO_SHUTDOWN_DISABLED` line remains unchanged) to keep
defaults consistent with bun.md and Node.js documentation.
Comment on lines
+29
to
+30
| - `NITRO_SHUTDOWN_DISABLED` - Disables the graceful shutdown feature when set to `'true'`. Defaults to `'false'`. | ||
| - `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `3000` milliseconds. |
There was a problem hiding this comment.
Inconsistent default for NITRO_SHUTDOWN_TIMEOUT across runtime docs.
This says the default is 3000 milliseconds, but the Node.js docs (line 37 of 1.node.md) document it as 5000 milliseconds. Per the PR objectives and srvx's default gracefulTimeout of 5 seconds, the correct value should be 5000.
Proposed fix
-- `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `3000` milliseconds.
+- `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `5000` milliseconds.🤖 Prompt for AI Agents
In `@docs/2.deploy/10.runtimes/bun.md` around lines 29 - 30, Update the docs entry
for the environment variable NITRO_SHUTDOWN_TIMEOUT in the Bun runtime docs to
use the correct default of 5000 milliseconds (to match Node.js docs and srvx's
gracefulTimeout), i.e., change the documented default from `3000` to `5000` and
ensure the description remains consistent with the existing
NITRO_SHUTDOWN_DISABLED line and the Node.js `NITRO_SHUTDOWN_TIMEOUT`
documentation.
Comment on lines
+28
to
+29
| - `NITRO_SHUTDOWN_DISABLED` - Disables the graceful shutdown feature when set to `'true'`. Defaults to `'false'`. | ||
| - `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `3000` milliseconds. |
There was a problem hiding this comment.
Same inconsistent NITRO_SHUTDOWN_TIMEOUT default as in bun.md.
Should be 5000 milliseconds to match the Node.js docs and srvx's default gracefulTimeout of 5 seconds.
Proposed fix
-- `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `3000` milliseconds.
+- `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `5000` milliseconds.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `NITRO_SHUTDOWN_DISABLED` - Disables the graceful shutdown feature when set to `'true'`. Defaults to `'false'`. | |
| - `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `3000` milliseconds. | |
| - `NITRO_SHUTDOWN_DISABLED` - Disables the graceful shutdown feature when set to `'true'`. Defaults to `'false'`. | |
| - `NITRO_SHUTDOWN_TIMEOUT` - Sets the amount of time (in milliseconds) before a forced shutdown occurs. Defaults to `5000` milliseconds. |
🤖 Prompt for AI Agents
In `@docs/2.deploy/10.runtimes/deno.md` around lines 28 - 29, Update the
NITRO_SHUTDOWN_TIMEOUT default in docs/2.deploy/10.runtimes/deno.md from 3000 to
5000 to match the Node.js docs and srvx's default gracefulTimeout (5 seconds);
specifically change the `NITRO_SHUTDOWN_TIMEOUT` line so it states the default
is `5000` milliseconds (and verify the `NITRO_SHUTDOWN_DISABLED` line remains
unchanged) to keep defaults consistent with bun.md and Node.js documentation.
Contributor
Author
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Linked issue
NITRO_SHUTDOWNenv variable #2376Type of change
Description
Implement env vars across runtimes
Wires documented-but-unimplemented env vars through to srvx
serve()options in the Node, Bun, and Deno server presets:NITRO_UNIX_SOCKET: Passes socket path to srvx vianode: { path }(Node.js) andbun: { unix }(Bun). Not available for Deno asDeno.servedoes not support UNIX sockets. Resolves UNIX socket support for bun preset. #1740.NITRO_SHUTDOWN_DISABLED: Maps togracefulShutdown: falsewhen set to'true'.NITRO_SHUTDOWN_TIMEOUT: Maps togracefulShutdown: { gracefulTimeout }(converted from ms to seconds for srvx).Wiring
gracefulShutdownthrough to srvx for the Bun preset should also resolve #2566 (close hook not executing with Bun preset), since srvx's graceful shutdown plugin handles SIGINT/SIGTERM and triggers proper server close.Remove env vars that cannot be configured through srvx
Two env vars were previously documented but cannot be implemented through srvx's current API, so they are removed from the docs rather than left as misleading:
NITRO_SHUTDOWN_SIGNALS-- srvx hardcodes SIGINT/SIGTERM (h3js/srvx@54e7074)NITRO_SHUTDOWN_FORCE-- srvx always callsprocess.exit(0)(h3js/srvx@54e7074)Add env var docs for Bun and Deno runtimes
The Bun and Deno runtime pages previously had no env var documentation. Now they document the variables supported by each runtime.
Fix Node.js handler preset docs
The "Handler (advanced)" section referenced preset
nodeand exportlistener, butnodeis an alias fornode-server(the full server). The actual preset isnode_middlewareand the export ismiddleware.Alignment with srvx
Verified against srvx v0.11.2 (which nitro is pinned to via
^0.11.2). The graceful shutdown plugin was refactored in h3js/srvx@54e7074:gracefulTimeoutis the only config option the implementation reads (forceTimeoutis still in the type but no longer used)process.exit(0)is always called after closeOur passthrough of
{ gracefulTimeout }aligns with both the current implementation and the type interface.Checklist