Skip to content

fix: log non-2xx upstream responses (status + request-id + body snippet) - #174

Merged
ranxianglei merged 1 commit into
masterfrom
2026-08-19_log-upstream-errors
Aug 21, 2026
Merged

fix: log non-2xx upstream responses (status + request-id + body snippet)#174
ranxianglei merged 1 commit into
masterfrom
2026-08-19_log-upstream-errors

Conversation

@ranxianglei

Copy link
Copy Markdown
Owner

Field report in #2: a relay answered 400 (请求参数无效) for 34 minutes; bili.log carried zero trace because non-2xx upstream responses were piped through verbatim without logging. Diagnosis had to be reconstructed by timestamp alignment against the relay request-id.

This PR makes the proxy log a warn on every non-2xx upstream response:

[sid] ← upstream 400 request-id=2026…MaylrIZ3: {"error":{"message":"请求参数无效…"}}
  • status + x-request-id/request-id header (when present) + first 600 chars of the error body (64KB buffered; larger bodies marked with trailing …)
  • body still passes through to the client byte-for-byte (chunked / oversized / empty bodies all handled)
  • adds setLogCapture() test hook in src/logger.ts (capture runs before file/stderr writes, cannot crash logging)
  • tests: verbatim passthrough + warn content (fixed-length, chunked, oversized)

Pre-flight: typecheck ✓, 456/456 tests ✓, build ✓. cc issue #2

@ranxianglei
ranxianglei force-pushed the 2026-08-19_log-upstream-errors branch from 0dc582a to 7772573 Compare August 20, 2026 01:09
@ranxianglei

Copy link
Copy Markdown
Owner Author

LGTM — this closes exactly the observability gap from #2: the relay now logs status + request-id + a capped body snippet on the non-2xx path before res.writeHead, and the test verifies all three failure shapes (400 + x-request-id passthrough, chunked 500, oversized 429 capped at 600 chars with the marker). All 3 tests pass.

Two non-blocking notes:

  • The snippet can echo parts of the request body on some upstreams (gateways that reflect the payload in their error). It goes only to the local bili.log and is capped at 600 chars, so I'd accept it — just be aware if you ever make the log shareable.
  • setLogCapture is a test-only global on a shipped module. Fine here (the hook is try/catch-wrapped so a broken sink can't crash logging, and the tests reset it in finally), just don't let it grow into a general side-channel.

@ranxianglei
ranxianglei merged commit 81cb655 into master Aug 21, 2026
5 checks passed
@ranxianglei ranxianglei mentioned this pull request Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant