Skip to content

logging: classify generic http 5xx errors as transient - #812

Merged
ben-dz merged 2 commits into
mainfrom
bdz/dberror-generic-5xx-transient
Aug 31, 2026
Merged

logging: classify generic http 5xx errors as transient#812
ben-dz merged 2 commits into
mainfrom
bdz/dberror-generic-5xx-transient

Conversation

@ben-dz

@ben-dz ben-dz commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

  • dberror.Classify now treats generic HTTP 5xx error strings (status code 500|502|503|504, also statuscode:/status_code= variants) as transient connectivity. The first status-code mention decides — wrapping prepends, so that is the failed request's own status, and a 4xx whose body quotes a 5xx stays actionable. The AWS-shaped regex shrinks to its 200-with-embedded-error case, the only shape the generic pattern can't cover.
  • Why: during the validators.app outage on 2026-08-30, RefreshValidatorsApp 503s classified as Unknown, so the solingest Temporal logger could not demote per-attempt "Activity error." lines and every retry paged (3 ERR lines per 5-min cycle). An S3 503 was already silenced by the AWS-scoped regex; a validators.app 503 paged — this closes that asymmetry for any non-AWS HTTP client.
  • No paging signal is lost: the workflow-side escalator (validatorsapp refresh failed, threshold 3 ≈ 15 min sustained) remains the real pager, and 501/505/4xx still classify as actionable.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 1 +11 / -0 +11
Tests 2 +15 / -0 +15
Total 3 +26 / -0 +26

A one-regex classifier change; the rest is test coverage pinning it.

Key files (click to expand)
  • utils/pkg/dberror/dberror.go — adds httpStatus5xxRe and its check in Classify

Testing Verification

  • New dberror cases: the exact prod validators.app 503 string, generic 500, status_code=502 variant classify transient; 400, 501, and a 4xx with an embedded 5xx in the body stay actionable; the existing AWS-scoped and statuscode: 200 negatives still pass.
  • New solingest logger case: a third-party 503 "Activity error." line now demotes to WARN (this is the test that would have caught the page).
  • Reviewed all non-logging users of IsTransient: the S3 dberror.Retry sites already matched via the AWS shape, Neo4j/ClickHouse retry sites never produce "status code NNN" strings, and the agent's Anthropic sites only gate Sentry reporting — so the behavior change is confined to log levels plus correctly retrying/not-Sentry-reporting 5xx shapes.
  • Full -race suite passed on the branch (the dev/controlcenter embed setup failure is pre-existing on fresh checkouts — CI builds that UI first).

dberror.Classify only recognized the AWS SDK 5xx shape, so validators.app
503s ("unexpected status code 503") stayed non-transient and every Temporal
retry attempt paged via the solingest logger. The workflow-side escalator
already pages sustained outages at ~15 min; the per-attempt lines are noise.
@ben-dz
ben-dz requested a review from a team August 30, 2026 14:07

@nikw9944 nikw9944 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two minor points on the classifier, neither blocking: awsRespErrRe's 5xx alternatives are now unreachable and its scoping comment is stale, and the new pattern matches anywhere in the error string, so a 4xx whose body embeds a 5xx classifies transient.

Comment thread utils/pkg/dberror/dberror.go Outdated
Comment thread utils/pkg/dberror/dberror.go Outdated
… 200

Review notes: the first status-code mention decides classification, so a 4xx
whose body quotes a 5xx stays actionable, and awsRespErrRe shrinks to the
200-with-embedded-error case now that retryable 5xx match shape-independently.
@ben-dz
ben-dz merged commit 5d54152 into main Aug 31, 2026
8 checks passed
@ben-dz
ben-dz deleted the bdz/dberror-generic-5xx-transient branch August 31, 2026 12:12
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.

2 participants