From 8465c3ac1326c61250fb62791f85919fc5f36c37 Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:34:41 +0200 Subject: [PATCH] feat: give each documented status its own error class 401, 413, 422, 429, and 529 had classes. 400, 403, 404, and every other 5xx arrived as a bare ApiError, so telling a malformed request from a revoked key from a wrong base URL from an outage meant reading #status and writing the branch yourself -- in a rescue, where a case on an integer is the worst place for it. Add BadRequest, PermissionDenied, NotFound, and ServerError, named after the classes the official Typesafe SDKs raise, so code written against those reads the same here. Overloaded becomes a ServerError: `rescue ServerError` now covers every 5xx including 529, and `rescue Overloaded` still picks out the one the API documents by name. The status-to-class mapping is a table rather than a case, and an undocumented status still lands on ApiError. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 ++++ README.md | 8 ++- lib/ruby_decision_model/client.rb | 40 ++++++++------- lib/ruby_decision_model/errors.rb | 14 +++++- test/error_classes_test.rb | 82 +++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 20 deletions(-) create mode 100644 test/error_classes_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..0a53f6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## Unreleased + +- Added `BadRequest` (400), `PermissionDenied` (403), `NotFound` (404), and + `ServerError` (any 5xx), matching the classes the official Typesafe SDKs + raise. These statuses used to arrive as a bare `ApiError`, so the only way + to tell a bad key from a bad URL from an outage was to read `#status`. +- `Overloaded` (529) is now a `ServerError`, so `rescue ServerError` covers + every 5xx while `rescue Overloaded` still picks out that one. + ## 0.1.0 - 2026-09-18 Provider-neutral release. One `Client`, two providers behind it. diff --git a/README.md b/README.md index 79638d6..2890df4 100644 --- a/README.md +++ b/README.md @@ -174,12 +174,16 @@ return is still accepted and treated as having no headers, which means no | `ConfigurationError` | No provider could be resolved, missing api_key, unknown provider, or bad `retry:` value | | `RequestError` | Questions hash was empty | | `TransportError` (`TimeoutError`) | Network or timeout failure after retries, carries `#cause_error` | -| `ApiError` | Non-2xx response, carries `#status`, `#body`, and `#headers` | +| `ApiError` | Non-2xx response, carries `#status`, `#body`, and `#headers`; the parent of the rest | +| `BadRequest` | 400 | | `Unauthorized` | 401 | +| `PermissionDenied` | 403 | +| `NotFound` | 404 | | `PayloadTooLarge` | 413 | | `UnprocessableEntity` | 422 (never retried) | | `RateLimited` | 429 (retried) | -| `Overloaded` | 529 (retried) | +| `ServerError` | any 5xx (retried) | +| `Overloaded` | 529, a `ServerError` (retried) | | `InvalidResponse` | Body wasn't JSON, wasn't a Hash, or an answer was malformed | | `MissingAnswers` | One or more question ids came back missing or wrong-typed, carries `#missing` | diff --git a/lib/ruby_decision_model/client.rb b/lib/ruby_decision_model/client.rb index 3457726..207ff61 100644 --- a/lib/ruby_decision_model/client.rb +++ b/lib/ruby_decision_model/client.rb @@ -162,25 +162,31 @@ def raise_transport_error(exception) raise TransportError.new("transport error: #{exception.message}", cause_error: exception) end + # Status => [error class, message]. Every status the API documents gets a + # class a caller can rescue on its own. + ERRORS = { + 400 => [BadRequest, "bad request"], + 401 => [Unauthorized, "unauthorized"], + 403 => [PermissionDenied, "permission denied"], + 404 => [NotFound, "not found"], + 413 => [PayloadTooLarge, "payload too large"], + 422 => [UnprocessableEntity, "unprocessable entity"], + 429 => [RateLimited, "rate limited"], + 529 => [Overloaded, "overloaded"] + }.freeze + def handle_response(status, response_body, response_headers, questions) - case status - when 200..299 - parse_success(response_body, response_headers, questions) - when 401 - raise Unauthorized.new("unauthorized", status: status, body: response_body, headers: response_headers) - when 413 - raise PayloadTooLarge.new("payload too large", status: status, body: response_body, headers: response_headers) - when 422 - raise UnprocessableEntity.new("unprocessable entity", status: status, body: response_body, - headers: response_headers) - when 429 - raise RateLimited.new("rate limited", status: status, body: response_body, headers: response_headers) - when 529 - raise Overloaded.new("overloaded", status: status, body: response_body, headers: response_headers) - else - raise ApiError.new("api error (status #{status})", status: status, body: response_body, - headers: response_headers) + return parse_success(response_body, response_headers, questions) if (200..299).cover?(status) + + klass, message = ERRORS.fetch(status) do + if (500..599).cover?(status) + [ServerError, "server error (status #{status})"] + else + [ApiError, "api error (status #{status})"] + end end + + raise klass.new(message, status: status, body: response_body, headers: response_headers) end def parse_success(response_body, response_headers, questions) diff --git a/lib/ruby_decision_model/errors.rb b/lib/ruby_decision_model/errors.rb index d35c51a..74a6303 100644 --- a/lib/ruby_decision_model/errors.rb +++ b/lib/ruby_decision_model/errors.rb @@ -29,15 +29,27 @@ def initialize(message, status:, body:, headers: {}) end end + # One class per status the API documents, so a caller can rescue the case + # it knows how to handle. The names follow the official Typesafe SDKs. + class BadRequest < ApiError; end + class Unauthorized < ApiError; end + class PermissionDenied < ApiError; end + + class NotFound < ApiError; end + class PayloadTooLarge < ApiError; end class UnprocessableEntity < ApiError; end class RateLimited < ApiError; end - class Overloaded < ApiError; end + # Any 5xx. Overloaded is one, so `rescue ServerError` covers both and + # `rescue Overloaded` still picks out the one the API defines. + class ServerError < ApiError; end + + class Overloaded < ServerError; end class InvalidResponse < Error attr_reader :answers diff --git a/test/error_classes_test.rb b/test/error_classes_test.rb new file mode 100644 index 0000000..0bc4b45 --- /dev/null +++ b/test/error_classes_test.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "test_helper" + +# One rescuable class per status the API documents. +class ErrorClassesTest < Minitest::Test + def questions + { "urgent" => RubyDecisionModel::Questions.noul("Is this urgent?") } + end + + def raise_for(status) + client = RubyDecisionModel::Client.new( + api_key: "test-key", sleeper: no_sleep, retry: { max_retries: 0 }, + transport: FakeTransport.new([[status, %({"error":"nope"}), { "x-thing" => "v" }]]) + ) + client.ask(state: {}, questions: questions) + end + + def assert_status_raises(status, klass) + error = assert_raises(klass) { raise_for(status) } + assert_equal status, error.status + assert_equal %({"error":"nope"}), error.body + error + end + + def test_documented_statuses_get_their_own_class + { + 400 => RubyDecisionModel::BadRequest, + 401 => RubyDecisionModel::Unauthorized, + 403 => RubyDecisionModel::PermissionDenied, + 404 => RubyDecisionModel::NotFound, + 413 => RubyDecisionModel::PayloadTooLarge, + 422 => RubyDecisionModel::UnprocessableEntity, + 429 => RubyDecisionModel::RateLimited, + 529 => RubyDecisionModel::Overloaded + }.each { |status, klass| assert_status_raises(status, klass) } + end + + def test_any_other_five_hundred_is_a_server_error + [500, 502, 503, 504, 599].each { |status| assert_status_raises(status, RubyDecisionModel::ServerError) } + end + + def test_overloaded_is_a_server_error_too + assert_status_raises(529, RubyDecisionModel::ServerError) + # and is still rescuable on its own + assert_status_raises(529, RubyDecisionModel::Overloaded) + end + + def test_a_server_error_is_not_an_overloaded + error = assert_raises(RubyDecisionModel::ServerError) { raise_for(503) } + refute_kind_of RubyDecisionModel::Overloaded, error + end + + def test_an_undocumented_status_falls_back_to_api_error + error = assert_status_raises(418, RubyDecisionModel::ApiError) + assert_match(/status 418/, error.message) + end + + def test_every_error_is_an_api_error_and_carries_its_headers + [400, 401, 403, 404, 413, 422, 429, 500, 529, 418].each do |status| + error = assert_raises(RubyDecisionModel::ApiError) { raise_for(status) } + assert_equal({ "x-thing" => "v" }, error.headers, "status #{status}") + end + end + + def test_messages_name_what_went_wrong + assert_match(/bad request/, assert_raises(RubyDecisionModel::BadRequest) { raise_for(400) }.message) + assert_match(/permission denied/, assert_raises(RubyDecisionModel::PermissionDenied) { raise_for(403) }.message) + assert_match(/not found/, assert_raises(RubyDecisionModel::NotFound) { raise_for(404) }.message) + assert_match(/status 503/, assert_raises(RubyDecisionModel::ServerError) { raise_for(503) }.message) + end + + def test_a_two_hundred_is_still_parsed + body = JSON.generate("answers" => { "urgent" => { "type" => "noul", "noul" => 0.5 } }, "usage" => {}) + client = RubyDecisionModel::Client.new( + api_key: "test-key", sleeper: no_sleep, transport: FakeTransport.new([[204, body]]) + ) + + # 204 is in 200..299 but carries a body here; the success path still runs. + assert_in_delta 0.5, client.ask(state: {}, questions: questions)["urgent"].noul + end +end