Skip to content

feat: Stop reporting client errors to Sentry - #45

Merged
joamag merged 1 commit into
masterfrom
feat/sentry-client-errors
Sep 14, 2026
Merged

joamag merged 1 commit into
masterfrom
feat/sentry-client-errors

Conversation

@joamag

@joamag joamag commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Related to #38.

Client errors reach Sentry through every capture surface, the controller actions (request.exception), the REST dispatch (request.end) and the logging handler, even though they are caused by the client rather than by a failure of the server. In the last 7 days the LDJ instance submitted 3,345 events for requests answered with a 404 (eg: MVCRequestNotHandled for /favicon.ico), against 1,235 for server errors.

Changes

  • is_ignored() also ignores the exceptions whose status code is a client error (400 to 499), as resolved by resolve_status_code() from the same status_code attribute that the MVC, REST and WSGI layers use to build the error response, so capture() drops them whatever the surface
  • Messages logged without an exception are still reported, even for a request answered with a client error, as they are explicitly logged by the server
  • The design document describes the behaviour

Verification

  • Tests for every surface (controller actions, REST dispatch, logging handler and explicit capture), for the bounds of the range (399, 400, 499 and 500), for textual and invalid status codes, and for a message logged while a request is answered with a client error
  • 5 of the new tests fail against the previous code and all of them pass with the change
  • 100% coverage of the changed statements (4 of 4)
  • Complete suite passing locally (21 bundles, 1248 tests) and black --check clean

- Exceptions with a 4XX status code are ignored whatever the capture surface
- Messages logged without an exception keep being reported
- Removes most of the noise, such as the requests for missing resources
- Tests cover every surface and the bounds of the client error range
Copilot AI lite review requested due to automatic review settings September 14, 2026 19:28

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-14T19:31:32.129429Z c4ab7af PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3d1d76e2-c967-4f58-b352-f57601362f74

📥 Commits

Reviewing files that changed from the base of the PR and between bef6904 and c4ab7af.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • diagnostics/src/diagnostics_sentry/system.py
  • diagnostics/src/diagnostics_sentry/test.py
  • doc/design/004-sentry_error_reporting.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Sentry diagnostics now ignore exceptions with 4XX status codes across capture paths. Tests cover request, record, direct capture, and status-code handling. Messages without exceptions remain reportable.

Changes

Sentry client-error filtering

Layer / File(s) Summary
Client-error filtering rule
diagnostics/src/diagnostics_sentry/system.py, doc/design/004-sentry_error_reporting.md, CHANGELOG.md
is_ignored now ignores exceptions with status codes from 400 through 499. The design document and changelog describe the updated behavior.
Capture-surface validation
diagnostics/src/diagnostics_sentry/test.py
Tests verify filtering through request, record, and direct capture paths. Tests also verify that messages without exceptions remain captured and that non-4XX values are not ignored.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to c4ab7

The change filters client-error exceptions while preserving explicitly logged messages, with no established merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: stopping Sentry reporting for client errors.
Description check ✅ Passed The description directly explains the client-error filtering behavior, preserved message reporting, affected capture surfaces, and verification.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 2 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@joamag joamag left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LGTM

@joamag
joamag merged commit 8d91b3a into master Sep 14, 2026
68 of 78 checks passed
@joamag
joamag deleted the feat/sentry-client-errors branch September 14, 2026 19:33
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