{{ message }}
fix: log expected request/response conditions at debug, not warn#5474
Open
artiom wants to merge 1 commit into
Open
fix: log expected request/response conditions at debug, not warn#5474artiom wants to merge 1 commit into
artiom wants to merge 1 commit into
Conversation
WalkthroughThe page response event handler now logs non-OK sub-resource responses at debug level instead of warn, and adds comments noting that these page-load responses are expected. ChangesPage response logging
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
7c9ed99 to
e471deb
Compare
Two warn-level logs fire on expected, non-actionable conditions and
produce a large volume of low-signal warnings:
1. /function page.on('response') logged every non-OK response at warn —
favicon 404s, redirects, and other routine sub-resource misses during
a normal page load (inconsistent with the trace-level page.on handlers
beside it). Now logged by status: server errors (5xx) and blocking/
rate-limit responses (403, 429) stay at warn — those are actionable
(target down or bot-detection) — while routine misses drop to debug.
The status code is now included in the message.
2. HTTP body validation failures (server.ts) are client errors: the
handler already returns 400 to the caller with the details, so the
server-side log is not operator-actionable. Lowered to debug.
Genuine errors/warnings (incl. the unexpected-disconnect warn in
browsers.cdp.ts) are unaffected.
e471deb to
0fffe31
Compare
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.

Why
Two
warn-level logs fire on expected, non-actionable conditions and produce a large volume of low-signal warnings.1.
/functionresponse handler —src/shared/utils/function/handler.tspage.on('response')logged awarnfor every non-OK response — favicon 404s, redirects, and other routine sub-resource misses during a normal page load. These are expected and not actionable, and inconsistent with the siblingpage.on(...)handlers (which log attrace).Now logged by status:
warndebugServer errors and blocking/rate-limit responses are the actionable cases (target down, bot-detection) and stay visible; routine misses drop to
debug. The status code is now included in the message.2. HTTP body validation failures —
src/server.tsA request body that fails schema validation is a client error — the handler already returns
400to the caller with the details (writeResponse(res, 400, …)). Logging it server-side atwarnis noise an operator can't act on, so it's lowered todebug.Not changed
The
"… disconnected unexpectedly"warn inbrowsers.cdp.tsstays atwarn— it's deliberate crash detection (Chrome OOM/segfault/SIGKILL), a genuine signal.