Repository navigation
fix: restore Cloudflare Worker routes after the v3.0.0 startup crash - #248
Conversation
Fastify compiles routes with new Function(), which workerd only allows while the isolate is starting. v3.0.0 moved listen() into the first fetch so it could read MONGODB_URI, and every route then crashed with EvalError (Cloudflare 1101). Read the URI from cloudflare:workers at module load, and construct the Mongo client on the first bin operation because @keyv/mongo connects from its constructor. Bin capture URLs followed the plain Node socket, so an https edge request produced an http:// URL. Stamp the Worker request scheme and use it when building the public bin URL. Co-authored-by: Jared Wray <me@jaredwray.com>
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. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #248 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 1404 1436 +32
Branches 287 300 +13
=========================================
+ Hits 1404 1436 +32 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![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: 6405a3c7fa
ℹ️ 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".
| }); | ||
|
|
||
| it("builds a capture URL from the forwarded protocol chain", () => { | ||
| expect(binCaptureUrl("http, https", "http", "mockhttp.org", "abc")).toBe( |
There was a problem hiding this comment.
Remove this direct helper test. Cover its cases through POST /bins. No production caller uses the binCaptureUrl export.
AGENTS.md reference: AGENTS.md:L66-L73
Useful? React with 👍 / 👎.
| createWorkerApp, | ||
| handleWorkerFetch, | ||
| listenWorkerApp, | ||
| requestWithPublicProtocol, |
There was a problem hiding this comment.
Remove these direct helper tests. Cover these cases through the Worker fetch boundary. No production caller uses the requestWithPublicProtocol export.
AGENTS.md reference: AGENTS.md:L66-L73
Useful? React with 👍 / 👎.
| await listening; | ||
| } | ||
| // Fastify compiles routes with `new Function()`. workerd allows that only | ||
| // while the isolate is starting, so the server has to listen at module load. |
There was a problem hiding this comment.
Replace is starting with a simple-present phrase. The repository prohibits this verb form in new comments.
AGENTS.md reference: AGENTS.md:L51-L52
Useful? React with 👍 / 👎.
| const raw = Array.isArray(forwardedProto) | ||
| ? forwardedProto.join(",") | ||
| : forwardedProto; | ||
| const forwarded = raw?.split(",").at(-1)?.trim().toLowerCase(); |
There was a problem hiding this comment.
When an HTTPS edge appends an internal HTTP hop as https, http, .at(-1) selects http. POST /bins then returns the wrong public scheme. Read the first client protocol entry instead.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| const headers = new Headers(request.headers); | ||
| headers.set(PUBLIC_PROTOCOL_HEADER, protocol); |
There was a problem hiding this comment.
For each Worker call, this marker enters Fastify's request.headers; headersRoute and the HTTP method routes return that map. Their responses report a header that the client did not send. Carry the scheme outside the echoed header map.
Useful? React with 👍 / 👎.
| import { type Unstable_DevWorker, unstable_dev } from "wrangler"; | ||
|
|
||
| // wrangler's HTTPS dev listener uses a self-signed certificate. | ||
| process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"; |
There was a problem hiding this comment.
If Vitest runs another file in this process, this module-level assignment leaves TLS verification off after this suite ends. That file can accept an invalid certificate. Restore the prior value in afterAll.
Useful? React with 👍 / 👎.
| }); | ||
| const body = await created.text(); | ||
| expect(created.status, body).toBe(500); | ||
| expect(body).not.toContain("Code generation from strings"); |
There was a problem hiding this comment.
Replace this assertion. MockHttpBins.save() wraps every adapter failure with a fixed message, so Fastify does not put the cause in this response. This check also passes for the prohibited EvalError. Assert the error at a boundary that exposes it.
AGENTS.md reference: AGENTS.md:L75-L77
Useful? React with 👍 / 👎.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce?
Bug fix. After v3.0.0 (deploy-site run 39346805553, commit 266157b, Worker version cdaa6cb9) every Worker route on mockhttp.org returned Cloudflare error 1101.
Root cause
PR #247 moved
listenWorkerApp(await createWorkerApp(env))from module load into the firstfetch, so it could readenv.MONGODB_URI. Fastify compiles routes withnew Function()(HandlerStorage._compileCreateParamsObject→Router.all→_setNotFoundHandler). workerd allows that only while the isolate is starting. On the first request it throws:Uncaught EvalError: Code generation from strings disallowed for this contextThat uncaught exception is Cloudflare 1101. Static assets never enter the Worker, so they kept loading.
Fix
import { env } from "cloudflare:workers"suppliesMONGODB_URIduring startup.@keyv/mongountil the first bin operation. Its constructor callsMongoClient.connect(), and workerd rejects that network I/O outside a request. With no URI, bins stay in memory as before.request.protocol/x-forwarded-proto. The Node server inside the Worker is plain HTTP, and the bridge rewritesx-forwarded-prototo that socket. The Worker stampsx-mockhttp-forwarded-protofrom the request URL, andPOST /binsuses that for the public capture URL.http vs https
http://in the local experiment was the wrangler dev listener, not a wrong host. Defaultwrangler devis HTTP, the Worker request URL ishttp://mockhttp.org/..., and the bin URL correctly follows that (http://mockhttp.org/b/<id>, nothttp://127.0.0.1:3000).An HTTPS edge is different. With wrangler's HTTPS dev listener, and without this stamp,
POST /binsstill returnedhttp://mockhttp.org/b/<id>because Fastify only saw the plain Node socket. With the stamp, the same request returnshttps://mockhttp.org/b/<id>. Production Workers receivehttps://mockhttp.org/...asrequest.url, so hosted bin URLs are https.Verification
Reproduced the 1101 class of failure under wrangler 4.127.0 / workerd before the change:
GET /getreturned 500 with theEvalErrorabove.After the change,
pnpm testpassed: 45 files, 536 tests, 100% lines. The new workerd test (test/worker-runtime.test.ts) covers/get,/ip,GET /bins,POST /bins, andPOST /b/<id>.Atlas itself was not exercised. This environment has no
MONGODB_URIand no MongoDB server. Under workerd,MONGODB_URI=mongodb://127.0.0.1:1/mockhttp(nothing listening) still servesGET /getwith 200, andPOST /binsreturns 500 from the failed connect rather thanEvalError. That shows the client stays closed during startup and connects on the bin write. A successful Atlas round trip still needs a real cluster.