From e2a132150af90c3578e5553d71b1b009d5074569 Mon Sep 17 00:00:00 2001 From: Richard Vowles Date: Fri, 1 May 2026 12:03:53 +1200 Subject: [PATCH 1/3] fix: rescue when there is an initial timeout If the first poll is a timeout or similar, then it will potentially cause an error all the way up the stack and prevent subsequent polling from sorting the situation out. --- examples/sinatra/Gemfile.lock | 3 +- lib/feature_hub/sdk/poll_edge_service.rb | 36 +-- .../feature_hub/sdk/poll_edge_service_spec.rb | 250 ++++++++++-------- 3 files changed, 163 insertions(+), 126 deletions(-) diff --git a/examples/sinatra/Gemfile.lock b/examples/sinatra/Gemfile.lock index 2e5e8fc..8e33d5e 100644 --- a/examples/sinatra/Gemfile.lock +++ b/examples/sinatra/Gemfile.lock @@ -4,7 +4,7 @@ PATH featurehub-sdk (2.1.1) concurrent-ruby (~> 1.3) faraday - ld-eventsource (~> 2.5.1) + ld-eventsource (>= 2.5.1, < 2.7.0) murmurhash3 (~> 0.1.7) sem_version (~> 2.0.0) @@ -44,6 +44,7 @@ GEM faraday-rack (1.0.0) faraday-retry (1.0.4) ffi (1.17.3-x86_64-darwin) + ffi (1.17.3-x86_64-linux-gnu) ffi-compiler (1.3.2) ffi (>= 1.15.5) rake diff --git a/lib/feature_hub/sdk/poll_edge_service.rb b/lib/feature_hub/sdk/poll_edge_service.rb index d760dde..5398346 100644 --- a/lib/feature_hub/sdk/poll_edge_service.rb +++ b/lib/feature_hub/sdk/poll_edge_service.rb @@ -108,21 +108,27 @@ def get_updates headers["if-none-match"] = @etag unless @etag.nil? @logger&.debug("polling for #{url}") - resp = @conn.get url, {}, headers - case resp.status - when 200 - success(resp) - when 236 - stopped_task - success(resp) - when 404, 400 # no such key - @repository.notify("failed", nil, "polling") - cancel_task - @logger&.error("featurehub: key does not exist, stopping polling") - when 503 # dacha busy - @logger&.debug("featurehub: dacha is busy, trying again") - else - @logger&.debug("featurehub: unknown error #{resp.status}") if resp.status != 304 + begin + resp = @conn.get url, {}, headers + case resp.status + when 200 + success(resp) + when 236 + stopped_task + success(resp) + when 404, 400 # no such key + @repository.notify("failed", nil, "polling") + cancel_task + @logger&.error("featurehub: key does not exist, stopping polling") + when 503 # dacha busy + @logger&.debug("featurehub: dacha is busy, trying again") + else + @logger&.debug("featurehub: unknown error #{resp.status}") if resp.status != 304 + end + rescue StandardError => e + # we can get timeout errors for transient network failures, this should not prevent the + # next poll from happening however + @logger&.error("featurehub: failed to connect or similar error #{e&.message}") end end diff --git a/spec/feature_hub/sdk/poll_edge_service_spec.rb b/spec/feature_hub/sdk/poll_edge_service_spec.rb index 4ccf398..60bc9a7 100644 --- a/spec/feature_hub/sdk/poll_edge_service_spec.rb +++ b/spec/feature_hub/sdk/poll_edge_service_spec.rb @@ -8,155 +8,185 @@ let(:timer) { instance_double(Concurrent::TimerTask) } let(:interval) { 0 } let(:logger) { double("logger") } - let(:poller) { described_class.new(repo, ["api_key"], "url/", interval, logger) } let(:conn) { instance_double(Faraday::Connection) } let(:resp) { instance_double(Faraday::Response) } - before do - allow(logger).to receive(:debug) - allow(logger).to receive(:info) - allow(Faraday).to receive(:new).with(url: "url/").and_return(conn) - allow(Concurrent::TimerTask).to receive(:new).with(execution_interval: interval, run_now: false).and_return(timer) - end + context "when polling" do + let(:poller) { described_class.new(repo, ["api_key"], "url/", interval, logger) } - context "with a different header" do - let(:interval) { 20 } + before(:each) do + allow(logger).to receive(:debug) + allow(logger).to receive(:info) + allow(Faraday).to receive(:new).with(url: "url/").and_return(conn) + allow(Concurrent::TimerTask).to receive(:new).with(execution_interval: interval, run_now: false).and_return(timer) + allow(timer).to receive(:shutdown) + end - it "should allow me to set a different header" do - expect(Digest::SHA256).to receive(:hexdigest).and_return("12345") + after(:each) do + poller.close + end - expect(conn).to receive(:get).with("features?apiKey=api_key&contextSha=12345", - {}, - hash_including("x-featurehub" => "blah")) - .and_return(resp) + context "conn fails with timeout error" do + let(:interval) { 20 } - expect(resp).to receive(:status).and_return(200) - expect(resp).to receive(:headers).and_return({}).twice - expect(resp).to receive(:body).and_return("[]") - expect(timer).to receive(:execute) + it "should continue to poll and not cancel if an exception is thrown" do + expect(Digest::SHA256).to receive(:hexdigest).and_return("12345") + expect(conn).to receive(:get).with("features?apiKey=api_key&contextSha=12345", + {}, + hash_including("x-featurehub" => "blah")) + .and_throw(Faraday::TimeoutError) + expect(logger).to receive(:error).with("featurehub: failed to connect or similar error uncaught throw Faraday::TimeoutError") - poller.context_change("blah") - expect(poller.sha_context).to eq("12345") - end - end + expect(timer).to receive(:execute) # starts the next timer going + expect(repo).not_to receive(:notify) + + poller.context_change("blah") - # test essentially works, can't figure out how to get it to match - # it "should pick up the etag header and try and use it again" do - # expect(conn).to receive(:get).with("features?apiKey=api_key", - # request: { timeout: 12 }, - # headers: hash_including(accept: "application/json")) - # .and_return(resp) - # expect(resp).to receive(:status).and_return(200).at_least(:twice) - # expect(resp).to receive(:headers).and_return({"etag" => "12345"}).at_least(:twice) - # expect(resp).to receive(:body).and_return("[]").twice - # expect(timer).to receive(:execute).twice - # expect(timer).to receive(:shutdown) - # poller.poll - # poller.close - # expect(conn).to receive(:get).with("features?apiKey=api_key", - # request: { timeout: 12 }, - # headers: hash_including("if-none-matches" => "12345")) - # .and_return(resp).once - # poller.poll - # end - - context "setting a 20 second interval" do - let(:interval) { 20 } - - before do - expect(conn).to receive(:get).with("features?apiKey=api_key&contextSha=0", - {}, - hash_including(accept: "application/json")) - .and_return(resp) + expect(poller.cancel).to eq(false) + end end - it "should start polling and process a 236 response" do - expect(resp).to receive(:status).and_return(236) - expect(resp).to receive(:headers).and_return({}).twice - expect(resp).to receive(:body).and_return("[]") - expect(timer).to receive(:shutdown) + context "with a different header" do + let(:interval) { 20 } - poller.poll + it "should allow me to set a different header" do + expect(Digest::SHA256).to receive(:hexdigest).and_return("12345") - expect(poller.stopped).to eq(true) - end + expect(conn).to receive(:get).with("features?apiKey=api_key&contextSha=12345", + {}, + hash_including("x-featurehub" => "blah")) + .and_return(resp) - it "should try again on a 503" do - expect(resp).to receive(:status).and_return(503) - expect(timer).to receive(:execute) - poller.poll - expect(poller.cancel).to eq(false) - end + expect(resp).to receive(:status).and_return(200) + expect(resp).to receive(:headers).and_return({}).twice + expect(resp).to receive(:body).and_return("[]") + expect(timer).to receive(:execute) - it "should cancel on a 404" do - expect(resp).to receive(:status).and_return(404) - expect(repo).to receive(:notify).with("failed", nil, "polling") - expect(logger).to receive(:error) - expect(timer).to receive(:shutdown) - poller.poll - expect(poller.cancel).to eq(true) + poller.context_change("blah") + expect(poller.sha_context).to eq("12345") + end end - context "should start polling and process a 200 response" do + # test essentially works, can't figure out how to get it to match + # it "should pick up the etag header and try and use it again" do + # expect(conn).to receive(:get).with("features?apiKey=api_key", + # request: { timeout: 12 }, + # headers: hash_including(accept: "application/json")) + # .and_return(resp) + # expect(resp).to receive(:status).and_return(200).at_least(:twice) + # expect(resp).to receive(:headers).and_return({"etag" => "12345"}).at_least(:twice) + # expect(resp).to receive(:body).and_return("[]").twice + # expect(timer).to receive(:execute).twice + # expect(timer).to receive(:shutdown) + # poller.poll + # poller.close + # expect(conn).to receive(:get).with("features?apiKey=api_key", + # request: { timeout: 12 }, + # headers: hash_including("if-none-matches" => "12345")) + # .and_return(resp).once + # poller.poll + # end + + context "setting a 20 second interval" do + let(:interval) { 20 } + before do - expect(resp).to receive(:status).and_return(200) + expect(conn).to receive(:get).with("features?apiKey=api_key&contextSha=0", + {}, + hash_including(accept: "application/json")) + .and_return(resp) + end + + it "should start polling and process a 236 response" do + expect(resp).to receive(:status).and_return(236) + expect(resp).to receive(:headers).and_return({}).twice + expect(resp).to receive(:body).and_return("[]") + expect(timer).to receive(:shutdown) + + poller.poll + + expect(poller.stopped).to eq(true) + end + + it "should try again on a 503" do + expect(resp).to receive(:status).and_return(503) expect(timer).to receive(:execute) + poller.poll + expect(poller.cancel).to eq(false) end - context "with an etag header" do + it "should cancel on a 404" do + expect(resp).to receive(:status).and_return(404) + expect(repo).to receive(:notify).with("failed", nil, "polling") + expect(logger).to receive(:error) + expect(timer).to receive(:shutdown) + poller.poll + expect(poller.cancel).to eq(true) + end + + context "should start polling and process a 200 response" do before do - expect(resp).to receive(:headers).and_return({ "etag" => "abcd" }).twice + expect(resp).to receive(:status).and_return(200) + expect(timer).to receive(:execute) end - it "and no data" do - expect(resp).to receive(:body).and_return("[]") + context "with an etag header" do + before do + expect(resp).to receive(:headers).and_return({ "etag" => "abcd" }).twice + end - poller.poll + it "and no data" do + expect(resp).to receive(:body).and_return("[]") - expect(poller.etag).to eq("abcd") + poller.poll + + expect(poller.etag).to eq("abcd") + end end - end - context "with an cache control header" do - it "that does not contain max-age" do - expect(resp).to receive(:headers) - .and_return({ "cache-control" => "no-store, no-age, bark, bark, bark" }) - .at_least(:once) - expect(resp).to receive(:body).and_return("[]") + context "with an cache control header" do + it "that does not contain max-age" do + expect(resp).to receive(:headers) + .and_return({ "cache-control" => "no-store, no-age, bark, bark, bark" }) + .at_least(:once) + expect(resp).to receive(:body).and_return("[]") - poller.poll + poller.poll - expect(poller.interval).to eq(interval) - end + expect(poller.interval).to eq(interval) + end - it "and the timer is created with it and its interval is set correctly" do - expect(resp).to receive(:headers) - .and_return({ "cache-control" => "no-store, max-age=24, bark, bark, bark" }) - .at_least(:once) - expect(resp).to receive(:body).and_return("[]") - expect(timer).to receive(:execution_interval=).with(24) + it "and the timer is created with it and its interval is set correctly" do + expect(resp).to receive(:headers) + .and_return({ "cache-control" => "no-store, max-age=24, bark, bark, bark" }) + .at_least(:once) + expect(resp).to receive(:body).and_return("[]") + expect(timer).to receive(:execution_interval=).with(24) - poller.poll + poller.poll + end end - end - context "with no headers" do - before do - expect(resp).to receive(:headers).and_return({}).twice - end + context "with no headers" do + before do + expect(resp).to receive(:headers).and_return({}).twice + end - it "and no data" do - expect(resp).to receive(:body).and_return("[]") + it "and no data" do + expect(resp).to receive(:body).and_return("[]") - poller.poll - end + poller.poll + end - it "and have environments" do - expect(resp).to receive(:body).and_return('[{"features": []}, {"features": []}]') - expect(repo).to receive(:notify).with("features", [], "polling").twice - poller.poll + it "and have environments" do + expect(resp).to receive(:body).and_return('[{"features": []}, {"features": []}]') + expect(repo).to receive(:notify).with("features", [], "polling").twice + poller.poll + end end end end + end + end From e7e76fa65f04caf00487a56e562f9a5c33787b3d Mon Sep 17 00:00:00 2001 From: Richard Vowles Date: Fri, 1 May 2026 12:05:28 +1200 Subject: [PATCH 2/3] these files should not be in the repo --- examples/sinatra/sinatra.iml | 51 --------------------- featurehub-sdk.iml | 87 ------------------------------------ 2 files changed, 138 deletions(-) delete mode 100644 examples/sinatra/sinatra.iml delete mode 100644 featurehub-sdk.iml diff --git a/examples/sinatra/sinatra.iml b/examples/sinatra/sinatra.iml deleted file mode 100644 index 17999a8..0000000 --- a/examples/sinatra/sinatra.iml +++ /dev/null @@ -1,51 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/featurehub-sdk.iml b/featurehub-sdk.iml deleted file mode 100644 index 323d9d1..0000000 --- a/featurehub-sdk.iml +++ /dev/null @@ -1,87 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file From 72859783a5349c0a55944b193071037f8f80a2c2 Mon Sep 17 00:00:00 2001 From: Richard Vowles Date: Fri, 1 May 2026 12:08:31 +1200 Subject: [PATCH 3/3] fix indentation --- spec/feature_hub/sdk/poll_edge_service_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/feature_hub/sdk/poll_edge_service_spec.rb b/spec/feature_hub/sdk/poll_edge_service_spec.rb index 60bc9a7..5cc06c6 100644 --- a/spec/feature_hub/sdk/poll_edge_service_spec.rb +++ b/spec/feature_hub/sdk/poll_edge_service_spec.rb @@ -147,8 +147,8 @@ context "with an cache control header" do it "that does not contain max-age" do expect(resp).to receive(:headers) - .and_return({ "cache-control" => "no-store, no-age, bark, bark, bark" }) - .at_least(:once) + .and_return({ "cache-control" => "no-store, no-age, bark, bark, bark" }) + .at_least(:once) expect(resp).to receive(:body).and_return("[]") poller.poll @@ -158,8 +158,8 @@ it "and the timer is created with it and its interval is set correctly" do expect(resp).to receive(:headers) - .and_return({ "cache-control" => "no-store, max-age=24, bark, bark, bark" }) - .at_least(:once) + .and_return({ "cache-control" => "no-store, max-age=24, bark, bark, bark" }) + .at_least(:once) expect(resp).to receive(:body).and_return("[]") expect(timer).to receive(:execution_interval=).with(24) @@ -186,7 +186,5 @@ end end end - end - end