fix: Validate the wait_for_action_attempt option

README.md

@@ -168,6 +168,8 @@ and every wait polls at least once,
 even when the `timeout` is shorter than the `polling_interval`.
 The `timeout` must not be negative,
 and the `polling_interval` must be greater than zero.
+The option accepts `true`, `false`, or a hash with `timeout` and `polling_interval` keys;
+any other value raises a `Seam::Http::Options::SeamInvalidOptionsError`.
 
 The `error` and `result` values are only present for their matching status:
 `error` is `nil` unless the `status` is `"error"`,

lib/seam/action_attempt_resolver.rb

@@ -1,23 +1,65 @@
 # frozen_string_literal: true
 
+require_relative "deep_hash_accessor"
 require_relative "options"
 require_relative "wait_for_action_attempt"
 
 module Seam
   class ActionAttemptResolver
     TIMEOUT = 5.0
     POLLING_INTERVAL = 0.5
+    WAIT_FOR_ACTION_ATTEMPT_OPTION_KEYS = %i[timeout polling_interval].freeze
 
     def self.resolve(action_attempt, client, wait_for_action_attempt)
+      validate_wait_for_action_attempt(wait_for_action_attempt)
+
       return wait_until_resolved(action_attempt, client) if wait_for_action_attempt == true
 
       options = wait_options(wait_for_action_attempt)
       return action_attempt if options.nil?
 
+      options = options.transform_keys(&:to_sym)
+
       wait_until_resolved(action_attempt, client, timeout: options[:timeout],
         polling_interval: options[:polling_interval])
     end
 
+    def self.normalize_wait_for_action_attempt(wait_for_action_attempt)
+      return true if wait_for_action_attempt.nil?
+
+      validate_wait_for_action_attempt(wait_for_action_attempt)
+
+      wait_for_action_attempt
+    end
+
+    def self.validate_wait_for_action_attempt(wait_for_action_attempt)
+      return if wait_for_action_attempt == true || wait_for_action_attempt == false
+
+      options = wait_options(wait_for_action_attempt)
+
+      if options.nil?
+        raise Http::Options::SeamInvalidOptionsError.new(
+          "The wait_for_action_attempt option must be true, false, or a hash with " \
+          "\"timeout\" and \"polling_interval\" keys, got #{wait_for_action_attempt.class}"
+        )
+      end
+
+      options.each do |key, option|
+        unless WAIT_FOR_ACTION_ATTEMPT_OPTION_KEYS.include?(key.to_s.to_sym)
+          raise Http::Options::SeamInvalidOptionsError.new(
+            "The wait_for_action_attempt option got an unknown key #{key.inspect}, " \
+            "expected \"timeout\" or \"polling_interval\""
+          )
+        end
+
+        if option == true || option == false || !option.is_a?(Numeric)
+          raise Http::Options::SeamInvalidOptionsError.new(
+            "The wait_for_action_attempt option #{key.inspect} must be a number, got #{option.class}"
+          )
+        end
+      end
+    end
+
     # The client wraps its defaults in a DeepHashAccessor, so the hash form of
     # this option reaches here as an accessor when it comes from the client
     # and as a plain Hash when it comes from the method call.

lib/seam/http_single_workspace.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative "action_attempt_resolver"
 require_relative "request"
 require_relative "parse_options"
 require_relative "resources/index"
@@ -41,7 +42,8 @@ def initialize(client: nil, api_key: OPTION_NOT_PROVIDED, personal_access_token:
           )
         end
 
-        wait_for_action_attempt = true if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
+        wait_for_action_attempt = nil if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
+        wait_for_action_attempt = Seam::ActionAttemptResolver.normalize_wait_for_action_attempt(wait_for_action_attempt)
         @defaults = Seam::DeepHashAccessor.new({"wait_for_action_attempt" => wait_for_action_attempt})
 
         # A client carries its own endpoint and authorization, so the auth

lib/seam/http_without_workspace.rb

@@ -1,5 +1,6 @@
 # frozen_string_literal: true
 
+require_relative "action_attempt_resolver"
 require_relative "request"
 require_relative "parse_options"
 require_relative "version"
@@ -35,7 +36,8 @@ def initialize(client: nil, personal_access_token: OPTION_NOT_PROVIDED, endpoint
           )
         end
 
-        wait_for_action_attempt = true if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
+        wait_for_action_attempt = nil if wait_for_action_attempt.equal?(OPTION_NOT_PROVIDED)
+        wait_for_action_attempt = Seam::ActionAttemptResolver.normalize_wait_for_action_attempt(wait_for_action_attempt)
         @wait_for_action_attempt = wait_for_action_attempt
         @defaults = {"wait_for_action_attempt" => wait_for_action_attempt}
 

spec/seam_client/wait_for_action_attempt_option_spec.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+RSpec.describe "the wait_for_action_attempt option" do
+  let(:api_key) { "seam_some_api_key" }
+
+  def invalid_options_error(message)
+    raise_error(Seam::Http::Options::SeamInvalidOptionsError, "Seam received invalid options: #{message}")
+  end
+
+  describe "on the client" do
+    it "rejects a value that is neither a boolean nor a hash" do
+      expect { Seam.new(api_key: api_key, wait_for_action_attempt: 1) }.to invalid_options_error(
+        'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got Integer'
+      )
+    end
+
+    it "rejects a value that is neither a boolean nor a hash on the without-workspace client" do
+      expect do
+        Seam::Http::WithoutWorkspace.new(personal_access_token: "seam_at_token", wait_for_action_attempt: "true")
+      end.to invalid_options_error(
+        'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got String'
+      )
+    end
+
+    it "rejects an unknown option key" do
+      expect { Seam.new(api_key: api_key, wait_for_action_attempt: {poll_interval: 1}) }.to invalid_options_error(
+        'The wait_for_action_attempt option got an unknown key :poll_interval, expected "timeout" or "polling_interval"'
+      )
+    end
+
+    it "rejects a non-numeric option value" do
+      expect { Seam.new(api_key: api_key, wait_for_action_attempt: {timeout: "5"}) }.to invalid_options_error(
+        "The wait_for_action_attempt option :timeout must be a number, got String"
+      )
+    end
+
+    it "rejects a boolean option value" do
+      expect { Seam.new(api_key: api_key, wait_for_action_attempt: {timeout: true}) }.to invalid_options_error(
+        "The wait_for_action_attempt option :timeout must be a number, got TrueClass"
+      )
+    end
+
+    it "treats nil as the default" do
+      seam = Seam.new(api_key: api_key, wait_for_action_attempt: nil)
+
+      expect(seam.defaults.wait_for_action_attempt).to be true
+    end
+
+    it "accepts string keys" do
+      seam = Seam.new(api_key: api_key, wait_for_action_attempt: {"timeout" => 1, "polling_interval" => 0.1})
+
+      expect(seam.defaults.wait_for_action_attempt.to_h).to eq("timeout" => 1, "polling_interval" => 0.1)
+    end
+  end
+
+  describe "on a route call", recorder: true do
+    def pending_action_attempt
+      {action_attempt: {action_attempt_id: "attempt-1", action_type: "UNLOCK_DOOR", status: "pending"}}.to_json
+    end
+
+    def unlock_door(wait_for_action_attempt)
+      seam.locks.unlock_door(device_id: "device-1", wait_for_action_attempt: wait_for_action_attempt)
+    end
+
+    before { recorder.respond_with(pending_action_attempt) }
+
+    it "rejects a truthy value that is not true before polling" do
+      expect { unlock_door(1) }.to invalid_options_error(
+        'The wait_for_action_attempt option must be true, false, or a hash with "timeout" and "polling_interval" keys, got Integer'
+      )
+
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door"])
+    end
+
+    it "rejects an unknown option key before polling" do
+      expect { unlock_door({poll_interval: 1}) }.to invalid_options_error(
+        'The wait_for_action_attempt option got an unknown key :poll_interval, expected "timeout" or "polling_interval"'
+      )
+
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door"])
+    end
+
+    it "honors string option keys" do
+      expect { unlock_door({"timeout" => 0.1, "polling_interval" => 3}) }.to raise_error(Seam::ActionAttemptTimeoutError)
+
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door", "/action_attempts/get"])
+    end
+
+    it "honors string option keys set on the client" do
+      seam = Seam.new(api_key: "seam_some_api_key", endpoint: recorder.endpoint,
+        wait_for_action_attempt: {"timeout" => 0.1, "polling_interval" => 3})
+
+      expect { seam.locks.unlock_door(device_id: "device-1") }.to raise_error(Seam::ActionAttemptTimeoutError)
+
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door", "/action_attempts/get"])
+    end
+  end
+end