diff --git a/CHANGELOG.md b/CHANGELOG.md index a350a28..75809a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,12 @@ gem but are what an operator runs against their own image. * An operation can say why it couldn't decode an input: a subclass of `HotCell::UnreadableInput` that defines `self.cause` sends that cause with the `unreadable` failure. `HotCell::Codes::PROTECTED` names the first such cause, `protected`. See `docs/codes.md`. +### HotCell::Client + +#### Added + +* The exception that the client raises for a failed call carries the `HotCell::Failure`, so an application can read the failure's `code` and `cause` without parsing the message. Call `hot_cell_failure` on the exception, or rescue `HotCell::Verdict`, a module that the client extends onto every such exception, to catch it whatever classes the application registered. See [Response codes](docs/codes.md#exception-classes). (#97) + ### ActiveStorage::HotCell::Client #### Fixed diff --git a/docs/client-api.md b/docs/client-api.md index 29a6fbe..eb10f52 100644 --- a/docs/client-api.md +++ b/docs/client-api.md @@ -116,6 +116,10 @@ On failure, `perform_in_hotcell` raises the cell's `permanent:` or `transient:` publishes a `perform.hot_cell` notification. See [Observability](observability.md#per-call-notification). +The exception carries the failure. Call `hot_cell_failure` on it to read the failure's `code` and `cause`, +or rescue `HotCell::Verdict` to catch it whatever classes you registered. See +[Response codes](codes.md#exception-classes). + ### Errors that the client raises for a bad call These classes descend from `HotCell::Error`. The client raises them for a mistake in the call itself, diff --git a/docs/client-legacy.md b/docs/client-legacy.md index d778cfa..7e4b900 100644 --- a/docs/client-legacy.md +++ b/docs/client-legacy.md @@ -59,14 +59,17 @@ end | `HotCell::Client::Legacy::PermanentFailure` | The cell marks the failure permanent. A retry fails the same way. | | `HotCell::Client::Legacy::TransientFailure` | Any other failure. A retry might succeed. | -Both classes have these attributes: +Both classes include `HotCell::Client::Legacy::Verdict`, so you can rescue `Verdict` to catch either one. +Call `hot_cell_failure` on the exception to read the failure, as with `hotcell-client`. See +[Exception classes](codes.md#exception-classes). The failure has these attributes: | Attribute | Description | | --- | --- | | `code` | The failure's code. See [Response codes](codes.md). | -| `error` | A `Hash` with `permanent`, which is `true` or `false`, and whichever of `code`, `cause`, `signal`, `class`, `message`, and `stderr` the cell sent. Each of those is a `String` of at most 512 bytes, with invalid UTF-8 removed. `stderr` keeps its last 512 bytes. | +| `permanent?` | `true` if the cell marked the failure permanent, and `false` otherwise. | +| `cause`, `signal`, `error_class`, `message`, `stderr` | The fields that the cell sent, or `nil`. Each is a `String` of at most 512 bytes, with invalid UTF-8 removed. `stderr` keeps its last 512 bytes. | -The text in `error` comes from the cell. Treat it as untrusted. +The text in the failure comes from the cell. Treat it as untrusted. When the client gets no usable answer, it raises `TransientFailure` with one of these codes: diff --git a/docs/codes.md b/docs/codes.md index 80d05c5..576643a 100644 --- a/docs/codes.md +++ b/docs/codes.md @@ -109,7 +109,8 @@ message: previewers send it for a password-protected PDF. An operation sends a cause by raising a subclass of `HotCell::UnreadableInput` that defines `self.cause`. -See [`unreadable(*classes)`](operation-api.md#unreadableclasses). +See [`unreadable(*classes)`](operation-api.md#unreadableclasses). A caller reads the cause with +`hot_cell_failure`. See [Exception classes](#exception-classes). ## Unknown codes and causes @@ -143,6 +144,18 @@ classification. The exception's message is the failure's `code`, `cause`, error class, and message, joined with `: `, then the `stderr` tail in parentheses when it exists. For example, `killed: crashed (libgomp: ...)`. +To read the failure without parsing the message, call `hot_cell_failure` on the exception. It returns the +`HotCell::Failure`, which has `code`, `cause`, `error_class`, `message`, `signal`, `stderr`, and +`permanent?`. The client extends every exception that it raises for a failed call with the +`HotCell::Verdict` module, so you can also rescue `HotCell::Verdict` whatever classes you registered: + +```ruby +rescue HotCell::Verdict => error + raise unless error.hot_cell_failure.cause == HotCell::Codes::PROTECTED + + lock_placeholder +``` + ## What Active Storage records A permanent verdict is irreversible only if the application records it. In the shipped Active Storage diff --git a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb index bcde396..b37fb45 100644 --- a/hotcell-client-legacy/lib/hot_cell/client/legacy.rb +++ b/hotcell-client-legacy/lib/hot_cell/client/legacy.rb @@ -29,25 +29,54 @@ class Legacy INPUT_MODE = 0o640 OUTPUT_MODE = 0o620 - # Attributes shared by both failure classes. A shared superclass would let `rescue` catch both under one - # name, and the two must never be handled alike: a permanent failure may be written down against a file - # forever, and a transient one must be retried. - module Failure - attr_reader :code, :error + # A cell's verdict on a request that did not succeed, with the readers of hotcell-core's HotCell::Failure. + class Failure + attr_reader :code, :cause, :signal, :error_class, :message, :stderr + + def initialize(fields) + @permanent = fields[:permanent] + @code = fields[:code].to_s + @cause = fields[:cause] + @signal = fields[:signal] + @error_class = fields[:class] + @message = fields[:message] + @stderr = fields[:stderr] + end + + def permanent? + @permanent + end + + def to_h + { code: code, permanent: permanent?, cause: cause, signal: signal, class: error_class, message: message, + stderr: stderr }.reject { |_key, value| value.nil? } + end + + # A cell that sent no code gets none in the message, rather than a leading ": ". + def to_s + [ (code unless code.empty?), cause, error_class, message ].compact.join(": ") + end + end + + # Included in both failure classes, so `rescue Verdict` catches either and `hot_cell_failure` reads the + # failure, as with hotcell-client's HotCell::Verdict. The classes share no superclass, so rescuing one by + # name never catches the other: a permanent failure may be written down against a file forever, and a + # transient one must be retried. + module Verdict + attr_reader :hot_cell_failure - def initialize(error) - @error = error - @code = error[:code].to_s - super([ error[:code], error[:cause], error[:class], error[:message] ].compact.join(": ")) + def initialize(failure) + @hot_cell_failure = failure + super(failure.to_s) end end class PermanentFailure < StandardError - include Failure + include Verdict end class TransientFailure < StandardError - include Failure + include Verdict end class Timeout < StandardError; end @@ -83,8 +112,8 @@ def perform(operation, inputs, outputs, payload = {}) # Built rather than passed to raise, because Ruby 2.x reads a trailing Hash given to raise as its options # and takes `cause` out of it. - error = sanitize(answer["error"]) - raise (error[:permanent] ? PermanentFailure : TransientFailure).new(error) + failure = Failure.new(sanitize(answer["error"])) + raise (failure.permanent? ? PermanentFailure : TransientFailure).new(failure) end private diff --git a/hotcell-client-legacy/test/cell_test.rb b/hotcell-client-legacy/test/cell_test.rb index c80ff48..c858334 100644 --- a/hotcell-client-legacy/test/cell_test.rb +++ b/hotcell-client-legacy/test/cell_test.rb @@ -34,20 +34,20 @@ def test_a_payload_and_a_result_round_trip_with_string_keys def test_a_permanent_failure_raises_permanent_failure error = assert_raises(HotCell::Client::Legacy::PermanentFailure) { client.perform "test.undecodable", [], [] } - assert_equal "unreadable", error.code + assert_equal "unreadable", error.hot_cell_failure.code assert_match "not an image at all", error.message end def test_a_transient_failure_raises_transient_failure error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.broken", [], [] } - assert_equal "failed", error.code + assert_equal "failed", error.hot_cell_failure.code end def test_an_operation_the_cell_does_not_carry_is_transient error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.nonexistent", [], [] } - assert_equal "unsupported", error.code + assert_equal "unsupported", error.hot_cell_failure.code end # The client checks no access mode, so the cell's own check is what refuses a writable input. @@ -59,7 +59,7 @@ def test_a_writable_input_is_refused_by_the_cell_as_invalid end end - assert_equal "invalid", error.code + assert_equal "invalid", error.hot_cell_failure.code end end @@ -71,7 +71,7 @@ def test_a_cell_that_answers_too_late_is_a_transient_timeout end end - assert_equal "timeout", error.code + assert_equal "timeout", error.hot_cell_failure.code assert_operator took, :<, 2 end @@ -80,8 +80,8 @@ def test_a_kill_carries_its_cause client(timeout: 10).perform "test.impatient", [], [] end - assert_equal "killed", error.code - assert_equal "deadline", error.error[:cause] + assert_equal "killed", error.hot_cell_failure.code + assert_equal "deadline", error.hot_cell_failure.cause end def test_the_group_can_read_an_input_and_write_an_output diff --git a/hotcell-client-legacy/test/transport_test.rb b/hotcell-client-legacy/test/transport_test.rb index cb4a7d0..114fa3f 100644 --- a/hotcell-client-legacy/test/transport_test.rb +++ b/hotcell-client-legacy/test/transport_test.rb @@ -9,7 +9,7 @@ def test_a_socket_that_does_not_exist_is_unavailable client = HotCell::Client::Legacy.new(File.join(Dir.tmpdir, "no-such-cell", "work.sock")) error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end # A supervisor that stops calling accept leaves its backlog full, where a blocking connect on Linux would wait @@ -33,7 +33,7 @@ def test_the_deadline_covers_a_connect_to_a_full_backlog end assert call.join(2), "the call waited on a cell whose backlog is full" - assert_includes %w[timeout unavailable], call.value.code + assert_includes %w[timeout unavailable], call.value.hot_cell_failure.code ensure call.kill if call pending.each { |socket| socket.close } @@ -49,7 +49,7 @@ def test_the_deadline_covers_the_whole_answer_rather_than_each_read error = nil took = elapsed { error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } } - assert_equal "timeout", error.code + assert_equal "timeout", error.hot_cell_failure.code assert_operator took, :<, 2 end end @@ -73,7 +73,7 @@ def test_the_deadline_covers_a_send_the_peer_never_reads end assert call.join(2), "the call waited on a peer that never reads" - assert_equal "timeout", call.value.code + assert_equal "timeout", call.value.hot_cell_failure.code ensure call.kill if call peer.kill @@ -87,7 +87,7 @@ def test_an_answer_that_passes_the_size_limit_without_a_newline_is_unavailable with_peer(oversize) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end end @@ -96,7 +96,7 @@ def test_an_answer_cut_off_before_its_newline_is_unavailable with_peer(partial) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end end @@ -104,7 +104,7 @@ def test_a_connection_closed_with_no_answer_is_unavailable with_peer(lambda { |connection| connection.close }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end end @@ -112,7 +112,7 @@ def test_an_answer_that_is_not_json_is_unavailable with_peer(lambda { |connection| connection.write "not json\n" }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end end @@ -120,7 +120,7 @@ def test_ok_must_be_true_rather_than_truthy with_peer(lambda { |connection| connection.write %({"v":1,"ok":"true","result":{}}\n) }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal "unavailable", error.code + assert_equal "unavailable", error.hot_cell_failure.code end end @@ -129,7 +129,41 @@ def test_the_permanent_flag_decides_even_for_a_code_this_client_has_never_heard_ with_peer(lambda { |connection| connection.write answer }) do |client| error = assert_raises(HotCell::Client::Legacy::PermanentFailure) { client.perform "test.echo", [], [] } - assert_equal "novel", error.code + assert_equal "novel", error.hot_cell_failure.code + end + end + + def test_the_raised_exception_carries_the_failure_itself + answer = %({"v":1,"ok":false,"error":{"code":"killed","permanent":false,"cause":"deadline","signal":"KILL",) + + %("class":"Vips::Error","message":"too slow","stderr":"fatal"}}\n) + with_peer(lambda { |connection| connection.write answer }) do |client| + error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } + failure = error.hot_cell_failure + + assert_equal [ "killed", "deadline", "KILL", "Vips::Error", "too slow", "fatal" ], + [ failure.code, failure.cause, failure.signal, failure.error_class, failure.message, failure.stderr ] + assert_equal false, failure.permanent? + end + end + + def test_a_failure_with_no_code_leaves_it_out_of_the_message + answer = %({"v":1,"ok":false,"error":{"cause":"protected","message":"locked"}}\n) + with_peer(lambda { |connection| connection.write answer }) do |client| + error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } + + assert_equal "protected: locked", error.message + end + end + + def test_a_verdict_can_be_rescued_as_one + answer = %({"v":1,"ok":false,"error":{"code":"unreadable","permanent":true,"cause":"protected"}}\n) + with_peer(lambda { |connection| connection.write answer }) do |client| + begin + client.perform "test.echo", [], [] + flunk "expected a raise" + rescue HotCell::Client::Legacy::Verdict => error + assert_equal "protected", error.hot_cell_failure.cause + end end end @@ -139,7 +173,7 @@ def test_a_failure_is_permanent_only_when_the_flag_is_true with_peer(lambda { |connection| connection.write answer }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal false, error.error[:permanent] + assert_equal false, error.hot_cell_failure.permanent? end end end @@ -169,7 +203,7 @@ def test_a_failures_text_is_valid_utf_8_whatever_the_cell_sent with_peer(lambda { |connection| connection.write answer }) do |client| error = assert_raises(HotCell::Client::Legacy::PermanentFailure) { client.perform "test.echo", [], [] } - assert error.error[:message].valid_encoding? + assert error.hot_cell_failure.message.valid_encoding? assert error.message.valid_encoding? assert_match "bad bytes", error.message end @@ -180,8 +214,8 @@ def test_a_failure_carries_only_the_protocols_fields with_peer(lambda { |connection| connection.write answer }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal [ :permanent, :code, :cause ], error.error.keys - assert error.error[:cause].valid_encoding? + assert_equal [ :code, :permanent, :cause ], error.hot_cell_failure.to_h.keys + assert error.hot_cell_failure.cause.valid_encoding? end end @@ -190,9 +224,9 @@ def test_a_failures_text_is_capped_and_stderr_keeps_its_tail with_peer(lambda { |connection| connection.write answer }) do |client| error = assert_raises(HotCell::Client::Legacy::TransientFailure) { client.perform "test.echo", [], [] } - assert_equal HotCell::Client::Legacy::MAX_FIELD_BYTES, error.error[:message].bytesize - assert_equal HotCell::Client::Legacy::MAX_FIELD_BYTES, error.error[:stderr].bytesize - assert error.error[:stderr].end_with?("fatal") + assert_equal HotCell::Client::Legacy::MAX_FIELD_BYTES, error.hot_cell_failure.message.bytesize + assert_equal HotCell::Client::Legacy::MAX_FIELD_BYTES, error.hot_cell_failure.stderr.bytesize + assert error.hot_cell_failure.stderr.end_with?("fatal") end end diff --git a/hotcell-client/lib/hot_cell/client.rb b/hotcell-client/lib/hot_cell/client.rb index aa69658..b9e3e54 100644 --- a/hotcell-client/lib/hot_cell/client.rb +++ b/hotcell-client/lib/hot_cell/client.rb @@ -200,7 +200,7 @@ def raise_for(response, cell) return if response.ok? failure = response.failure - error = cell.exception_for(failure).new(failure.to_s) + error = Verdict.wrap(cell.exception_for(failure).new(failure.to_s), failure) cell.report_contract_skew error if failure.code == "protocol" raise error diff --git a/hotcell-client/lib/hot_cell/failures.rb b/hotcell-client/lib/hot_cell/failures.rb index 4c3d7c7..ca5d267 100644 --- a/hotcell-client/lib/hot_cell/failures.rb +++ b/hotcell-client/lib/hot_cell/failures.rb @@ -21,4 +21,17 @@ class PermanentFailure < StandardError; end # classification, so a later tidying pass that gives both a common ancestor silently turns every retryable # failure into a permanent one. class TransientFailure < StandardError; end + + # Extended onto every exception the client raises, so an application can read the failure's code, + # cause and permanence without parsing the message — and rescue HotCell::Verdict itself, whatever + # classes the registration injected. + module Verdict + attr_reader :hot_cell_failure + + def self.wrap(error, failure) + error.extend(self) + error.instance_variable_set(:@hot_cell_failure, failure) + error + end + end end diff --git a/hotcell-client/test/classification_test.rb b/hotcell-client/test/classification_test.rb index fb04f1a..c217807 100644 --- a/hotcell-client/test/classification_test.rb +++ b/hotcell-client/test/classification_test.rb @@ -74,6 +74,27 @@ def test_the_raised_message_carries_enough_to_re_decide_on_later assert_match "bad PNG header", error.message end + def test_a_verdict_can_be_rescued_as_one + register_with failed(code: "capacity") + + begin + Anything.perform_in_hotcell [], [], {} + flunk "expected a raise" + rescue HotCell::Verdict => error + assert_equal "capacity", error.hot_cell_failure.code + end + end + + def test_the_raised_exception_carries_the_failure_itself + register_with failed(code: "killed", cause: "deadline") + + error = assert_raises(TemporarilyUnavailable) { Anything.perform_in_hotcell [], [], {} } + + assert_equal "killed", error.hot_cell_failure.code + assert_equal "deadline", error.hot_cell_failure.cause + refute_predicate error.hot_cell_failure, :permanent? + end + # Applications rescue broadly around representations, so "raise" is indistinguishable from "placeholder" # and contract skew is otherwise invisible. An application running several clients against several # independently-booted cells needs to know which one skewed.