diff --git a/lib/workos/audit_logs.rb b/lib/workos/audit_logs.rb index 7865e2ca..bf20bd13 100644 --- a/lib/workos/audit_logs.rb +++ b/lib/workos/audit_logs.rb @@ -2,6 +2,7 @@ require 'net/http' require 'uri' +require 'securerandom' module WorkOS # The Audit Logs module provides convenience methods for working with the @@ -18,6 +19,9 @@ class << self # # @return [nil] def create_event(organization:, event:, idempotency_key: nil) + # Auto-generate idempotency key if not provided + idempotency_key = SecureRandom.uuid if idempotency_key.nil? + request = post_request( path: '/audit_logs/events', auth: true, @@ -28,7 +32,8 @@ def create_event(organization:, event:, idempotency_key: nil) }, ) - execute_request(request: request) + # Explicitly setting to 3 retries for the audit log event creation request + execute_request(request: request, retries: WorkOS.config.audit_log_max_retries) end # Create an Export of Audit Log Events. diff --git a/lib/workos/client.rb b/lib/workos/client.rb index 1fa102ff..435d3d10 100644 --- a/lib/workos/client.rb +++ b/lib/workos/client.rb @@ -14,20 +14,44 @@ def client end end - def execute_request(request:) + # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity + def execute_request(request:, retries: nil) + retries = retries.nil? ? WorkOS.config.max_retries : retries + attempt = 0 + http_client = client + begin - response = client.request(request) + response = http_client.request(request) + http_status = response.code.to_i + + if http_status >= 400 + if retryable_error?(http_status) && attempt < retries + attempt += 1 + delay = calculate_retry_delay(attempt, response) + sleep(delay) + raise RetryableError.new(http_status: http_status) + else + handle_error_response(response: response) + end + end + + response rescue Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout - raise TimeoutError.new( - message: 'API Timeout Error', - ) + if attempt < retries + attempt += 1 + delay = calculate_backoff(attempt) + sleep(delay) + retry + else + raise TimeoutError.new( + message: 'API Timeout Error', + ) + end + rescue RetryableError + retry end - - http_status = response.code.to_i - handle_error_response(response: response) if http_status >= 400 - - response end + # rubocop:enable Metrics/AbcSize, Metrics/PerceivedComplexity def get_request(path:, auth: false, params: {}, access_token: nil) uri = URI(path) @@ -123,6 +147,13 @@ def handle_error_response(response:) http_status: http_status, request_id: response['x-request-id'], ) + when 408 + raise TimeoutError.new( + message: json['message'], + http_status: http_status, + request_id: response['x-request-id'], + retry_after: response['Retry-After'], + ) when 422 message = json['message'] code = json['code'] @@ -156,6 +187,32 @@ def handle_error_response(response:) private + def retryable_error?(http_status) + http_status >= 500 || http_status == 408 || http_status == 429 + end + + def calculate_backoff(attempt) + base_delay = 1.0 + max_delay = 30.0 + jitter_percentage = 0.25 + + delay = [base_delay * (2**(attempt - 1)), max_delay].min + jitter = delay * jitter_percentage * rand + delay + jitter + end + + def calculate_retry_delay(attempt, response) + # If it's a 408 or 429 with Retry-After header, use that + http_status = response.code.to_i + if [408, 429].include?(http_status) && response['Retry-After'] + retry_after = response['Retry-After'].to_i + return retry_after if retry_after.positive? + end + + # Otherwise use exponential backoff + calculate_backoff(attempt) + end + def extract_error(errors) errors.map do |error| "#{error['field']}: #{error['code']}" diff --git a/lib/workos/configuration.rb b/lib/workos/configuration.rb index 58be1f5c..f1573b73 100644 --- a/lib/workos/configuration.rb +++ b/lib/workos/configuration.rb @@ -3,10 +3,12 @@ module WorkOS # Configuration class sets config initializer class Configuration - attr_accessor :api_hostname, :timeout, :key + attr_accessor :api_hostname, :timeout, :key, :max_retries, :audit_log_max_retries def initialize @timeout = 60 + @max_retries = 0 + @audit_log_max_retries = 3 end def key! diff --git a/lib/workos/errors.rb b/lib/workos/errors.rb index c7c2acd7..53c69307 100644 --- a/lib/workos/errors.rb +++ b/lib/workos/errors.rb @@ -48,6 +48,12 @@ def to_s "#{status_string}#{@message}#{id_string}" end end + + def retryable? + return true if http_status && (http_status >= 500 || http_status == 408 || http_status == 429) + + false + end end # APIError is a generic error that may be raised in cases where none of the @@ -83,4 +89,14 @@ class NotFoundError < WorkOSError; end # UnprocessableEntityError is raised when a request is made that cannot be processed class UnprocessableEntityError < WorkOSError; end + + # RetryableError is raised internally to trigger retry logic for retryable HTTP errors + class RetryableError < StandardError + attr_reader :http_status + + def initialize(http_status:) + @http_status = http_status + super() + end + end end diff --git a/spec/lib/workos/audit_logs_spec.rb b/spec/lib/workos/audit_logs_spec.rb index df93b6b2..6348c793 100644 --- a/spec/lib/workos/audit_logs_spec.rb +++ b/spec/lib/workos/audit_logs_spec.rb @@ -6,6 +6,7 @@ before do WorkOS.configure do |config| config.key = 'example_api_key' + config.audit_log_max_retries = 3 end end @@ -53,15 +54,23 @@ end context 'without idempotency key' do - it 'creates an event' do - VCR.use_cassette 'audit_logs/create_event', match_requests_on: %i[path body] do - response = described_class.create_event( - organization: 'org_123', - event: valid_event, - ) + it 'creates an event with auto-generated idempotency_key' do + allow(SecureRandom).to receive(:uuid).and_return('test-uuid-1234') - expect(response.code).to eq '201' - end + request = double('request') + expect(described_class).to receive(:post_request).with( + path: '/audit_logs/events', + auth: true, + idempotency_key: 'test-uuid-1234', + body: hash_including(organization_id: 'org_123'), + ).and_return(request) + + allow(described_class).to receive(:execute_request).and_return(double(code: '201')) + + described_class.create_event( + organization: 'org_123', + event: valid_event, + ) end end @@ -81,6 +90,71 @@ end end end + + context 'with retry logic using same idempotency key' do + it 'retries with the same idempotency key on retryable errors' do + allow(described_class).to receive(:client).and_return(double('client')) + + call_count = 0 + allow(described_class.client).to receive(:request) do |request| + call_count += 1 + # Verify the same idempotency key is used on every retry + expect(request['Idempotency-Key']).to eq('test-idempotency-key') + + if call_count < 3 + # Return 500 error for first 2 attempts + response = double('response', code: '500', body: '{"message": "Internal Server Error"}') + allow(response).to receive(:[]).with('x-request-id').and_return('test-request-id') + allow(response).to receive(:[]).with('Retry-After').and_return(nil) + response + else + # Success on 3rd attempt + double('response', code: '201', body: '{}') + end + end + + expect(described_class).to receive(:sleep).exactly(2).times + + response = described_class.create_event( + organization: 'org_123', + event: valid_event, + idempotency_key: 'test-idempotency-key', + ) + + expect(response.code).to eq('201') + expect(call_count).to eq(3) + end + end + + context 'with retry limit exceeded' do + it 'stops retrying after hitting retry limit' do + allow(described_class).to receive(:client).and_return(double('client')) + + call_count = 0 + allow(described_class.client).to receive(:request) do |request| + call_count += 1 + expect(request['Idempotency-Key']).to eq('test-idempotency-key') + + response = double('response', code: '503', body: '{"message": "Service Unavailable"}') + allow(response).to receive(:[]).with('x-request-id').and_return('test-request-id') + allow(response).to receive(:[]).with('Retry-After').and_return(nil) + response + end + + expect(described_class).to receive(:sleep).exactly(3).times + + expect do + described_class.create_event( + organization: 'org_123', + event: valid_event, + idempotency_key: 'test-idempotency-key', + ) + end.to raise_error(WorkOS::APIError) + + # Should make 4 total attempts: 1 initial + 3 retries + expect(call_count).to eq(4) + end + end end end diff --git a/spec/lib/workos/client_retry_spec.rb b/spec/lib/workos/client_retry_spec.rb new file mode 100644 index 00000000..64c13c38 --- /dev/null +++ b/spec/lib/workos/client_retry_spec.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +describe WorkOS::Client do + before do + WorkOS.configure do |config| + config.key = 'test_api_key' + config.max_retries = 3 + end + end + + after do + # Reset to default after each test + WorkOS.config.max_retries = 0 + end + + let(:test_module) do + Module.new do + extend WorkOS::Client + + def self.test_request + request = get_request(path: '/test', auth: true) + execute_request(request: request) + end + end + end + + describe 'retry logic' do + context 'with 500 errors' do + it 'retries up to max_retries times' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request) do + double('response', code: '500', body: '{"message": "Internal Server Error"}', '[]': nil) + end + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::APIError) + end + end + + context 'with 503 errors' do + it 'retries on service unavailable' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request) do + double('response', code: '503', body: '{"message": "Service Unavailable"}', '[]': nil) + end + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::APIError) + end + end + + context 'with 408 errors' do + it 'retries with exponential backoff' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request) do + double('response', code: '408', body: '{"message": "Request Timeout"}', '[]': nil) + end + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::TimeoutError) + end + end + + context 'with network timeout errors' do + it 'retries on Net::OpenTimeout' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request).and_raise(Net::OpenTimeout) + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::TimeoutError, 'API Timeout Error') + end + + it 'retries on Net::ReadTimeout' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request).and_raise(Net::ReadTimeout) + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::TimeoutError, 'API Timeout Error') + end + + it 'retries on Net::WriteTimeout' do + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request).and_raise(Net::WriteTimeout) + + expect(test_module.client).to receive(:request).exactly(4).times + expect(test_module).to receive(:sleep).exactly(3).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::TimeoutError, 'API Timeout Error') + end + end + + context 'with successful retry' do + it 'succeeds after retryable failure' do + allow(test_module).to receive(:client).and_return(double('client')) + + call_count = 0 + allow(test_module.client).to receive(:request) do + call_count += 1 + if call_count < 3 + response = double('response', code: '500', body: '{"message": "Internal Server Error"}') + allow(response).to receive(:[]).with('x-request-id').and_return('test-request-id') + allow(response).to receive(:[]).with('Retry-After').and_return(nil) + response + else + double('response', code: '200', body: '{"success": true}') + end + end + + expect(test_module).to receive(:sleep).exactly(2).times + + response = test_module.test_request + expect(response.code).to eq('200') + end + end + + context 'respects max_retries configuration' do + it 'uses configured max_retries value' do + WorkOS.config.max_retries = 2 + + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request) do + double('response', code: '500', body: '{"message": "Internal Server Error"}', '[]': nil) + end + + expect(test_module.client).to receive(:request).exactly(3).times + expect(test_module).to receive(:sleep).exactly(2).times + + expect do + test_module.test_request + end.to raise_error(WorkOS::APIError) + end + + it 'does not retry when max_retries is 0' do + WorkOS.config.max_retries = 0 + + allow(test_module).to receive(:client).and_return(double('client')) + allow(test_module.client).to receive(:request) do + double('response', code: '500', body: '{"message": "Internal Server Error"}', '[]': nil) + end + + expect(test_module.client).to receive(:request).once + expect(test_module).not_to receive(:sleep) + + expect do + test_module.test_request + end.to raise_error(WorkOS::APIError) + end + end + end + + describe '#retryable_error?' do + it 'returns true for 5xx errors' do + expect(test_module.send(:retryable_error?, 500)).to eq(true) + expect(test_module.send(:retryable_error?, 503)).to eq(true) + expect(test_module.send(:retryable_error?, 599)).to eq(true) + end + + it 'returns true for 408 errors' do + expect(test_module.send(:retryable_error?, 408)).to eq(true) + end + + it 'returns true for 429 errors' do + expect(test_module.send(:retryable_error?, 429)).to eq(true) + end + end +end