From 7e1a2d5c8c37442c2eb0491f875def4a1783a5d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 20:19:51 +0000 Subject: [PATCH] fix: Stop polling an action attempt once the timeout passes The wait loop advanced its clock only by the polling interval, so a polling_interval of zero never timed out and polled without pause, the time spent waiting on the API never counted against the timeout, and every wait overran the timeout by one interval. Poll against a monotonic deadline, sleeping the shorter of the interval and the remaining budget, so the wait still polls once when the timeout is shorter than the interval and stops as soon as the deadline passes. Reject a negative timeout and a polling interval that is not greater than zero with the SDK's invalid options error before the first poll. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01SQW83gyXeUG61RDrHEky97 --- README.md | 6 ++ lib/seam/action_attempt_resolver.rb | 38 ++++++++-- .../action_attempt_polling_spec.rb | 73 +++++++++++++++++++ spec/support/raw_request_recorder.rb | 6 +- 4 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 spec/seam_client/action_attempt_polling_spec.rb diff --git a/README.md b/README.md index 2259b6c..00fc061 100644 --- a/README.md +++ b/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"`. diff --git a/lib/seam/action_attempt_resolver.rb b/lib/seam/action_attempt_resolver.rb index 394563e..5c53787 100644 --- a/lib/seam/action_attempt_resolver.rb +++ b/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 diff --git a/spec/seam_client/action_attempt_polling_spec.rb b/spec/seam_client/action_attempt_polling_spec.rb new file mode 100644 index 0000000..2cf34f8 --- /dev/null +++ b/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 diff --git a/spec/support/raw_request_recorder.rb b/spec/support/raw_request_recorder.rb index c5a3cc2..4fd4a4f 100644 --- a/spec/support/raw_request_recorder.rb +++ b/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" \