🦋 Changeset detected

Latest commit: 573eafc

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

✅ All changesets look good

UnknownError: ProviderInitError

github run

@dario-piotrowicz Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

npm i https://pkg.pr.new/create-cloudflare@14081

npm i https://pkg.pr.new/@cloudflare/deploy-helpers@14081

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@14081

npm i https://pkg.pr.new/miniflare@14081

npm i https://pkg.pr.new/@cloudflare/pages-shared@14081

npm i https://pkg.pr.new/@cloudflare/unenv-preset@14081

npm i https://pkg.pr.new/@cloudflare/vite-plugin@14081

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@14081

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@14081

npm i https://pkg.pr.new/@cloudflare/workers-utils@14081

npm i https://pkg.pr.new/wrangler@14081

commit: 573eafc

Now I have a thorough understanding of the changes. Let me analyze:

The core change (in packages/miniflare/src/runtime/index.ts):

1. Adds a Promise.race between waitForPorts() and the process exit promise, so if workerd exits before writing all listen events, Miniflare detects it promptly instead of hanging.
2. Adds a new fallback in handleStartupFailure(): if stderr has content but doesn't match the specific "Address already in use" pattern, throw a generic ERR_RUNTIME_FAILURE with the stderr contents.

Let me think about potential issues:

1. Race condition with stderr buffering: When processExitPromise resolves (process exits), ports will be undefined. Then handleStartupFailure() is called. But there's a question: has all stderr output been buffered by this point? The exit event fires after the process terminates, but Node.js stdio streams can have buffered data that hasn't been consumed yet. However, looking at the code, the stderr is piped through the startupLogBuffer.stderrStream Transform, and handleRuntimeStdio is called to set up consumers. The stderrBuffer accumulates chunks as they pass through the Transform stream. When the process exits, the stdio streams should have already delivered their data since the child process flushes on exit. This is generally safe, though in edge cases with very fast exits, there could be a timing issue. But in practice this should be fine.

2. The waitForPorts function and control pipe close: When workerd exits, the control pipe (FD3) should also close, which would cause the for await loop to end. So in theory, waitForPorts should also resolve (returning undefined since requiredSockets.length > 0). The Promise.race is belt-and-suspenders, but the comment in the code specifically says waitForPorts() would "hang indefinitely." This is actually true in some scenarios — if the child process is spawned with stdio: ["pipe", "pipe", "pipe", "pipe"] and the FD3 stream is a duplex or there's a Node.js behavior where the readable end doesn't emit close/end synchronously with process exit. The Promise.race is a safe and pragmatic fix.

3. Test quality: The test correctly uses a fake workerd that exits with stderr output, validates the error type and message. It properly cleans up with onTestFinished. Good.

4. One subtle issue: Looking at waitForPorts, when processExitPromise wins the race, the waitForPorts function is still running (the for await loop is still active). The readline interface on the control pipe is never explicitly closed by anything in this path. However, this should be fine because dispose() is called in cleanup, which destroys all stdio streams including the control pipe, which will cause the readline interface to end.

5. No unhandled promise rejection: The losing promise in the Promise.race (the waitForPorts call) could theoretically throw if the controlPipe emits an error after the race is won by processExitPromise. But the controlPipe destruction happens in dispose(), and by that point the updateConfig method has already returned. The waitForPorts promise would be a floating promise in this scenario. However, looking at the code, waitForPorts uses for await on a readline interface, which shouldn't throw on stream destruction — it would just end iteration. So this should be safe.

The changes look correct, well-tested, and solve a real problem. The approach is sound.

LGTM

github run

Codeowners approval required for this PR:

• @cloudflare/wrangler