From f34ed1960bffef51a71428c7d943c30b44fcc0cc Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:49:56 +0200 Subject: [PATCH] feat: read what the server said went wrong Both providers explain their errors. Typesafe's 422 details the offending field; OpenRouter answers with {"error": {"code", "message"}}. Neither reached the exception. ApiError#message was a constant -- "unprocessable entity" -- and the explanation sat on #body as a JSON string, so every caller who wanted the reason wrote the same parse, and every log line that printed the exception and not the body threw the reason away. ApiError now parses a JSON object body into #detail, #error_code, and #parsed_body, reading them whether the payload nests them under "error" or puts them at the top level, and appends the detail to the message. A 422 reads "unprocessable entity: questions.sev.criteria must have at least 2 entries". Everything that is not a JSON object -- an HTML error page from a proxy, plain text from a load balancer, an empty body, a JSON array -- leaves all three nil and the message exactly as it was. Parsing happens once, lazily, and #body is still the raw string it always was. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 +++ README.md | 10 ++- lib/ruby_decision_model/errors.rb | 48 ++++++++++- test/error_body_test.rb | 129 ++++++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 test/error_body_test.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..0490414 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## Unreleased + +- `ApiError` parses a JSON error body into `#detail`, `#error_code`, and + `#parsed_body`, and appends the detail to the exception message. Typesafe's + 422 names the offending field and OpenRouter answers with + `{"error": {"code", "message"}}`; both used to arrive as an opaque string on + `#body` under a message that said only "unprocessable entity". A body that + is not a JSON object leaves all three nil and the message unchanged. + ## 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..5562d52 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ 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`, `#headers`, `#detail`, `#error_code`, and `#parsed_body` | | `Unauthorized` | 401 | | `PayloadTooLarge` | 413 | | `UnprocessableEntity` | 422 (never retried) | @@ -183,6 +183,14 @@ return is still accepted and treated as having no headers, which means no | `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` | +When the server sends a JSON error body, `#detail` is the message it carried +and `#error_code` its code, whether the payload nests them under `"error"` (as +OpenRouter does) or puts them at the top level. The detail is appended to the +exception message, so a 422 reads `unprocessable entity: questions.sev.criteria +must have at least 2 entries` rather than `unprocessable entity`. A body that +is not a JSON object — an HTML error page, a proxy's plain text — leaves all +three `nil` and the message unchanged. `#body` is always the raw string. + Status: 0.1.0, API may change. The companion gem `decide` builds decisions and verdicts on top of this client. diff --git a/lib/ruby_decision_model/errors.rb b/lib/ruby_decision_model/errors.rb index d35c51a..e96e35b 100644 --- a/lib/ruby_decision_model/errors.rb +++ b/lib/ruby_decision_model/errors.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "json" + module RubyDecisionModel class Error < StandardError; end @@ -22,10 +24,54 @@ class ApiError < Error attr_reader :status, :body, :headers def initialize(message, status:, body:, headers: {}) - super(message) @status = status @body = body @headers = headers || {} + super(detail.nil? ? message : "#{message}: #{detail}") + end + + # What the server said went wrong, when it said anything parseable. + # Typesafe's 422 names the offending field; OpenRouter answers with + # {"error": {"code": ..., "message": ...}}. Both are plain strings on + # #body, which means every caller that wants the reason writes this. + def detail + return @detail if defined?(@detail) + + @detail = extract("message") || extract("error_description") + end + + def error_code + return @error_code if defined?(@error_code) + + @error_code = extract("code") || extract("type") + end + + # The decoded error body, or nil when it was not a JSON object. + def parsed_body + return @parsed_body if defined?(@parsed_body) + + @parsed_body = begin + decoded = JSON.parse(body.to_s) + decoded.is_a?(Hash) ? decoded : nil + rescue JSON::ParserError, TypeError + nil + end + end + + private + + # Reads a field from the error object, whether the payload nests it under + # "error" or puts it at the top level. + def extract(field) + return nil if parsed_body.nil? + + nested = parsed_body["error"] + value = nested.is_a?(Hash) ? nested[field] : parsed_body[field] + value = parsed_body[field] if value.nil? + return nil unless value.is_a?(String) || value.is_a?(Integer) + + stringified = value.to_s + stringified.empty? ? nil : stringified end end diff --git a/test/error_body_test.rb b/test/error_body_test.rb new file mode 100644 index 0000000..4c2a110 --- /dev/null +++ b/test/error_body_test.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require "test_helper" + +# The server says what went wrong. The exception should say it too. +class ErrorBodyTest < Minitest::Test + def questions + { "urgent" => RubyDecisionModel::Questions.noul("Is this urgent?") } + end + + def raise_for(status, body) + client = RubyDecisionModel::Client.new( + api_key: "k", sleeper: no_sleep, retry: { max_retries: 0 }, + transport: FakeTransport.new([[status, body, {}]]) + ) + client.ask(state: {}, questions: questions) + end + + def error_for(status, body, klass = RubyDecisionModel::ApiError) + assert_raises(klass) { raise_for(status, body) } + end + + # --- the shapes the two providers actually send --- + + def test_a_nested_error_object_is_read + # OpenRouter: {"error": {"code": ..., "message": ...}} + error = error_for(400, JSON.generate("error" => { "code" => "invalid_request", + "message" => "questions is required" })) + + assert_equal "questions is required", error.detail + assert_equal "invalid_request", error.error_code + end + + def test_a_top_level_message_is_read + # Typesafe's 422 details the offending field. + error = error_for(422, JSON.generate("message" => "questions.sev.criteria must have at least 2 entries"), + RubyDecisionModel::UnprocessableEntity) + + assert_equal "questions.sev.criteria must have at least 2 entries", error.detail + end + + def test_the_detail_is_appended_to_the_exception_message + error = error_for(422, JSON.generate("message" => "criteria is required"), + RubyDecisionModel::UnprocessableEntity) + + assert_match(/unprocessable entity: criteria is required/, error.message) + end + + def test_an_integer_code_is_stringified + error = error_for(429, JSON.generate("error" => { "code" => 429, "message" => "slow down" }), + RubyDecisionModel::RateLimited) + + assert_equal "429", error.error_code + end + + def test_the_whole_parsed_body_is_available + error = error_for(422, JSON.generate("message" => "bad", "field" => "questions.sev"), + RubyDecisionModel::UnprocessableEntity) + + assert_equal "questions.sev", error.parsed_body["field"] + end + + # --- bodies that are not that --- + + def test_a_plain_text_body_leaves_the_message_alone + error = error_for(500, "upstream connect error") + + assert_nil error.detail + assert_nil error.error_code + assert_nil error.parsed_body + assert_equal "api error (status 500)", error.message + assert_equal "upstream connect error", error.body + end + + def test_an_html_error_page_does_not_raise + error = error_for(502, "502 Bad Gateway") + + assert_nil error.detail + assert_equal "api error (status 502)", error.message + end + + def test_a_json_array_body_is_not_treated_as_an_error_object + error = error_for(500, "[1,2,3]") + + assert_nil error.parsed_body + assert_nil error.detail + end + + def test_an_empty_body_does_not_raise + error = error_for(500, "") + + assert_nil error.detail + assert_nil error.parsed_body + end + + def test_a_message_that_is_not_a_string_is_ignored + error = error_for(500, JSON.generate("message" => { "nested" => "object" })) + + assert_nil error.detail + assert_equal "api error (status 500)", error.message + end + + def test_an_empty_message_is_ignored + error = error_for(500, JSON.generate("message" => "")) + + assert_nil error.detail + end + + def test_the_body_is_still_the_raw_string + body = JSON.generate("message" => "nope") + error = error_for(500, body) + + assert_equal body, error.body + end + + def test_parsing_happens_once + error = error_for(500, JSON.generate("message" => "nope")) + + assert_same error.parsed_body, error.parsed_body + end + + def test_an_error_built_by_hand_still_works + error = RubyDecisionModel::ApiError.new("boom", status: 500, body: nil) + + assert_equal "boom", error.message + assert_nil error.detail + assert_empty error.headers + end +end