Add HotCell::Verdict so a raised exception carries its failure

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

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,

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:
 

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

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

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

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,31 +96,31 @@ 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
 
   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
 
   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
 
   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
 

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

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