fix(proxy): record upstream status on failed-closed metadata lookups - #3822
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![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: 6f5aac125a
ℹ️ 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".
|
@codex review exact head |
When the API answers a proxy routing/access metadata lookup with a non-ok, non-401/403/404 response (e.g. a transient 500 from a DB blip), the proxy correctly fails closed with a public 502 "Proxy access metadata request was rejected" — but it recorded nothing about the upstream response, so every such incident was undiagnosable from the proxy's own logs. Confirmed in production on 2026-08-17: an API 500 on /projects/-/proxy-access/... matched a failed-closed proxy log that carried no upstream detail. ProxyLookupFailure now carries two structured diagnostic fields: upstreamStatus and a bounded (<=256 bytes, best-effort) upstream body snippet. They are populated in the !response.ok rejection branch and in the invalid-content-type / invalid-response branches (there the status is 200 but the snippet still identifies the payload). The handler's "Proxy metadata lookup failed closed" log statements include both fields explicitly. Observability only: public status codes and client-facing messages are byte-identical — the handler sends error.message verbatim to end users, so the upstream status deliberately stays out of the message string. The snippet read never throws; abort, stream errors, and adversarial chunking all degrade to an absent snippet, and the reader is cancelled so body-settlement guarantees are unchanged. src/proxy: 51 passed (502 steps), 0 failed. Refs VERYFRONT-STUDIO-60
7bff73f to
1eadc0a
Compare
|
@codex review exact rebased head |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
The proxy now records the upstream HTTP status when a routing or access metadata lookup fails closed. Public status codes and response messages remain unchanged.
Root cause
A non-success metadata response was converted to a public 502 or 503 without retaining the upstream status, which made transient API failures difficult to distinguish in proxy logs.
What changed
ProxyLookupFailurecarries an optionalupstreamStatus.Testing
deno test --preload=src/testing/preload.ts --no-check --allow-all src/proxy/project-metadata-client.test.ts src/proxy/handler.test.ts, 2 tests, 82 steps.--unstable-net src/proxy/, 51 tests, 502 steps.deno check, changed-file lint and format checks, andgit diff --checkpass.Risk
Low. This adds one numeric internal diagnostic field. Client-facing behavior is unchanged, and no upstream payload is logged.
Refs VERYFRONT-STUDIO-60