Repository navigation
fix: skip ASSETS binding fetch for non-GET/HEAD requests - #54
Merged
Merged
Conversation
POST/PUT/PATCH/DELETE requests to unmatched paths (e.g. bot traffic to /xmlrpc.php) were passed to env.ASSETS.fetch(), which internally reconstructs the Request in workerd C++. If any middleware had consumed the request body, this threw "Cannot reconstruct a Request with a used body" and returned 500. Static assets are only served via GET/HEAD — the ASSETS worker returns 405 for other methods anyway. Guard all three ASSETS fetch call sites: - serveWithAssets: entry-point fallback for non-pages apps - fetchPagesAsset: pages-mode asset fallback for extension paths - notFound handler: skip page rendering and asset lookup entirely for non-GET/HEAD requests to non-API paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fengmk2
approved these changes
Mar 31, 2026
![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: 811def6bed
ℹ️ 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".
…n tests getPort() found a free port by binding to port 0, then immediately closed the socket. When ~16 integration tests ran in parallel, another test's getPort() could grab the same port during the gap between close and the child process's listen(), causing "address in use" failures. Hold the socket open until waitForServer() is called (right after spawn()), so no other test can steal the port. Also bump the node-target production server timeout from 10s to 20s to match other integration tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Brooooooklyn
force-pushed
the
fix/skip-assets-fetch-for-non-get
branch
from
March 31, 2026 08:43
811def6 to
7a9dba8
Compare
cpojer
pushed a commit
that referenced
this pull request
Sep 11, 2026
…#54) When all assets are already cached, CF returns empty buckets and the session JWT is already the completion token. The previous implementation incorrectly tried to "exchange" the upload token by POSTing to the upload endpoint with CF_API_TOKEN, which is not how the CF API works. This commit rewrites uploadAssetBuckets to match Wrangler's implementation: - Return session JWT directly when buckets are empty (no exchange needed) - Use buckets.flat().length === 0 to handle both [] and [[]] from CF - Add per-bucket retry with exponential backoff (5 attempts) - Remove apiToken parameter (only used in broken exchange logic) - Fix Object.keys(assetsByHash) on Map (always returned 0) Co-authored-by: Claude Opus 4.6 (1M context) <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
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.
POST/PUT/PATCH/DELETE requests to unmatched paths (e.g. bot traffic to
/xmlrpc.php) were passed to env.ASSETS.fetch(), which internally
reconstructs the Request in workerd C++. If any middleware had consumed
the request body, this threw "Cannot reconstruct a Request with a used
body" and returned 500.
Static assets are only served via GET/HEAD — the ASSETS worker returns
405 for other methods anyway. Guard all three ASSETS fetch call sites:
non-GET/HEAD requests to non-API paths
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Note
Low Risk
Small, targeted guards around static-asset fallthrough and test harness changes; main risk is unexpected 404 behavior for non-GET/HEAD requests that previously reached
ASSETS.fetch().Overview
Stops routing fallbacks from hitting the
ASSETSbinding for non-GET/HEADrequests by adding method guards in pages-mode (fetchPagesAsset()and theapp.notFoundhandler) and in the worker entry fallback helperserveWithAssets().Improves integration test stability by holding ports allocated by
getPort()untilwaitForServer()runs (reducing parallel-test TOCTOU port races) and increases the Node production server startup wait innode-target.test.ts.Written by Cursor Bugbot for commit 7a9dba8. This will update automatically on new commits. Configure here.