fix(worker): externalize dotenv and @vercel/blob to fix boot crash - #416
Merged
Conversation
The worker container failed to boot in CI: "Dynamic require of 'fs' is not
supported" from an inlined dotenv, then the identical crash one dependency
further down once that was fixed — @vercel/blob (added for the Vercel Blob
artifact store) pulls in @vercel/oidc -> jose's CJS build, which does the
same dynamic require("buffer"). tsup.config.ts already documents this exact
failure mode for @prisma/client and @sentry/node; dotenv and @vercel/blob
just hadn't been added to the same external list yet. The Dockerfile's
`pnpm install --filter @ghost/worker...` puts all of these in node_modules
regardless, so externalizing costs nothing at runtime.
Verified by building and booting the worker locally with the CI smoke
test's exact env vars — "Ghost worker started" now logs instead of
crashing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…cond one The capture socket was unreachable on a real deployment, and the deploy docs were the reason. They told you to run the worker as the host's *background worker* type — correct advice when the worker was only a queue consumer, and now the one thing that makes recording impossible. A Render background worker (and Fly's and Railway's equivalents) is outbound-only: no hostname, no port, nothing can connect to it. Recording needs the user's browser to open a WebSocket to the process holding the browser. Two changes make the existing service enough, so nobody has to pay for a second one: - The port comes from `PORT` when `GHOST_CAPTURE_PORT` is unset. That is the variable every one of these hosts injects, so the worker needs no extra configuration to be reachable. - The HTTP server binds unconditionally and answers `/health` and `/`, even with capture switched off. Binding only when the feature flag was set would mean clearing that flag fails the deploy — a host decides a web service is broken when no port opens — and takes replay down with the worker. `/` is answered as well as `/health` because a default health check probes the root, and a worker that 404s there is restarted on a loop, which reads as a crashing process rather than a missing setting. The capture endpoint itself is unchanged in spirit: `/capture` is mounted only when `GHOST_CAPTURE_KEY` is set, so with no key the upgrade is refused by the HTTP server before any of this code runs. Serving health and accepting captures are now separate decisions, which is what they always were. DEPLOY.md now says to pick the web service type and why, including that most hosts cannot change a service's type after creation — so this is the one deployment choice worth getting right first. It costs nothing: these hosts price by instance size, and the two service types of the same size cost the same. Validation: pnpm typecheck (5/5, forced), pnpm test (461 passing: core 224, web 116, worker 120, mcp 1), pnpm build (4/4). New tests cover the fallback to an injected PORT, and that a keyless worker still serves health while refusing a connection to /capture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
867cee8 marked both external in tsup.config.ts, which worked in a local dev tree (root node_modules already had them hoisted from other packages) but not in the Docker image: the deps stage runs `pnpm install --frozen-lockfile --filter @ghost/worker...`, and neither package was declared as a dependency of @ghost/worker itself (only transitively, via @ghost/core) — the filtered install had no reason to place them where the externalized bare import could resolve them, so the container crashed with ERR_MODULE_NOT_FOUND instead of the original "Dynamic require" error. @prisma/client and @sentry/node already work as externals precisely because they *are* listed directly, per tsup.config.ts's existing comment; this makes dotenv and @vercel/blob consistent with that. Verified against the real artifact, not a substitute: built the actual worker Docker image (`docker build -f apps/worker/Dockerfile`) and booted it with the CI smoke test's exact env vars — logs "Ghost worker started" instead of crashing. Co-Authored-By: Claude Sonnet 5 <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
1 participant
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.
Summary
PR #413 merged with a worker container boot crash still in it — the
buildjob's smoke test was still failing/pending when the merge wentthrough. Master currently cannot boot the worker container:
dotenv(pulled in transitively via@ghost/core/env, which isnoExternalin the worker's tsup config) and then@vercel/blob(via@vercel/oidc->jose's CJS build) both get bundled into the ESM outputand hit the same "Dynamic require" crash
tsup.config.tsalreadydocuments for
@prisma/client/@sentry/node. Adds both to the sameexternallist — the Dockerfile'spnpm install --filter @ghost/worker...already puts them innode_modules, so this costsnothing at runtime.
Test plan
pnpm buildinapps/workersucceedsdist/index.jslocally with the CI smoke test's exact envvars — logs
"Ghost worker started"instead of crashingbuildjob (worker container smoke test) passes