diff --git a/Gemfile.lock b/Gemfile.lock index 82a2ed3..c728c23 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - stekker_zaptec (1.3.0) + stekker_zaptec (1.4.0) activemodel activesupport faraday diff --git a/lib/zaptec/client.rb b/lib/zaptec/client.rb index 5856125..d872e6f 100644 --- a/lib/zaptec/client.rb +++ b/lib/zaptec/client.rb @@ -4,6 +4,7 @@ class Client USER_ROLE = 1 OWNER_ROLE = 2 TOKENS_CACHE_KEY = "zaptec.auth.tokens".freeze + DEFAULT_HIERARCHY_MAX_WAIT = 30.seconds attr_reader :credentials @@ -72,11 +73,21 @@ def get_installation(installation_id) end # https://api.zaptec.com/help/index.html#/Installation/get_api_installation__id__hierarchy - def get_installation_hierarchy(installation_id) + # + # Zaptec occasionally returns 204 while an installation is still being provisioned. + # Retries with exponential backoff, doubling from 2 s, until the cumulative wait + # would exceed `max_wait` — at which point one final attempt runs at exactly + # `max_wait` seconds. `max_wait: 0` means a single attempt, no retries. + def get_installation_hierarchy(installation_id, max_wait: DEFAULT_HIERARCHY_MAX_WAIT) response = get("/api/installation/#{installation_id}/hierarchy") - - if response.status == 204 - sleep 2 + slept = 0 + next_attempt_at = 2 + + while response.status == 204 && slept < max_wait + target = [next_attempt_at, max_wait].min + sleep(target - slept) + slept = target + next_attempt_at *= 2 response = get("/api/installation/#{installation_id}/hierarchy") end diff --git a/lib/zaptec/version.rb b/lib/zaptec/version.rb index f6727e6..4391d74 100644 --- a/lib/zaptec/version.rb +++ b/lib/zaptec/version.rb @@ -1,3 +1,3 @@ module Zaptec - VERSION = "1.3.0".freeze + VERSION = "1.4.0".freeze end diff --git a/spec/zaptec/client_spec.rb b/spec/zaptec/client_spec.rb index 394b5d7..0a33d17 100644 --- a/spec/zaptec/client_spec.rb +++ b/spec/zaptec/client_spec.rb @@ -506,7 +506,7 @@ ) end - it "retries once after a 204 response" do + it "retries with exponential backoff after 204 responses until one succeeds" do hierarchy_body = { Id: "2bbec6f9-c3ce-4edf-a72f-b1b2a663c6ba", Name: "Stekker test", @@ -517,6 +517,7 @@ WebMock::API .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") .to_return( + { status: 204, body: "", headers: {} }, { status: 204, body: "", headers: {} }, { status: 200, body: hierarchy_body, headers: { "Content-Type": "application/json" } }, ) @@ -524,15 +525,54 @@ token_cache = build_token_cache("T123") client = Zaptec::Client.new(username: "zap", password: "tec", token_cache:) - allow(client).to receive(:sleep) + sleeps = [] + allow(client).to receive(:sleep) { |duration| sleeps << duration } installation_hierarchy = client.get_installation_hierarchy("I123") expect(installation_hierarchy.name).to eq("Stekker test") - expect(client).to have_received(:sleep).with(2) + expect(sleeps).to eq([2, 2]) + end + + it "keeps retrying until the cumulative wait reaches max_wait" do + WebMock::API + .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") + .to_return(status: 204, body: "", headers: {}) + + token_cache = build_token_cache("T123") + client = Zaptec::Client.new(username: "zap", password: "tec", token_cache:) + + sleeps = [] + allow(client).to receive(:sleep) { |duration| sleeps << duration } + + expect { client.get_installation_hierarchy("I123", max_wait: 50) } + .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + # Attempts happen at cumulative t = 0, 2, 4, 8, 16, 32, 50 seconds + expect(sleeps).to eq([2, 2, 4, 8, 16, 18]) + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(7) + end + + it "caps the final sleep so the last attempt happens at exactly max_wait seconds" do + WebMock::API + .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") + .to_return(status: 204, body: "", headers: {}) + + token_cache = build_token_cache("T123") + client = Zaptec::Client.new(username: "zap", password: "tec", token_cache:) + + sleeps = [] + allow(client).to receive(:sleep) { |duration| sleeps << duration } + + expect { client.get_installation_hierarchy("I123", max_wait: 5) } + .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + # Attempts at cumulative t = 0, 2, 4, 5 + expect(sleeps).to eq([2, 2, 1]) + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(4) end - it "raises RequestFailed after two consecutive 204 responses" do + it "accepts an ActiveSupport::Duration for max_wait" do WebMock::API .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") .to_return(status: 204, body: "", headers: {}) @@ -540,10 +580,31 @@ token_cache = build_token_cache("T123") client = Zaptec::Client.new(username: "zap", password: "tec", token_cache:) - allow(client).to receive(:sleep) + sleeps = [] + allow(client).to receive(:sleep) { |duration| sleeps << duration } - expect { client.get_installation_hierarchy("I123") } + expect { client.get_installation_hierarchy("I123", max_wait: 5.seconds) } .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + expect(sleeps).to eq([2, 2, 1]) + end + + it "makes a single attempt with no retries when max_wait is zero" do + WebMock::API + .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") + .to_return(status: 204, body: "", headers: {}) + + token_cache = build_token_cache("T123") + client = Zaptec::Client.new(username: "zap", password: "tec", token_cache:) + + sleeps = [] + allow(client).to receive(:sleep) { |duration| sleeps << duration } + + expect { client.get_installation_hierarchy("I123", max_wait: 0) } + .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + expect(sleeps).to be_empty + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").once end end