Skip to content

Group Faraday errors in Sentry by host - #1034

Merged
zetter-rpf merged 1 commit into
mainfrom
group-faraday-sentry-errors
Sep 24, 2026
Merged

zetter-rpf merged 1 commit into
mainfrom
group-faraday-sentry-errors

Conversation

@zetter-rpf

@zetter-rpf zetter-rpf commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

What's changed?

A large proportion of the errors we get in Sentry are HTTP related errors from Faraday - two thirds of errors in the last 14 days.

Currently Sentry tends to group them by controller error/action. This leads to many groups as there are certain APIs (profile and userinfo) that we use from many different controller actions. This makes it hard to see what APIs are having problems, and to set any ignore limits to allow for some failed requests for a given API.

This change creates a Sentry middleware that records the host in the error since not all sentry errors had the host accessible. To make sure we use the middleware everywhere I've made a custom faraday client (HttpClient) and replaced our uses of faraday with that.

See sentry fingerprinting docs for more.

@cla-bot cla-bot Bot added the cla-signed label Sep 24, 2026
@zetter-rpf
zetter-rpf marked this pull request as ready for review September 24, 2026 08:30
Copilot AI lite review requested due to automatic review settings September 24, 2026 08:30
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Test coverage

93.7% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/35979674415

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 review overview

🟡 Changes recommended

The integration task still uses Faraday directly, bypassing host recording and Sentry grouping.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds host-aware Sentry grouping for Faraday errors through a shared HTTP client and middleware.

Changes:

  • Records request hosts on Faraday exceptions.
  • Fingerprints errors by exception class and host.
  • Migrates API clients and adds coverage.
File Summary Review
spec/​lib/​record_request_host_in_errors_spec.rb Tests host recording. Reviewed
spec/​configuration/​sentry_config_spec.rb Tests Sentry grouping. Reviewed
lib/​user_info_api_client.rb Uses shared client. Reviewed
lib/​scratch_config_importer.rb Uses shared client. Reviewed
lib/​scratch_asset_importer.rb Uses shared client. Reviewed
lib/​record_request_host_in_errors.rb Records hosts on errors. Reviewed
lib/​profile_api_client.rb Uses shared client. Reviewed
lib/​hydra_public_api_client.rb Uses shared client. Reviewed
lib/​http_client.rb Provides shared Faraday client. Moderate: remaining integration-test client bypasses this wrapper.
config/​initializers/​sentry.rb Configures host-based fingerprints. Moderate: integration-task failures bypass host-aware grouping.
app/​services/​subscriptions/​turnstile_verifier.rb Uses shared client. Reviewed
app/​services/​subscriptions/​pardot_form_handler_submitter.rb Uses shared client. Reviewed
app/​controllers/​api/​google_auth_controller.rb Uses shared client. Reviewed

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/http_client.rb
@raspberrypiherokubot
raspberrypiherokubot temporarily deployed to editor-api-p-group-fara-urktqj September 24, 2026 08:35 Inactive
A large proportion of the errors we get in Sentry are connection related errors from Faraday - two thirds of errors in the last 14 days.

Currently Sentry tends to group them by controller error/action. This leads to many groups as there are certain APIs (profile and userinfo) that we use from many different controller actions. This makes it hard to see what APIs are having problems, and to set any ignore limits to allow for some failed requests for a given API.

This change creates a Sentry middleware that records the host in the error since not all sentry errors had the host accessible. To make sure we use the middleware everywhere I've made a custom faraday client (HttpClient) and replaced our uses of faraday with that.

See sentry fingerprinting docs for more:

https://docs.sentry.io/platforms/ruby/usage/sdk-fingerprinting/

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zetter-rpf
zetter-rpf force-pushed the group-faraday-sentry-errors branch from cc90535 to 12bf7e1 Compare September 24, 2026 09:10
@zetter-rpf
zetter-rpf temporarily deployed to editor-api-p-group-fara-urktqj September 24, 2026 09:11 Inactive

@mwtrew mwtrew 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.

Nice idea

@zetter-rpf
zetter-rpf merged commit 323f3d0 into main Sep 24, 2026
8 checks passed
@zetter-rpf
zetter-rpf deleted the group-faraday-sentry-errors branch September 24, 2026 14:50

This branch was previously deployed

1 inactive deployment
editor-api-p-group-fara-urktqj — 12bf7e13 Deployed Sep 24, 2026 by zetter-rpf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants