fix: Stop polling an action attempt once the timeout passes

README.md

@@ -163,6 +163,12 @@ When the `wait_for_action_attempt` option is enabled, the SDK:
 - Raises a `Seam::ActionAttemptTimeoutError` if the action attempt is still pending when the `timeout` is reached.
 - Both errors expose an `action_attempt` property.
 
+Polling stops as soon as the `timeout` passes,
+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 `error` and `result` values are only present for their matching status:
 `error` is `nil` unless the `status` is `"error"`,
 and `result` is `nil` unless the `status` is `"success"`.

lib/seam/action_attempt_resolver.rb

@@ -1,9 +1,13 @@
 # frozen_string_literal: true
 
+require_relative "options"
 require_relative "wait_for_action_attempt"
 
 module Seam
   class ActionAttemptResolver
+    TIMEOUT = 5.0
+    POLLING_INTERVAL = 0.5
+
     def self.resolve(action_attempt, client, wait_for_action_attempt)
       return wait_until_resolved(action_attempt, client) if wait_for_action_attempt == true
 
@@ -25,16 +29,18 @@ def self.wait_options(wait_for_action_attempt)
     end
 
     def self.wait_until_resolved(action_attempt, client, timeout: nil, polling_interval: nil)
-      timeout = timeout.nil? ? 5.0 : timeout
-      polling_interval = polling_interval.nil? ? 0.5 : polling_interval
+      timeout = TIMEOUT if timeout.nil?
+      polling_interval = POLLING_INTERVAL if polling_interval.nil?
+      validate_poll_options(timeout, polling_interval)
 
-      time_waiting = 0.0
+      deadline = now + timeout
 
       while action_attempt.status == "pending"
-        sleep(polling_interval)
-        time_waiting += polling_interval
+        remaining = deadline - now
+
+        raise Seam::ActionAttemptTimeoutError.new(action_attempt, timeout) if remaining <= 0
 
-        raise Seam::ActionAttemptTimeoutError.new(action_attempt, timeout) if time_waiting > timeout
+        sleep([polling_interval, remaining].min)
 
         action_attempt = update_action_attempt(action_attempt, client)
       end
@@ -44,11 +50,31 @@ def self.wait_until_resolved(action_attempt, client, timeout: nil, polling_inter
       action_attempt
     end
 
+    def self.validate_poll_options(timeout, polling_interval)
+      unless timeout >= 0
+        raise Http::Options::SeamInvalidOptionsError.new(
+          "The timeout option must not be negative, got #{timeout}"
+        )
+      end
+
+      unless polling_interval > 0
+        raise Http::Options::SeamInvalidOptionsError.new(
+          "The polling_interval option must be greater than zero, got #{polling_interval}"
+        )
+      end
+    end
+
     def self.update_action_attempt(action_attempt, client)
       response = client.get("/action_attempts/get", {action_attempt_id: action_attempt.action_attempt_id})
 
       action_attempt.update_from_response(response.body["action_attempt"])
       action_attempt
     end
+
+    def self.now
+      Process.clock_gettime(Process::CLOCK_MONOTONIC)
+    end
+
+    private_class_method :validate_poll_options, :update_action_attempt, :now
   end
 end

spec/seam_client/action_attempt_polling_spec.rb

@@ -0,0 +1,73 @@
+# frozen_string_literal: true
+
+RSpec.describe Seam::ActionAttemptResolver, 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
+
+  def monotonic_now
+    Process.clock_gettime(Process::CLOCK_MONOTONIC)
+  end
+
+  describe "poll option validation" do
+    before { recorder.respond_with(pending_action_attempt) }
+
+    it "rejects a polling interval of zero before polling" do
+      expect { unlock_door({polling_interval: 0}) }.to raise_error(
+        Seam::Http::Options::SeamInvalidOptionsError,
+        "Seam received invalid options: The polling_interval option must be greater than zero, got 0"
+      )
+
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door"])
+    end
+
+    it "rejects a negative polling interval" do
+      expect { unlock_door({polling_interval: -1}) }.to raise_error(
+        Seam::Http::Options::SeamInvalidOptionsError,
+        "Seam received invalid options: The polling_interval option must be greater than zero, got -1"
+      )
+    end
+
+    it "rejects a negative timeout" do
+      expect { unlock_door({timeout: -1}) }.to raise_error(
+        Seam::Http::Options::SeamInvalidOptionsError,
+        "Seam received invalid options: The timeout option must not be negative, got -1"
+      )
+    end
+
+    it "rejects a NaN timeout" do
+      expect { unlock_door({timeout: Float::NAN}) }.to raise_error(
+        Seam::Http::Options::SeamInvalidOptionsError,
+        "Seam received invalid options: The timeout option must not be negative, got NaN"
+      )
+    end
+  end
+
+  describe "the timeout deadline" do
+    it "polls exactly once when the timeout is shorter than the polling interval" do
+      recorder.respond_with(pending_action_attempt)
+      started = monotonic_now
+
+      expect { unlock_door({timeout: 0.1, polling_interval: 3}) }.to raise_error(Seam::ActionAttemptTimeoutError)
+
+      expect(monotonic_now - started).to be < 3
+      expect(recorder.requests.map(&:path)).to eq(["/locks/unlock_door", "/action_attempts/get"])
+    end
+
+    it "counts the time spent waiting on slow responses" do
+      recorder.respond_with(pending_action_attempt, delay: 0.2)
+      started = monotonic_now
+
+      expect { unlock_door({timeout: 0.5, polling_interval: 0.1}) }.to raise_error(Seam::ActionAttemptTimeoutError)
+
+      expect(monotonic_now - started).to be < 1.5
+      expect(recorder.requests.map(&:path)).to eq(
+        ["/locks/unlock_door", "/action_attempts/get", "/action_attempts/get"]
+      )
+    end
+  end
+end

spec/support/raw_request_recorder.rb

@@ -23,12 +23,14 @@ def initialize
     @endpoint = "http://127.0.0.1:#{@server.addr[1]}"
     @requests = []
     @response_body = "{}"
+    @response_delay = 0
     @thread = Thread.new { serve }
     @thread.abort_on_exception = true
   end
 
-  def respond_with(body)
+  def respond_with(body, delay: 0)
     @response_body = body
+    @response_delay = delay
   end
 
   def stop
@@ -66,6 +68,8 @@ def handle(socket)
     body = content_length.positive? ? socket.read(content_length) : nil
     @requests << RecordedRequest.new(method, target, body)
 
+    sleep(@response_delay) if @response_delay.positive?
+
     socket.write(
       "HTTP/1.1 200 OK\r\n" \
       "Content-Type: application/json\r\n" \