Repository navigation
Start integration servers again when another process takes their port - #396
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://fix-dev-server-port-race-void-cloud.voidzero-docs.workers.dev (commit b40f70a)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34196c3fbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Integration tests chose a free port, released it, then spawned a dev or preview server that binds it seconds later. Another process could bind the port in that gap. Vite then moved to the next port, which is often the port another test was just given, and the test kept polling the old port until it timed out after 45 s. startServer() now owns the whole start: it picks a port, spawns the server, and waits for it. Vite servers get --strictPort, so a taken port makes them exit instead of moving into another test's port. When a server reports that its port is taken, startServer() starts it again on a new port, up to three times. Servers that cannot be strict (Astro) are restarted when they report the move. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Another HTTP server that took the chosen port could answer before the started server bound it. startServer then returned at once, and the test ran against that other server while ours exited with "already in use". Answers now count only after the server prints its URL with the chosen port (Vite and Astro "Local: http://127.0.0.1:<port>/", the Node, Bun and Deno servers "Listening on http://...:<port>"). A server that exits on the taken port is still started again on a new one. Nuxt prints no URL under Vitest, so its test opts out and keeps relying on its page body check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
fix/dev-server-port-race
branch
from
October 8, 2026 13:46
b46dafc to
b40f70a
Compare
Brooooooklyn
added a commit
that referenced
this pull request
Oct 8, 2026
## Problem A hosted app (napi-rs) returned about 100 worker 500s in one day. The 5xx alert email pointed to `void project requests` and `void project logs`. Neither command showed the cause: ``` $ void project requests --status 5xx 16:30:14 GET 500 static 1897ms <- the worker answered, but the label says "static" $ void project logs --level error 16:30:12 GET https://napi.rs/sponsors.svg 500 [error] TypeError: Cannot read properties of undefined (reading 'href') <- no stack, although the API returns it 13:00:25 POST https://internal/__void/scheduled 500 [error] AggregateError: Scheduled jobs failed <- which job failed, and why, is lost ``` Three causes: ``` request label dispatch: a path with a file extension (/sponsors.svg) stays "static", even when the app's own route answered it logs stack CLI: prints only `Name: message`; drops errorInfo[].stack and exception.stack cron error runtime: Workers log capture keeps an error's own message and stack, but not AggregateError.errors, so the inner errors vanish ``` ## What this PR changes ``` request label (dispatch metering) dotted path on an SSR deploy, no platform failure ├─ manifest says "not a file" and the worker answers the path ─> ssr ├─ worker owns the path and returns 5xx or times out ─────────> ssr └─ listed file, asset-layer answer, platform asset failure ───> static (unchanged) The ISR write-back checks the path directly, so these routes never enter the ISR cache. void project logs [error] TypeError: Cannot read properties of undefined (reading 'href') at assets/resvg-gkFxLqVH.js:14881:64 <- dimmed frames under the at assets/resvg-gkFxLqVH.js:15409:3 argument that logged them [exception] frames print the same way scheduled jobs job fails ─> console.error('[void] Scheduled job crons/x.ts failed for cron "0 5 * * *":', error) (logged at once, with the original error, so its stack is kept) all jobs done ─> throw AggregateError(errors, 'Scheduled jobs failed: crons/x.ts') (HTTP 500 and the shared onError are unchanged) ``` Real output from this branch's CLI against the live napi-rs data: ``` 15:23:32 GET https://napi.rs/sponsors.svg 500 [error] TypeError: Cannot read properties of undefined (reading 'href') at assets/resvg-gkFxLqVH.js:14881:64 at assets/resvg-gkFxLqVH.js:15409:3 at new Promise (<anonymous>) at assets/resvg-gkFxLqVH.js:15408:19 at assets/client-7iHZB5sW.js:16:49 ``` ## Decisions | Question | Decision | Why | |---|---|---| | How does dispatch know the worker answered? | Reuse the asset-manifest answer it already reads for the static cache | No new KV read on the hot path | | New request type such as `worker`? | No, use `ssr` | The CLI, docs and usage queries already know `ssr` | | Fix cron errors in the shared Hono `onError`? | No, in the scheduled runner | `onError` serves every route; the runner knows the job file and cron | | Change the platform API or tail worker for stacks? | No, CLI only | The API already returns `errorInfo[].stack` and `exception.stack` | | `void platform deployment logs` (operator command) | Not changed | Outside the reported problem | ## Rollout ``` merge ─> release void ──> users upgrade the CLI ──────> stacks print (no redeploy needed) └──> apps redeploy ──────────────> cron errors named deploy hosted dispatch ─────────────────────────> new rows labeled ssr (old Analytics rows keep "static") ``` ## Known limits - Some app routes on paths with a file extension can still be labeled `static` when they succeed or return 4xx. This applies to requests with a Cookie or Authorization header, to HEAD, Range and POST requests, and to SSR apps that uploaded no static files. On framework deploys, these requests also stay `static` when they fail with a 5xx. A correct label would need an asset-manifest read on every credentialed asset request. That is too much cost for a display label. On Void SSR apps, server errors are labeled `ssr` for every request shape. - A failed cron now shows two log lines on the hosted path: the per-job line and the `AggregateError` line. - Framework cron runners and queue runners have the same error-loss pattern. They are not changed here. ## Testing | Check | Result | |---|---| | New tests on old code | fail (dispatch 7+, logs rendering 1, scheduled dispatch 2) | | Changed test files | 5 files, 353 tests pass | | `vp run test --base origin/main` | 475 files, 10329 tests pass | | `vp lint`, `vp run check` | pass (31 typecheck tasks) | | `vp test run` | 689 files pass. 1 file failed on a port race in the Solid dev-server test: the test waited on a port that `vp dev` found busy. It passes on rerun. The race is fixed in #396. | | Branch CLI against live napi-rs logs | frames print under every error | | `vp dev` fixture with one failing cron job | per-job line with file, cron, message and stack; the other job runs; HTTP 500 | 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
solid-pages-playground.test.ts > solid-pages client (dev)failed once in a full local run and passed on rerun:Every dev and preview server test starts the same way. The test picks a free port, frees it, and starts the server. The server binds the port 1–7 s later. Anything can take the port in that gap.
Two things make it worse:
port + 1, it lands on another test's port. That test then fails too.--strictPortnever stops. It moves silently, and the test only finds out after the full timeout.Who took 52761
The process is gone, so it cannot be named for that run. What I checked:
listen(0)?listen()in 6 full runs (+2 overlapping pairs): only each test server on its own port (and inspector ports near 9229). No collision happened in those runs.--socket-addr=entry=127.0.0.1:55332).listen(0)get the port.So the port was taken by a process that binds a port number it chose itself. On Linux CI, another test's
listen(0)can also take it. The test design turns any of these into a 45 s timeout. The fix does not depend on which process it was.What this PR changes
startServer().--strictPort, so they never move into another test's port.getPort()and the port hold are gone. The hold only covered the time until spawn, not the seconds until the server binds.startServer()with--strictPort.Decisions
printsUrl: falseNODE_ENV=test). Its test checks the page bodyNuxt + Voidinstead.astro devhas no--strictPort. The message is enough to restart it.Known limits
127.0.0.1on the same port. They print their URL, so there is nothing to retry. Requests to127.0.0.1then reach the other process.Testing
mainwith a process binding its port (and port + 1) right after spawnPort N is in use×2, 45 s timeoutAnother process took port N; starting the server on a new port.vp dev, ssr playground)Port N is already in use. With it: retries, returns our page, server still runningvp test run packages/void/test/unit/integration-helpers.test.tsvp test run packages/solid/test/integration/solid-pages-playground.test.ts×3startServer()×2CI=truevp test run --project parallel-integration×2main: 27.6–27.9 s)vp run test --base origin/main(selects all 691 files)vp test runvp lint,vp fmt --check,vp run check🤖 Generated with Claude Code