fix(cli): probe the dev port through the native Deno runtime - #3598
fix(cli): probe the dev port through the native Deno runtime#3598kojiwakayama wants to merge 1 commit into
Conversation
Under a Deno-installed CLI, `veryfront dev` died before binding with an unclassified `Cannot read properties of null (reading 'fd')`, right after "Using local filesystem (no proxy mode)". `build`, `serve`, `routes` and `doctor` were fine; the npm-global install of the same version was fine. The port scan added in #3562 probes with a bare `Deno` listen. In the npm build dnt rewrites every bare `Deno` member access to `@deno/shim-deno`, whose TCP listen reads `server._handle.fd` immediately after `net.createServer().listen()` - and Deno's own `node:net` compat has not populated `_handle` by then. Node never reached it, because the shim is only in play when the published package runs under Deno. Resolve the namespace through `getDenoRuntime()`, which reads the global with `Reflect.get` and so survives the dnt rewrite, and guard the source against the bare access coming back.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe port fallback now obtains the Deno runtime through ChangesDeno runtime compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
CI: the three red checks are pre-existing on
|
Carries over the static invariant test from the duplicate PR #3598 alongside the behavioural one already here. They catch different things: - the behavioural test poisons the ambient `Deno.listen` and asserts the probe still answers correctly, proving the fix works when the shim is in place; - this one reads the source and asserts no bare `Deno.<member>` access survives anywhere in the file, catching a future reintroduction on paths the behavioural test never executes. Verified red both ways before landing: against the pre-fix source both tests fail, and with a bare `Deno.hostname()` planted in `findAvailablePort`'s throw path the behavioural test passes while this one fails naming `Deno.hostname`. Comments are stripped before matching, since dnt rewrites code and not prose, and `isPortAvailable`'s doc comment has to stay free to name `Deno.listen` as the call the fix removed.
|
Closing as a duplicate of #3599 — same bug, found independently, and this PR's analysis was right. The diagnosis here stands on its own: dnt rewrites every bare The invariant test from this PR has been carried over to #3599 — see eebdc22. It was not redundant with the test already there, and that is why it was worth porting:
That complementarity was verified, not assumed. With a bare One adaptation was needed. #3599 fixes the bug by deleting the Deno branch entirely and probing with The Branch |
Symptom
Under a Deno-installed CLI,
veryfront devdies before it binds anything:Reproduced against the published
veryfront@0.1.1229in a sandbox outside thisrepo, on a project scaffolded by that same published CLI
(
veryfront init deno-app --template ai-agent --runtime deno --yes). It is specificto
devunder Deno —build,serve,routesanddoctorall succeed under thesame Deno-global install, and
node ./node_modules/veryfront/bin/veryfront.js devon the identical project serves HTTP 200.
This blocks
--runtime deno, which/docs/code/getting-started/installationadvertises as a supported install path.Root cause
The finding said the
.fdaccess was not in the framework's ownesm/srcbut in adependency only reachable on the Deno dev path. It is
@deno/shim-deno. Raw stack,recovered by instrumenting the CLI error boundary in the published tarball:
cli/commands/dev/port-fallback.tsprobed the port with a bareDenolisten call.In the npm build dnt rewrites every bare
Denomember access todntShim.Deno(
@deno/shim-deno) — the publishedport-fallback.jsline 38 really readsdntShim.Deno.listen(...). That shim implements listen asUnder Node
_handleis populated synchronously; under Deno'snode:netcompat it isstill
nullat that point, so the probe throws. The port scan runs unconditionally onevery
devstart, sodevwas dead on arrival for every Deno user.Fix
Resolve the namespace through
getDenoRuntime(), the existing platform helper thatreads the global with
Reflect.get(globalThis, "Deno")— a form dnt does not rewrite,which is exactly why
src/platform/compat/http/native-response.tsalready uses thesame trick for
Deno.serve/Deno.upgradeWebSocket. One-line behaviour change; theNode
node:netfallback is untouched.Test
cli/commands/dev/port-fallback.test.tsgains a guard that fails if the module reachesthe runtime through the binding dnt rewrites. It fails on the pre-fix source with
dnt would rewrite Deno.listen to the broken @deno/shim-deno namespaceand passesafter. A plain unit test cannot catch this: in a Deno test run the bare
Denois thenative namespace, so the defect only exists in the built artifact.
Verified against the published repro
--runtime denoscaffold, the finding's owncommand
deno task dev:Cannot read properties of null (reading 'fd'),curl→ 000.deno task build:npmon this branch →npm pack→ installed the tarball into aclean tree outside this repo →
deno run -A .../veryfront/bin/veryfront.js devin the same unmodified scaffold:
✓ Ready in 576ms,curl→ 500, andgrep -c "reading 'fd'"over the whole dev log → 0.Not fixed here — a second, separate Deno-only defect
That 500 is not this bug. With the crash gone, the dev server binds and serves, but SSR
of the first page fails under Deno with
The bundle file does exist on disk (380 bytes, and so do its three transitive
http-*.mjsimports); Deno still rejects the dynamic import as an unprepared module, andrecoverHttpBundleByHashreports failure because there was nothing to recover. It isdeterministic across restarts and warm caches, and does not occur under Node on the same
project. That belongs to
src/modules/react-loader/ssr-module-loader+src/transforms/esm/http-cache.ts, not to the port probe, and is filed separately ratherthan smuggled into this PR.
Summary by CodeRabbit
Bug Fixes
Documentation
Tests