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" \