From dd5a1a488cb3f5ece84dfabb72ebf61011fea0a1 Mon Sep 17 00:00:00 2001 From: Martijn Versluis Date: Fri, 3 Jul 2026 16:31:29 +0200 Subject: [PATCH 1/2] Let callers configure the installation-hierarchy retry backoff --- Gemfile.lock | 2 +- lib/zaptec/client.rb | 13 +++++++--- lib/zaptec/version.rb | 2 +- spec/zaptec/client_spec.rb | 49 +++++++++++++++++++++++++++++++++++--- 4 files changed, 58 insertions(+), 8 deletions(-) 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..1ff68ba 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_RETRY_DELAYS = [2, 4].freeze attr_reader :credentials @@ -72,11 +73,17 @@ 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. + # `retry_delays` is the list of pauses (in seconds) between retries; its length + # determines the number of retries. `[]` means a single attempt, no retries. + def get_installation_hierarchy(installation_id, retry_delays: DEFAULT_HIERARCHY_RETRY_DELAYS) response = get("/api/installation/#{installation_id}/hierarchy") - if response.status == 204 - sleep 2 + retry_delays.each do |delay| + break unless response.status == 204 + + sleep delay 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..c24218a 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 the default 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" } }, ) @@ -529,10 +530,11 @@ installation_hierarchy = client.get_installation_hierarchy("I123") expect(installation_hierarchy.name).to eq("Stekker test") - expect(client).to have_received(:sleep).with(2) + expect(client).to have_received(:sleep).with(2).ordered + expect(client).to have_received(:sleep).with(4).ordered end - it "raises RequestFailed after two consecutive 204 responses" do + it "raises RequestFailed when every attempt returns 204 with the default retries" do WebMock::API .stub_request(:get, "https://api.zaptec.com/api/installation/I123/hierarchy") .to_return(status: 204, body: "", headers: {}) @@ -544,6 +546,47 @@ expect { client.get_installation_hierarchy("I123") } .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + expect(client).to have_received(:sleep).with(2).ordered + expect(client).to have_received(:sleep).with(4).ordered + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(3) + end + + it "honours a caller-supplied retry_delays list" 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:) + + allow(client).to receive(:sleep) + + expect { client.get_installation_hierarchy("I123", retry_delays: [1, 3, 5, 7]) } + .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + expect(client).to have_received(:sleep).with(1).ordered + expect(client).to have_received(:sleep).with(3).ordered + expect(client).to have_received(:sleep).with(5).ordered + expect(client).to have_received(:sleep).with(7).ordered + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(5) + end + + it "makes a single attempt with no retries when retry_delays is empty" 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:) + + allow(client).to receive(:sleep) + + expect { client.get_installation_hierarchy("I123", retry_delays: []) } + .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") + + expect(client).not_to have_received(:sleep) + expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").once end end From ed592e7971b8d79843177b0dc2e4a237a5cffbee Mon Sep 17 00:00:00 2001 From: Martijn Versluis Date: Fri, 3 Jul 2026 16:48:41 +0200 Subject: [PATCH 2/2] Cap installation-hierarchy retries by a max_wait deadline --- lib/zaptec/client.rb | 22 ++++++++------ spec/zaptec/client_spec.rb | 62 ++++++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/lib/zaptec/client.rb b/lib/zaptec/client.rb index 1ff68ba..d872e6f 100644 --- a/lib/zaptec/client.rb +++ b/lib/zaptec/client.rb @@ -4,7 +4,7 @@ class Client USER_ROLE = 1 OWNER_ROLE = 2 TOKENS_CACHE_KEY = "zaptec.auth.tokens".freeze - DEFAULT_HIERARCHY_RETRY_DELAYS = [2, 4].freeze + DEFAULT_HIERARCHY_MAX_WAIT = 30.seconds attr_reader :credentials @@ -75,15 +75,19 @@ def get_installation(installation_id) # https://api.zaptec.com/help/index.html#/Installation/get_api_installation__id__hierarchy # # Zaptec occasionally returns 204 while an installation is still being provisioned. - # `retry_delays` is the list of pauses (in seconds) between retries; its length - # determines the number of retries. `[]` means a single attempt, no retries. - def get_installation_hierarchy(installation_id, retry_delays: DEFAULT_HIERARCHY_RETRY_DELAYS) + # 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") - - retry_delays.each do |delay| - break unless response.status == 204 - - sleep delay + 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/spec/zaptec/client_spec.rb b/spec/zaptec/client_spec.rb index c24218a..0a33d17 100644 --- a/spec/zaptec/client_spec.rb +++ b/spec/zaptec/client_spec.rb @@ -506,7 +506,7 @@ ) end - it "retries with the default backoff after 204 responses until one succeeds" do + it "retries with exponential backoff after 204 responses until one succeeds" do hierarchy_body = { Id: "2bbec6f9-c3ce-4edf-a72f-b1b2a663c6ba", Name: "Stekker test", @@ -525,16 +525,16 @@ 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).ordered - expect(client).to have_received(:sleep).with(4).ordered + expect(sleeps).to eq([2, 2]) end - it "raises RequestFailed when every attempt returns 204 with the default retries" do + 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: {}) @@ -542,17 +542,18 @@ 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: 50) } .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") - expect(client).to have_received(:sleep).with(2).ordered - expect(client).to have_received(:sleep).with(4).ordered - expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(3) + # 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 "honours a caller-supplied retry_delays list" do + 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: {}) @@ -560,19 +561,18 @@ 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", retry_delays: [1, 3, 5, 7]) } + expect { client.get_installation_hierarchy("I123", max_wait: 5) } .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") - expect(client).to have_received(:sleep).with(1).ordered - expect(client).to have_received(:sleep).with(3).ordered - expect(client).to have_received(:sleep).with(5).ordered - expect(client).to have_received(:sleep).with(7).ordered - expect(WebMock).to have_requested(:get, "https://api.zaptec.com/api/installation/I123/hierarchy").times(5) + # 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 "makes a single attempt with no retries when retry_delays is empty" 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: {}) @@ -580,12 +580,30 @@ 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", retry_delays: []) } + expect { client.get_installation_hierarchy("I123", max_wait: 5.seconds) } .to raise_error(Zaptec::Errors::RequestFailed, "Empty response for installation hierarchy") - expect(client).not_to have_received(:sleep) + 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