refactor(error): use idiomatic Ruby predicates

examples/error.rb

@@ -6,7 +6,7 @@
   Wreq.get("not-a-valid-url")
 rescue Wreq::Error => error
   puts "#{error.class}: #{error.message}"
-  puts "builder: #{error.is_builder}"
+  puts "builder: #{error.builder?}"
   puts "uri: #{error.uri.inspect}"
   puts "status: #{error.status.inspect}"
 end

lib/wreq_ruby/error.rb

@@ -5,10 +5,10 @@ module Wreq
     # Base class for wreq-ruby runtime errors.
     #
     # Error remains a RuntimeError so existing rescue handlers keep working.
-    # Its subclass records one error category. The `is_*` methods preserve every
-    # predicate reported by the native `wreq::Error`, so more than one can be
-    # true. For example, a request timeout raises TimeoutError while both
-    # `is_timeout` and `is_request` are true.
+    # Its subclass records one primary category. Predicate methods retain every
+    # classification reported by the native `wreq::Error`, so more than one can
+    # be true. For example, a request timeout raises TimeoutError while both
+    # `timeout?` and `request?` return true.
     #
     # A native kind such as BodyError, TlsError, or StatusError takes precedence
     # over details found in its cause chain. Native request errors are then
@@ -22,7 +22,7 @@ module Wreq
     #     Wreq.get("not-a-valid-url")
     #   rescue Wreq::Error => error
     #     warn "#{error.class}: #{error.message}"
-    #     warn "invalid request" if error.is_builder
+    #     warn "invalid request" if error.builder?
     #   end
     class Error < RuntimeError
       # Get the URI recorded by the native error.
@@ -40,57 +40,57 @@ class Error < RuntimeError
       attr_reader :status
 
       # @return [Boolean] Whether the native error came from a builder
-      def is_builder
+      def builder?
       end
 
       # @return [Boolean] Whether the native error came from redirect handling
-      def is_redirect
+      def redirect?
       end
 
       # @return [Boolean] Whether the native error represents an HTTP status
-      def is_status
+      def status?
       end
 
       # A request timeout uses TimeoutError even when this is also a connection
       # or proxy connection error.
       #
       # @return [Boolean] Whether the native error is related to a timeout
-      def is_timeout
+      def timeout?
       end
 
       # This may be true on connection and timeout subclasses because those
       # errors occur while sending a request.
       #
       # @return [Boolean] Whether the native error is related to a request
-      def is_request
+      def request?
       end
 
       # @return [Boolean] Whether the native error is related to connecting
-      def is_connect
+      def connection?
       end
 
       # @return [Boolean] Whether the native error is related to a proxy connection
-      def is_proxy_connect
+      def proxy_connection?
       end
 
       # @return [Boolean] Whether the native error is a connection reset
-      def is_connection_reset
+      def connection_reset?
       end
 
       # @return [Boolean] Whether the native error is related to a body
-      def is_body
+      def body?
       end
 
       # @return [Boolean] Whether the native error is related to TLS
-      def is_tls
+      def tls?
       end
 
       # @return [Boolean] Whether the native error is related to decoding
-      def is_decode
+      def decoding?
       end
 
       # @return [Boolean] Whether the native error is related to an upgrade
-      def is_upgrade
+      def upgrade?
       end
     end
 
@@ -139,7 +139,7 @@ class ForkError < Error; end
     # Raised when the client cannot connect to the destination server.
     #
     # If the native error reports both a destination connection failure and a
-    # timeout, Wreq::TimeoutError is raised and `is_connect` remains true. A
+    # timeout, Wreq::TimeoutError is raised and `connection?` remains true. A
     # system proxy or VPN that accepts the connection but never responds may
     # instead appear as a general request timeout.
     #
@@ -155,7 +155,7 @@ class ConnectionError < Error; end
     # Raised when the client cannot connect to the configured proxy.
     #
     # If the native error reports both a proxy connection failure and a timeout,
-    # Wreq::TimeoutError is raised and `is_proxy_connect` remains true.
+    # Wreq::TimeoutError is raised and `proxy_connection?` remains true.
     #
     # @example Handle a proxy connection failure
     #   begin
@@ -242,8 +242,8 @@ class RedirectError < Error; end
     # Raised when a request operation exceeds its timeout.
     #
     # This includes destination and proxy connection timeouts when the native
-    # error reports them as timeouts. Check `is_connect` or `is_proxy_connect`
-    # to see whether the native error also identifies that phase. `is_request`
+    # error reports them as timeouts. Check `connection?` or `proxy_connection?`
+    # to see whether the native error also identifies that phase. `request?`
     # can be true on the same error.
     #
     # @example Handle a request timeout

src/error.rs

@@ -49,7 +49,8 @@ macro_rules! define_error_mapping {
     (
         $(
             $predicate:ident [$role:ident]:
-                $method:ident => $class:ident $(($ruby_name:literal))?
+                $native_method:ident as $ruby_method:ident
+                => $class:ident $(($ruby_name:literal))?
         ),+ $(,)?
     ) => {
         /// How a native predicate participates in Ruby exception classification.
@@ -85,7 +86,7 @@ macro_rules! define_error_mapping {
             /// Evaluate this predicate before the native error is consumed.
             fn matches_wreq(self, error: &wreq::Error) -> bool {
                 match self {
-                    $(Self::$predicate => error.$method(),)+
+                    $(Self::$predicate => error.$native_method(),)+
                 }
             }
 
@@ -105,15 +106,18 @@ macro_rules! define_error_mapping {
         )+
 
         $(
-            fn $method(rb_self: RObject) -> Result<bool, MagnusError> {
+            fn $native_method(rb_self: RObject) -> Result<bool, MagnusError> {
                 error_has_predicate(rb_self, ErrorPredicate::$predicate)
             }
         )+
 
-        /// Define the native wreq predicate methods on Wreq::Error.
+        /// Define idiomatic Ruby predicate methods on `Wreq::Error`.
         fn include_error_predicates(class: ExceptionClass) -> Result<(), MagnusError> {
             $(
-                class.define_method(stringify!($method), magnus::method!($method, 0))?;
+                class.define_method(
+                    concat!(stringify!($ruby_method), "?"),
+                    magnus::method!($native_method, 0),
+                )?;
             )+
             Ok(())
         }
@@ -136,22 +140,25 @@ macro_rules! define_error_mapping {
 
 // wreq keeps its error kind private. Keep its mutually exclusive kind predicates
 // separate from request details, which inspect the source chain and may overlap.
+// Each entry maps the native method before `as` to the Ruby predicate after it.
 // Entries within each role are classified from top to bottom.
 define_error_mapping! {
-    Builder [NativeKind]: is_builder => BUILDER_ERROR("BuilderError"),
-    Body [NativeKind]: is_body => BODY_ERROR("BodyError"),
-    Tls [NativeKind]: is_tls => TLS_ERROR("TlsError"),
-    Decode [NativeKind]: is_decode => DECODING_ERROR("DecodingError"),
-    Redirect [NativeKind]: is_redirect => REDIRECT_ERROR("RedirectError"),
-    Status [NativeKind]: is_status => STATUS_ERROR("StatusError"),
-    Upgrade [NativeKind]: is_upgrade => WREQ_ERROR,
-    Request [NativeKind]: is_request => REQUEST_ERROR("RequestError"),
+    Builder [NativeKind]: is_builder as builder => BUILDER_ERROR("BuilderError"),
+    Body [NativeKind]: is_body as body => BODY_ERROR("BodyError"),
+    Tls [NativeKind]: is_tls as tls => TLS_ERROR("TlsError"),
+    Decode [NativeKind]: is_decode as decoding => DECODING_ERROR("DecodingError"),
+    Redirect [NativeKind]: is_redirect as redirect => REDIRECT_ERROR("RedirectError"),
+    Status [NativeKind]: is_status as status => STATUS_ERROR("StatusError"),
+    Upgrade [NativeKind]: is_upgrade as upgrade => WREQ_ERROR,
+    Request [NativeKind]: is_request as request => REQUEST_ERROR("RequestError"),
     ConnectionReset [RequestDetail]:
-        is_connection_reset => CONNECTION_RESET_ERROR("ConnectionResetError"),
-    Timeout [RequestDetail]: is_timeout => TIMEOUT_ERROR("TimeoutError"),
+        is_connection_reset as connection_reset
+        => CONNECTION_RESET_ERROR("ConnectionResetError"),
+    Timeout [RequestDetail]: is_timeout as timeout => TIMEOUT_ERROR("TimeoutError"),
     ProxyConnect [RequestDetail]:
-        is_proxy_connect => PROXY_CONNECTION_ERROR("ProxyConnectionError"),
-    Connect [RequestDetail]: is_connect => CONNECTION_ERROR("ConnectionError"),
+        is_proxy_connect as proxy_connection
+        => PROXY_CONNECTION_ERROR("ProxyConnectionError"),
+    Connect [RequestDetail]: is_connect as connection => CONNECTION_ERROR("ConnectionError"),
 }
 
 /// Native predicates retained after consuming a wreq error.

test/error_hierarchy_test.rb

@@ -19,18 +19,18 @@ class ErrorHierarchyTest < Minitest::Test
     BuilderError
   ].freeze
   NATIVE_ERROR_PREDICATES = %i[
-    is_builder
-    is_redirect
-    is_status
-    is_timeout
-    is_request
-    is_connect
-    is_proxy_connect
-    is_connection_reset
-    is_body
-    is_tls
-    is_decode
-    is_upgrade
+    builder?
+    redirect?
+    status?
+    timeout?
+    request?
+    connection?
+    proxy_connection?
+    connection_reset?
+    body?
+    tls?
+    decoding?
+    upgrade?
   ].freeze
 
   def test_regular_errors_share_stable_root
@@ -57,9 +57,9 @@ def test_root_and_specific_errors_can_be_rescued
     end
 
     assert_instance_of Wreq::BuilderError, root_error
-    assert root_error.is_builder
+    assert_predicate root_error, :builder?
     assert_nil root_error.status
-    assert_equal [:is_builder], active_native_predicates(root_error)
+    assert_equal [:builder?], active_native_predicates(root_error)
     assert_raises(Wreq::BuilderError) { Wreq.get("not-a-valid-url") }
   end
 
@@ -75,9 +75,9 @@ def test_native_error_predicates_are_not_mutually_exclusive
     with_hanging_server do |url, _accepted|
       error = assert_raises(Wreq::TimeoutError) { client.get(url, timeout: 1) }
 
-      assert error.is_timeout
-      assert error.is_request
-      assert_equal %i[is_timeout is_request], active_native_predicates(error)
+      assert_predicate error, :timeout?
+      assert_predicate error, :request?
+      assert_equal %i[timeout? request?], active_native_predicates(error)
     end
   end
 
@@ -137,8 +137,8 @@ def test_raise_for_status_exposes_status_without_consuming_body
         assert_equal status, error.status
         assert_equal response.url, error.uri
         assert_predicate error.uri, :frozen?
-        assert error.is_status
-        assert_equal [:is_status], active_native_predicates(error)
+        assert_predicate error, :status?
+        assert_equal [:status?], active_native_predicates(error)
         refute_respond_to error, :kind
         refute_respond_to error, :retryable?
         refute_includes error.message, "response-secret"
@@ -203,7 +203,7 @@ def test_option_context_preserves_native_error_metadata
     error = assert_raises(Wreq::BuilderError) { Wreq::Client.new(proxy: "://") }
 
     assert_includes error.message, ":proxy"
-    assert_equal [:is_builder], active_native_predicates(error)
+    assert_equal [:builder?], active_native_predicates(error)
   end
 
   private