Drop the dead url fields that broke every invocation - #17
Merged
Conversation
#16 removed the baseUrl const but left the url: fields referencing it, so rawPathToScreenshotData threw ReferenceError. That runs before the S3 cache check, so every request failed - including ones that would have been served straight from cache. url was only ever read by the Chrome renderer, so it goes with it. Nothing caught this: node --check only parses, the unit tests don't touch index.js, and dev/render.js calls the renderers directly and bypasses the handler entirely. A grep for "baseUrl =" matched the declaration and not the uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018iQj4EdMfNsWP6NtFTqwUU
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 currently failing every request. Merging this immediately.
#16 removed
const baseUrlbut left theurl:fields that reference it, sorawPathToScreenshotDatathrowsReferenceError: baseUrl is not defined. That call is the first thing the handler does — before the S3 cache check — so even requests that would have been served from cache fail.urlwas only read by the Chrome renderer, so it should have gone with it in #16. My mistake.Verified by actually invoking the handler
Why nothing caught it
Worth recording, because every check I ran was blind to it:
node --checkparses, it doesn't resolve identifiersindex.jsdev/render.jscallsgenerate()directly and never goes through the handlerbaseUrl =, which matched the declaration and not the two usesThe gap is that nothing exercises the handler. A test that calls
exports.handlerwith each path shape would have caught this in a second, and would also have caught the two Dockerfile omissions if it ran in CI against the built image.