Add timeout to GetMediaUrl's HTTParty (forem#18136)

app/services/podcasts/get_media_url.rb

@@ -9,6 +9,8 @@ class GetMediaUrl
       OpenSSL::SSL::SSLError,
     ].freeze
 
+    TIMEOUT = 20
+
     def initialize(enclosure_url)
       @enclosure_url = enclosure_url.to_s
     end
@@ -43,7 +45,7 @@ def result_struct
 
     def url_reachable?(url)
       url = Addressable::URI.parse(url).normalize.to_s
-      HTTParty.head(url).code == 200
+      HTTParty.head(url, timeout: TIMEOUT).code == 200
     rescue *HANDLED_ERRORS
       false
     end

spec/services/podcasts/feed_spec.rb

@@ -64,18 +64,16 @@
     end
 
     it "re-checks episodes urls when setting as unreachable" do
-      allow(HTTParty).to receive(:get).with("http://podcast.example.com/podcast",
-                                            httparty_options).and_raise(Errno::ECONNREFUSED)
-      episode = create(:podcast_episode, podcast: unpodcast, reachable: true,
-                                         media_url: "http://podcast.example.com/ep1.mp3")
-      allow(HTTParty).to receive(:head).with("http://podcast.example.com/ep1.mp3").and_raise(Errno::ECONNREFUSED)
-      allow(HTTParty).to receive(:head).with("https://podcast.example.com/ep1.mp3").and_raise(Errno::ECONNREFUSED)
-
-      sidekiq_perform_enqueued_jobs do
-        described_class.new(unpodcast).get_episodes
-      end
-
+      options = { timeout: Podcasts::GetMediaUrl::TIMEOUT }
+      error = Errno::ECONNREFUSED
+      allow(HTTParty).to receive(:get).with("http://podcast.example.com/podcast", httparty_options).and_raise(error)
+      allow(HTTParty).to receive(:head).with("http://podcast.example.com/ep1.mp3", options).and_raise(error)
+      allow(HTTParty).to receive(:head).with("https://podcast.example.com/ep1.mp3", options).and_raise(error)
+
+      episode = create(:podcast_episode, podcast: unpodcast, reachable: true, media_url: "http://podcast.example.com/ep1.mp3")
+      sidekiq_perform_enqueued_jobs { described_class.new(unpodcast).get_episodes }
       episode.reload
+
       expect(episode.reachable).to be false
     end
 

spec/services/podcasts/get_media_url_spec.rb

@@ -3,103 +3,93 @@
 RSpec.describe Podcasts::GetMediaUrl, type: :service do
   let(:https_url) { "https://hello.example.com/" }
   let(:http_url) { "http://hello.example.com/" }
+  let(:options) { { timeout: described_class::TIMEOUT } }
 
   it "https, reachable" do
     stub_request(:head, https_url).to_return(status: 200)
     result = described_class.call(https_url)
-    expect(result.https).to be true
-    expect(result.reachable).to be true
-    expect(result.url).to eq(https_url)
+
+    expect(result).to have_attributes(https: true, reachable: true, url: https_url)
   end
 
   it "normalizes url" do
     url = "https://hello.example.com/hi%20there.mp3"
     stub_request(:head, url).to_return(status: 200)
     result = described_class.call(url)
-    expect(result.https).to be true
-    expect(result.reachable).to be true
-    expect(result.url).to eq(url)
+
+    expect(result).to have_attributes(https: true, reachable: true, url: url)
   end
 
   it "https, unreachable" do
     stub_request(:head, https_url).to_return(status: 404)
     result = described_class.call(https_url)
-    expect(result.https).to be true
-    expect(result.reachable).to be false
-    expect(result.url).to eq(https_url)
+
+    expect(result).to have_attributes(https: true, reachable: false, url: https_url)
   end
 
   it "http, https reachable" do
     stub_request(:head, https_url).to_return(status: 200)
     result = described_class.call(http_url)
-    expect(result.https).to be true
-    expect(result.reachable).to be true
-    expect(result.url).to eq(https_url)
+
+    expect(result).to have_attributes(https: true, reachable: true, url: https_url)
   end
 
   it "http, https unreachable, http reachable" do
     httparty_result = double
     allow(httparty_result).to receive(:code).and_return(200)
-    allow(HTTParty).to receive(:head).with(http_url).and_return(httparty_result)
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::ECONNREFUSED)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_return(httparty_result)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(Errno::ECONNREFUSED)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be true
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: true, url: http_url)
   end
 
   it "http, https unreachable" do
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::ECONNREFUSED)
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(Errno::ECONNREFUSED)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(Errno::ECONNREFUSED)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(Errno::ECONNREFUSED)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be false
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: false, url: http_url)
   end
 
   it "http, https unreachable with other exception" do
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::EINVAL)
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(Errno::EINVAL)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(Errno::EINVAL)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(Errno::EINVAL)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be false
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: false, url: http_url)
   end
 
   it "http, https unreachable with invalid url exception" do
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(URI::InvalidURIError)
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(URI::InvalidURIError)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(URI::InvalidURIError)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(URI::InvalidURIError)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be false
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: false, url: http_url)
   end
 
   it "http, https unreachable with openssl error" do
     httparty_result = instance_double(HTTParty::Response, code: 200)
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(OpenSSL::SSL::SSLError)
-    allow(HTTParty).to receive(:head).with(http_url).and_return(httparty_result)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(OpenSSL::SSL::SSLError)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_return(httparty_result)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be true
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: true, url: http_url)
   end
 
   it "marks unreachable with addressable invalid url exception" do
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(Addressable::URI::InvalidURIError)
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(Addressable::URI::InvalidURIError)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(Addressable::URI::InvalidURIError)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(Addressable::URI::InvalidURIError)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be false
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: false, url: http_url)
   end
 
   it "marks socket errors as invalid url exception" do
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(SocketError)
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(SocketError)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(SocketError)
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(SocketError)
     result = described_class.call(http_url)
-    expect(result.https).to be false
-    expect(result.reachable).to be false
-    expect(result.url).to eq(http_url)
+
+    expect(result).to have_attributes(https: false, reachable: false, url: http_url)
   end
 end

spec/services/podcasts/update_episode_media_url_spec.rb

@@ -11,23 +11,22 @@
     episode = create(:podcast_episode, podcast: podcast, media_url: "http://example.com/1.mp3")
     described_class.call(episode, http_url)
     episode.reload
-    expect(episode.media_url).to eq(https_url)
-    expect(episode.reachable).to be true
-    expect(episode.https).to be true
+
+    expect(episode).to have_attributes(reachable: true, https: true, media_url: https_url)
   end
 
   it "keeps http when https and http are not reachable" do
     http_url = "http://example.com/1.mp3"
     https_url = "https://example.com/1.mp3"
-    allow(HTTParty).to receive(:head).with(http_url).and_raise(Errno::ECONNREFUSED)
-    allow(HTTParty).to receive(:head).with(https_url).and_raise(Errno::ECONNREFUSED)
+    options = { timeout: Podcasts::GetMediaUrl::TIMEOUT }
+    allow(HTTParty).to receive(:head).with(http_url, options).and_raise(Errno::ECONNREFUSED)
+    allow(HTTParty).to receive(:head).with(https_url, options).and_raise(Errno::ECONNREFUSED)
 
     episode = create(:podcast_episode, podcast: podcast, media_url: http_url)
     described_class.call(episode, http_url)
     episode.reload
-    expect(episode.media_url).to eq(http_url)
-    expect(episode.reachable).to be false
-    expect(episode.https).to be false
+
+    expect(episode).to have_attributes(reachable: false, https: false, media_url: http_url)
   end
 
   it "does fine when there's nothing to update" do
@@ -36,8 +35,7 @@
     episode = create(:podcast_episode, podcast: podcast, media_url: url)
     described_class.call(episode, url)
     episode.reload
-    expect(episode.media_url).to eq(url)
-    expect(episode.reachable).to be true
-    expect(episode.https).to be true
+
+    expect(episode).to have_attributes(reachable: true, https: true, media_url: url)
   end
 end