From 1f5682b7d7ba8d97dfb135a9ff389d0840326ede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guitaut?= Date: Thu, 21 Jul 2022 15:58:17 +0200 Subject: [PATCH] =?UTF-8?q?FIX:=20Don=E2=80=99t=20raise=20an=20error=20on?= =?UTF-8?q?=20onebox=20timeouts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently when generating oneboxes if the connection timeouts and we’re using the `FinalDestination#get` method, then it raises an exception. We already catch this exception when using the `FinalDestination#resolve` method so this patch just applies the same logic to `FinalDestination#get`. --- lib/final_destination.rb | 3 ++ spec/lib/final_destination_spec.rb | 56 ++++++++++++++++++++---------- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/lib/final_destination.rb b/lib/final_destination.rb index 6426924232b..7070bcfdeec 100644 --- a/lib/final_destination.rb +++ b/lib/final_destination.rb @@ -518,6 +518,9 @@ class FinalDestination end result + rescue Timeout::Error + log(:warn, "FinalDestination could not resolve URL (timeout): #{@uri}") if @verbose + nil rescue StandardError unsafe_close ? [:ok, headers_subset] : raise end diff --git a/spec/lib/final_destination_spec.rb b/spec/lib/final_destination_spec.rb index 2c78f440ce8..3d0e1749e27 100644 --- a/spec/lib/final_destination_spec.rb +++ b/spec/lib/final_destination_spec.rb @@ -424,28 +424,46 @@ describe FinalDestination do end end - describe '.get' do + describe '#get' do + let(:fd) { FinalDestination.new("http://wikipedia.com", opts.merge(verbose: true)) } - it "can correctly stream with a redirect" do - FinalDestination.clear_https_cache!("wikipedia.com") - - stub_request(:get, "http://wikipedia.com/"). - to_return(status: 302, body: "" , headers: { "location" => "https://wikipedia.com/" }) - - # webmock does not do chunks - stub_request(:get, "https://wikipedia.com/"). - to_return(status: 200, body: "" , headers: {}) - - result = nil - chunk = nil - - result = FinalDestination.new("http://wikipedia.com", opts).get do |resp, c| - chunk = c - throw :done + context "when there is a redirect" do + before do + described_class.clear_https_cache!("wikipedia.com") + stub_request(:get, "http://wikipedia.com/"). + to_return(status: 302, body: "" , headers: { "location" => "https://wikipedia.com/" }) + # webmock does not do chunks + stub_request(:get, "https://wikipedia.com/"). + to_return(status: 200, body: "" , headers: {}) end - expect(result).to eq("https://wikipedia.com/") - expect(chunk).to eq("") + it "correctly streams" do + chunk = nil + result = fd.get do |resp, c| + chunk = c + throw :done + end + + expect(result).to eq("https://wikipedia.com/") + expect(chunk).to eq("") + end + end + + context "when there is a timeout" do + subject(:get) { fd.get {} } + + before do + fd.stubs(:safe_session).raises(Timeout::Error) + end + + it "logs the exception" do + Rails.logger.expects(:warn).with("default: FinalDestination could not resolve URL (timeout): https://wikipedia.com") + get + end + + it "returns nothing" do + expect(get).to be_blank + end end end