fix(api): guard initOpenNextCloudflareForDev against duplicate calls#1276
fix(api): guard initOpenNextCloudflareForDev against duplicate calls#1276spokodev wants to merge 2 commits into
Conversation
Calling initOpenNextCloudflareForDev twice in the same process used to spawn two competing miniflare instances and fail with an obscure workerd SQLITE_BUSY error. Track the invocation at module scope and throw a user-actionable error that points at the Next.js config file the moment the second call happens, instead of letting the failure surface deep inside the Workers runtime. The existing cross-process guard (shouldContextInitializationRun) is unchanged - the new check fires before it, so it catches the same- process case that the cross-process check is not designed to handle. Closes opennextjs#1251
🦋 Changeset detectedLatest commit: caac90a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export async function initOpenNextCloudflareForDev(options?: GetPlatformProxyOptions) { | ||
| if (initOpenNextCloudflareForDevHasBeenCalled) { | ||
| throw new Error( | ||
| `\n\nERROR: \`initOpenNextCloudflareForDev\` was called more than once in the same process.\n` + | ||
| `It should be called exactly once, at the top of your Next.js config file.\n` + | ||
| `Multiple calls start competing Workers runtimes, which fail with an obscure workerd \`SQLITE_BUSY\` error.\n` | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚩 Unhandled rejection when the guard throws in unawaited calls
The function's JSDoc at line 253 says "although async it doesn't need to be awaited". If a user accidentally writes initOpenNextCloudflareForDev() twice without await, the second call returns a rejected promise that is never caught, producing an unhandledRejection event (which crashes Node.js 15+ by default). This is arguably the desired behavior — a loud crash forces the user to notice and fix their config — but it's a change from the previous behavior where a duplicate no-op call would silently succeed. Worth confirming this is the intended DX tradeoff.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Good catch, thanks. Fixed in caac90a: the guard now lives in a synchronous wrapper that delegates the async work to an internal impl, so a duplicate call throws at the call site instead of producing an unhandled rejection when the call isn't awaited. The duplicate-call tests now assert a synchronous throw.
The guard lived inside the async function body, so a second call returned a rejected promise. Since the function is documented as not needing to be `await`ed, an unawaited duplicate call surfaced as an unhandledRejection (which crashes Node 15+) rather than a throw at the call site. Move the guard into a synchronous wrapper that delegates the async work to an internal impl, so the duplicate-call error throws at the call site regardless of awaiting. Tests updated to assert a synchronous throw.
1 participant
Closes #1251.
Calling
initOpenNextCloudflareForDevmore than once in the same Node.js process used to spawn two competing miniflare instances and fail with the obscure workerdSQLITE_BUSYerror captured in the issue. The existingshouldContextInitializationRunguard only protects across the two processes thatnext devspawns, not against repeated calls in the same one.Picking Option 1 from the issue (hard fail with a friendly message), per @vicb's comment.
This PR:
packages/cloudflare/src/api/cloudflare-context.tsthat flips totrueon the first invocation ofinitOpenNextCloudflareForDevSQLITE_BUSYsymptom so a future user with the same workerd output can search and find this guardnext devprocessVerification:
packages/cloudflare/src/api/cloudflare-context.spec.tscovers four cases: first call resolves, second call rejects with the expected message shape, error mentions both the config file andSQLITE_BUSY, and a fresh module import resets the guard so we are not accidentally relying on global state that would survive a real new processvi.stubGlobal("AsyncLocalStorage", undefined)is used to short-circuitshouldContextInitializationRunin the spec so the test isolates the duplicate-call guard from the wrangler / miniflare setupcloudflare-context.tstomainmakes 3 of the 4 new tests fail with the expected mismatch, applying the fix turns them greenpnpm code:checksclean (prettier + eslint + tsc)pnpm --filter cloudflare testsuite green: 33 files, 332 tests, 0 regressions.changeset/init-open-next-cloudflare-for-dev-duplicate-call-guard.md(@opennextjs/cloudflare: patch)