fix(container): don't read PORT - the base image sets it to 34197 (#120) - #159
Merged
Conversation
The preview service has been returning 502 on every render since the 2026-08-05 deploy. Root cause is in this repo, and it is one line. #154 made the listen port overridable so `test/shutdown.test.mjs` could bind a free port: const PORT = Number(process.env.PORT ?? 8080); But our base image is `factoriotools/factorio`, and its own image config sets `PORT=34197` - Factorio's UDP game port. Read straight out of the registry config blob for the pinned digest: ENV: PATH=... PORT=34197 RCON_PORT=27015 SAVES=/factorio/saves ... So in production that expression resolves to 34197, not 8080. The server binds the wrong port, Cloudflare's runtime waits `TIMEOUT_TO_GET_PORTS_MS` (20s) for something to answer on 8080, and gives up: Failed to start container: There has been an internal error connecting to the port Every render 502s after ~21s. Confirmed against production at 05:55, 05:56, 06:00, 06:02 and 06:05 UTC - five fresh unused seeds, five 502s. Fix: name the override `FMW_CONTAINER_PORT`. A project-prefixed variable cannot collide with whatever a base image happens to export. 8080 stays the default, matching the Dockerfile's EXPOSE and the Worker's `defaultPort` in `preview-service/worker/src/container.ts`. WHY THE ORIGINAL CHECK MISSED IT, because the lesson generalises: the local verification was run as `env -u PORT node server.mjs` - with PORT explicitly UNSET, which is the single condition under which an inherited-variable bug cannot appear. A test that clears the variable it is defending against proves nothing. `test/port.test.mjs` therefore SETS `PORT=34197` in both of its cases. The new guard is not vacuous, and that was established by planting the regression rather than by reading it: restoring `process.env.PORT` makes BOTH tests fail (pass 0, fail 2). The second case asserts the 8080 default without depending on 8080 being free - it accepts either "listening on 8080" or an EADDRINUSE naming 8080, since both prove the server targeted it. Container tests: 3/3 pass (render, shutdown, port). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01QZvNS2H4cbaZk46ybA7hbj
The static CI job formats .mjs too. The hotfix worktree had no node_modules, so the formatter could not run before the first push.
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.
Production is down, and this is the fix
The preview service has returned 502 on every render since the 2026-08-05 deploy. Confirmed against production at 05:55, 05:56, 06:00, 06:02 and 06:05 UTC - five fresh unused seeds, five 502s, each after ~21s.
Root cause: one line, and it is ours
#154 made the listen port overridable so
test/shutdown.test.mjscould bind a free port:But the base image is
factoriotools/factorio, and its own image config setsPORT=34197- Factorio's UDP game port. Read straight out of the registry config blob for the pinned digest:So in production that expression resolves to 34197, not 8080. The server binds the wrong port, Cloudflare's runtime waits
TIMEOUT_TO_GET_PORTS_MS(20s) for something to answer on 8080, then gives up:Billing corroborates it: placements are born and die within seconds (10 GiB-s in the 05:57 minute, 50 in 06:00).
The fix
Name the override
FMW_CONTAINER_PORT. A project-prefixed variable cannot collide with whatever a base image happens to export. 8080 stays the default, matching the Dockerfile'sEXPOSEand the Worker'sdefaultPortinpreview-service/worker/src/container.ts.Why the original check missed it
The local verification was run as
env -u PORT node server.mjs- withPORTexplicitly unset, which is the single condition under which an inherited-variable bug cannot appear. A test that clears the variable it is defending against proves nothing.test/port.test.mjstherefore setsPORT=34197in both cases.The guard is not vacuous
Established by planting the regression, not by reading it: restoring
process.env.PORTmakes both new tests fail (pass 0, fail 2).The second case asserts the 8080 default without depending on 8080 being free on a dev machine - it accepts either
listening on 8080or anEADDRINUSEnaming 8080, since both prove the server targeted it.Testing
Container tests 3/3 pass (render, shutdown, port). Full
verifyruns in CI here.After merge
This needs
pnpm run preview:deployto reach production - the deploy is what is currently broken, so merging alone does not restore service.Then #120's actual measurement can finally run: one 200 render, quiet, wait ~15 min, walk the buckets. Pass = a
placementIdappearing for a bucket or two then stopping - never "the newest bucket is empty", which is the backfill artifact that manufactured the phantom 8.5-minute tail.🤖 Generated with Claude Code
https://claude.ai/code/session_01QZvNS2H4cbaZk46ybA7hbj