fix(logs): tag skip reasons as savings:skip(...) (#114) (#115)#119
Merged
Conversation
…115) Two issues (#114, #115) reported that pxpipe was "causing" 429 unsupported_model and 529 overloaded errors. It wasn't -- but our log output was actively misleading people into believing it was, and this change fixes that. Background: for every proxied request, both the node and worker log paths print a one-line summary: POST /v1/messages -> 429 (755ms) unsupported_model cache_read=0 The third field is a tag describing what pxpipe's compression layer did with the request. When compression ran, it reads something like "compressed 30761ch -> 4img/98060B". When compression was skipped, it printed the internal skip reason -- bare, with no prefix. The problem is where that bare reason lands: immediately after the HTTP status, exactly where a reader expects the error explanation to appear. So when Anthropic returned a 429 whose body also happened to contain "unsupported_model", the log line read as "pxpipe decided my model is unsupported and returned a 429". In reality the two strings merely coincided: pxpipe's "unsupported_model" skip reason means "this model is not in the PXPIPE_MODELS savings scope (default: Fable 5 only), so the request was passed through byte-for-byte untouched", while the 429 body was Anthropic's own upstream response, forwarded verbatim. pxpipe never generates 429s or 529s itself. The fix is to wrap the skip reason so it cannot be read as an error cause: Before: POST /v1/messages -> 429 (755ms) unsupported_model After: POST /v1/messages -> 429 (755ms) savings:skip(unsupported_model) Applied to both log call sites (src/node.ts and src/worker.ts -- the latter is what shows up in `wrangler tail`). The "compressed ..." tag for transformed requests is unchanged, and an empty reason still prints nothing. Log-only, no behavior change; all 61 existing tests pass. Fixes #114 Fixes #115
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.
Log-only change, no behavior change.
The skip reason was printed bare next to the HTTP status, so users read it as the error cause (#114, #115).
Before (looks like pxpipe rejected the model):
After (clearly just a compression-scope note):
unsupported_modelhere only means the request's model is outside thePXPIPE_MODELSsavings scope (default: Fable 5 only), so pxpipe skipped compression and passed the request through to Anthropic untouched.The 429/529 response bodies reported in the issues are verbatim Anthropic upstream errors, pxpipe does not generate them.
Applied to both log paths (
src/node.ts,src/worker.ts).Successful compressions still log
compressed 30761ch → 4img/98060Bas before.Fixes #114
Fixes #115