From 6a24cdb54ca583a7451f9daa86ef19a72f849d1a Mon Sep 17 00:00:00 2001 From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com> Date: Thu, 22 Oct 2026 09:56:18 +0100 Subject: [PATCH] Avoid timeouts for batch requests We're seeing increased connection errors from the SchoolClasses#import action and reports of errors when importing students Looking at the last month, some of these requests take 30 seconds (which I believe may have hit timeouts before, they are just even more likely to now) The platform team are looking at ways we can speed up the issue as creating passwords for users may be a bottleneck. We could also look at moving the class import into a background job. --- lib/profile_api_client.rb | 21 ++++++--- spec/lib/profile_api_client_spec.rb | 44 +++++++++++++++++++ .../profile_api_client_examples.rb | 16 +++++++ 3 files changed, 76 insertions(+), 5 deletions(-) diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index d02119fad..b19dbef78 100644 --- a/lib/profile_api_client.rb +++ b/lib/profile_api_client.rb @@ -111,7 +111,7 @@ def validate_school_students(token:, students:, school_id:) students = Array(students) endpoint = "/api/v1/schools/#{school_id}/students/preflight-student-upload" - response = connection(token).post(endpoint) do |request| + response = connection(token, connection_options_for_slow_requests).post(endpoint) do |request| request.body = students.to_json request.headers['Content-Type'] = 'application/json' end @@ -128,7 +128,7 @@ def create_school_students(token:, students:, school_id:, preflight: false) students = Array(students) endpoint = "/api/v1/schools/#{school_id}/students" endpoint += '/preflight' if preflight - response = connection(token).post(endpoint) do |request| + response = connection(token, connection_options_for_slow_requests).post(endpoint) do |request| request.body = students.to_json request.headers['Content-Type'] = 'application/json' end @@ -146,7 +146,7 @@ def create_school_students_sso(token:, students:, school_id:) students = Array(students) endpoint = "/api/v1/schools/#{school_id}/students/sso" - response = connection(token).post(endpoint) do |request| + response = connection(token, connection_options_for_slow_requests).post(endpoint) do |request| request.body = students.to_json request.headers['Content-Type'] = 'application/json' end @@ -220,8 +220,8 @@ def update_school_email_domains(token:, school_id:, school_email_domains: []) private - def connection(token) - HttpClient.new(ENV.fetch('IDENTITY_URL')) do |faraday| + def connection(token, options = {}) + HttpClient.new(ENV.fetch('IDENTITY_URL'), options) do |faraday| faraday.request :json faraday.response :json faraday.response :raise_error, allowed_statuses: [401] @@ -263,5 +263,16 @@ def unauthorized!(response) # The API is only available to verified non-student users that are over 13. Others get a 401. raise UnauthorizedError, 'Profile API unauthorized' if response.status == 401 end + + def connection_options_for_slow_requests + { + request: { + read_timeout: 50, + open_timeout: 5, + write_timeout: 5, + timeout: 50 + } + } + end end end diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 1033baab1..53a497e1e 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -228,6 +228,48 @@ def create_school_student end end + describe '.validate_school_students' do + subject(:validate_school_students_response) { validate_school_students } + + let(:students) { [{ name: 'name', username: 'username', password: 'password' }] } + let(:school) { build(:school, id: SecureRandom.uuid) } + let(:validate_students_url) { "#{api_url}/api/v1/schools/#{school.id}/students/preflight-student-upload" } + + before do + stub_request(:post, validate_students_url).to_return(status: 200, body: '{}', headers: { 'content-type' => 'application/json' }) + end + + it_behaves_like 'an authenticated JSON API request', :post, url: -> { validate_students_url } + it_behaves_like 'a request that handles standard HTTP errors', :post, url: -> { validate_students_url } + it_behaves_like 'a request that handles an unexpected response status', :post, url: -> { validate_students_url }, status: 201 + it_behaves_like 'a request that allows longer for a response' + + it 'sends the student details in the request body' do + validate_school_students_response + expect(WebMock).to have_requested(:post, validate_students_url).with(body: students.to_json) + end + + it 'raises 422 exception with the validation errors if 422 status code is returned' do + response = { errors: [{ message: 'The password is well dodgy' }] } + stub_request(:post, validate_students_url) + .to_return(status: 422, body: response.to_json, headers: { 'content-type' => 'application/json' }) + + expect { validate_school_students }.to raise_error(ProfileApiClient::Student422Error) do |error| + expect(error.errors.first['message']).to eq('The password is well dodgy') + end + end + + it 'returns nil if token is blank' do + expect(described_class.validate_school_students(token: '', students:, school_id: school.id)).to be_nil + end + + private + + def validate_school_students + described_class.validate_school_students(token:, students:, school_id: school.id) + end + end + describe '.create_school_students' do subject(:create_school_students_response) { create_school_students } @@ -245,6 +287,7 @@ def create_school_student it_behaves_like 'an authenticated JSON API request', :post, url: -> { create_students_url } it_behaves_like 'a request that handles standard HTTP errors', :post, url: -> { create_students_url } it_behaves_like 'a request that handles an unexpected response status', :post, url: -> { create_students_url }, status: 202 + it_behaves_like 'a request that allows longer for a response' it 'raises 422 exception with the relevant message if 400 status code is returned' do response = { errors: [{ message: 'The password is well dodgy' }] } @@ -317,6 +360,7 @@ def create_school_students it_behaves_like 'an authenticated JSON API request', :post, url: -> { create_students_sso_url } it_behaves_like 'a request that handles standard HTTP errors', :post, url: -> { create_students_sso_url } it_behaves_like 'a request that handles an unexpected response status', :post, url: -> { create_students_sso_url }, status: 202 + it_behaves_like 'a request that allows longer for a response' it 'raises 422 exception with the relevant message if 400 status code is returned' do response = { errors: [{ message: 'The password is well dodgy' }] } diff --git a/spec/support/shared_examples/profile_api_client_examples.rb b/spec/support/shared_examples/profile_api_client_examples.rb index 9a89e7a20..7b64754dd 100644 --- a/spec/support/shared_examples/profile_api_client_examples.rb +++ b/spec/support/shared_examples/profile_api_client_examples.rb @@ -73,3 +73,19 @@ expect { subject }.to raise_error(ProfileApiClient::UnexpectedResponse) end end + +RSpec.shared_examples 'a request that allows longer for a response' do + let(:faraday_connections) { [] } + + before do + connections = faraday_connections + allow(HttpClient).to receive(:new).and_wrap_original do |original_method, *args, &block| + original_method.call(*args, &block).tap { |connection| connections << connection } + end + end + + it 'gives the profile api longer to respond than the default timeouts allow' do + subject + expect(faraday_connections.last.options).to have_attributes(open_timeout: 5, write_timeout: 5, read_timeout: 50, timeout: 50) + end +end