From 12bf7e133cd0be35878db88029f5df291829a580 Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Thu, 24 Sep 2026 10:10:47 +0100 Subject: [PATCH] Group Faraday errors in Sentry by host 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) --- app/controllers/api/google_auth_controller.rb | 2 +- .../pardot_form_handler_submitter.rb | 2 +- .../subscriptions/turnstile_verifier.rb | 2 +- config/initializers/sentry.rb | 8 +++++ lib/http_client.rb | 12 +++++++ lib/hydra_public_api_client.rb | 2 +- lib/profile_api_client.rb | 2 +- lib/record_request_host_in_errors.rb | 17 ++++++++++ lib/scratch_asset_importer.rb | 2 +- lib/scratch_config_importer.rb | 2 +- lib/tasks/integration_tests.rake | 2 +- lib/user_info_api_client.rb | 2 +- spec/configuration/sentry_config_spec.rb | 29 +++++++++++++++++ .../lib/record_request_host_in_errors_spec.rb | 32 +++++++++++++++++++ 14 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 lib/http_client.rb create mode 100644 lib/record_request_host_in_errors.rb create mode 100644 spec/configuration/sentry_config_spec.rb create mode 100644 spec/lib/record_request_host_in_errors_spec.rb diff --git a/app/controllers/api/google_auth_controller.rb b/app/controllers/api/google_auth_controller.rb index 9f60d3067..c003e56d0 100644 --- a/app/controllers/api/google_auth_controller.rb +++ b/app/controllers/api/google_auth_controller.rb @@ -23,7 +23,7 @@ def exchange_code private def faraday - Faraday.new do |f| + HttpClient.new do |f| f.request :url_encoded f.options.timeout = 10 f.options.open_timeout = 5 diff --git a/app/services/subscriptions/pardot_form_handler_submitter.rb b/app/services/subscriptions/pardot_form_handler_submitter.rb index 39ae502f8..c3842c2eb 100644 --- a/app/services/subscriptions/pardot_form_handler_submitter.rb +++ b/app/services/subscriptions/pardot_form_handler_submitter.rb @@ -39,7 +39,7 @@ def call(form_payload:) attr_reader :endpoint_url def faraday - @faraday ||= Faraday.new do |f| + @faraday ||= HttpClient.new do |f| f.request :url_encoded f.options.timeout = REQUEST_TIMEOUT_SECONDS f.options.open_timeout = OPEN_TIMEOUT_SECONDS diff --git a/app/services/subscriptions/turnstile_verifier.rb b/app/services/subscriptions/turnstile_verifier.rb index c7477c370..2e933b96c 100644 --- a/app/services/subscriptions/turnstile_verifier.rb +++ b/app/services/subscriptions/turnstile_verifier.rb @@ -40,7 +40,7 @@ def passed? attr_reader :secret_key, :remote_ip, :token def faraday - @faraday ||= Faraday.new do |f| + @faraday ||= HttpClient.new do |f| f.request :url_encoded f.options.timeout = 5 f.options.open_timeout = 2 diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index 447ea8551..5a154c4ad 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -7,4 +7,12 @@ config.rails.structured_logging.enabled = false config.traces_sample_rate = 0.1 + + config.before_send = lambda do |event, hint| + exception = hint[:exception] + next event unless exception.is_a?(Faraday::Error) + + event.fingerprint = [exception.class.name, exception.try(:request_host) || 'unknown-host'] + event + end end diff --git a/lib/http_client.rb b/lib/http_client.rb new file mode 100644 index 000000000..01d702298 --- /dev/null +++ b/lib/http_client.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'faraday' + +module HttpClient + def self.new(url = nil, options = {}) + Faraday.new(url, options) do |f| + f.use RecordRequestHostInErrors + yield f + end + end +end diff --git a/lib/hydra_public_api_client.rb b/lib/hydra_public_api_client.rb index e06cf72e0..67561b66c 100644 --- a/lib/hydra_public_api_client.rb +++ b/lib/hydra_public_api_client.rb @@ -50,7 +50,7 @@ def stubbed_user end def conn - @conn ||= Faraday.new(API_URL) do |f| + @conn ||= HttpClient.new(API_URL) do |f| f.request :url_encoded f.response :raise_error f.response :json diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index ad41f358d..d02119fad 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -221,7 +221,7 @@ def update_school_email_domains(token:, school_id:, school_email_domains: []) private def connection(token) - Faraday.new(ENV.fetch('IDENTITY_URL')) do |faraday| + HttpClient.new(ENV.fetch('IDENTITY_URL')) do |faraday| faraday.request :json faraday.response :json faraday.response :raise_error, allowed_statuses: [401] diff --git a/lib/record_request_host_in_errors.rb b/lib/record_request_host_in_errors.rb new file mode 100644 index 000000000..51f4e71e9 --- /dev/null +++ b/lib/record_request_host_in_errors.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'faraday' + +class RecordRequestHostInErrors < Faraday::Middleware + module RequestHost + attr_accessor :request_host + end + + def call(env) + super + rescue Faraday::Error => e + e.extend(RequestHost) + e.request_host = env.url.host + raise + end +end diff --git a/lib/scratch_asset_importer.rb b/lib/scratch_asset_importer.rb index 8cf5cee21..5591100d8 100644 --- a/lib/scratch_asset_importer.rb +++ b/lib/scratch_asset_importer.rb @@ -102,7 +102,7 @@ def s3_client end def connection - @connection ||= Faraday.new(url: asset_base_url) do |faraday| + @connection ||= HttpClient.new(url: asset_base_url) do |faraday| faraday.response :raise_error end end diff --git a/lib/scratch_config_importer.rb b/lib/scratch_config_importer.rb index 1a6ef53cf..c883b5163 100644 --- a/lib/scratch_config_importer.rb +++ b/lib/scratch_config_importer.rb @@ -20,7 +20,7 @@ def import end def connection - Faraday.new(url: asset_config_url) do |faraday| + HttpClient.new(url: asset_config_url) do |faraday| faraday.response :raise_error end end diff --git a/lib/tasks/integration_tests.rake b/lib/tasks/integration_tests.rake index 06faeb68d..70f3dfbb5 100644 --- a/lib/tasks/integration_tests.rake +++ b/lib/tasks/integration_tests.rake @@ -20,7 +20,7 @@ namespace :integration_tests do end def connection - Faraday.new do |faraday| + HttpClient.new do |faraday| faraday.request :json faraday.headers = { 'Accept' => 'application/vnd.github+json', diff --git a/lib/user_info_api_client.rb b/lib/user_info_api_client.rb index 74f96370b..60f1025d3 100644 --- a/lib/user_info_api_client.rb +++ b/lib/user_info_api_client.rb @@ -49,7 +49,7 @@ def transform_result(result) end def conn - Faraday.new( + HttpClient.new( headers: { authorization: "Bearer #{API_KEY}" }, url: API_URL ) do |f| diff --git a/spec/configuration/sentry_config_spec.rb b/spec/configuration/sentry_config_spec.rb new file mode 100644 index 000000000..d68cb6c7d --- /dev/null +++ b/spec/configuration/sentry_config_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Sentry do + describe 'before_send' do + let(:event) { Sentry::ErrorEvent.new(configuration: described_class.configuration, integration_meta: nil) } + + def fingerprint_for(exception) + described_class.configuration.before_send.call(event, { exception: }).fingerprint + end + + it 'groups Faraday errors by class and request host' do + exception = Faraday::ServerError.new('boom') + exception.extend(RecordRequestHostInErrors::RequestHost) + exception.request_host = 'api.example.com' + + expect(fingerprint_for(exception)).to eq(['Faraday::ServerError', 'api.example.com']) + end + + it 'groups Faraday errors raised outside a request under an unknown host' do + expect(fingerprint_for(Faraday::TimeoutError.new)).to eq(['Faraday::TimeoutError', 'unknown-host']) + end + + it 'leaves other exceptions on the default grouping' do + expect(fingerprint_for(StandardError.new)).to be_blank + end + end +end diff --git a/spec/lib/record_request_host_in_errors_spec.rb b/spec/lib/record_request_host_in_errors_spec.rb new file mode 100644 index 000000000..899378302 --- /dev/null +++ b/spec/lib/record_request_host_in_errors_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe RecordRequestHostInErrors do + let(:connection) do + HttpClient.new('https://api.example.com') do |f| + f.response :raise_error + end + end + + def request_host_from_error + connection.get('/things') + nil + rescue Faraday::Error => e + e.request_host + end + + describe '#call' do + it 'records the host on errors raised by response middleware' do + stub_request(:get, 'https://api.example.com/things').to_return(status: 500) + + expect(request_host_from_error).to eq('api.example.com') + end + + it 'records the host on errors raised by the adapter' do + stub_request(:get, 'https://api.example.com/things').to_timeout + + expect(request_host_from_error).to eq('api.example.com') + end + end +end