From 4c36da46fe1027dd90caf4c478e8d2a1d841fe06 Mon Sep 17 00:00:00 2001
From: Sebastian Kippe <sebastian@kip.pe>
Date: Tue, 9 Jun 2020 10:48:26 +0200
Subject: [PATCH 1/2] Improve handling of failed requests

This adds a custom exception class with an error type that can be more
easily used by clients. It also explicitly handles failed connections as
such.

closes #9
---
 lib/manifique/errors.rb           | 11 +++++++
 lib/manifique/web_client.rb       |  7 +++--
 spec/manifique/web_client_spec.rb | 48 +++++++++++++++++++++++++++++--
 3 files changed, 62 insertions(+), 4 deletions(-)
 create mode 100644 lib/manifique/errors.rb

diff --git a/lib/manifique/errors.rb b/lib/manifique/errors.rb
new file mode 100644
index 0000000..5ee9f73
--- /dev/null
+++ b/lib/manifique/errors.rb
@@ -0,0 +1,11 @@
+module Manifique
+  class Error < ::StandardError
+    attr_reader :type, :url
+
+    def initialize(msg="Encountered an error", type="generic", url)
+      @type = type
+      @url = url
+      super(msg)
+    end
+  end
+end
diff --git a/lib/manifique/web_client.rb b/lib/manifique/web_client.rb
index 801691e..c81e993 100644
--- a/lib/manifique/web_client.rb
+++ b/lib/manifique/web_client.rb
@@ -1,8 +1,9 @@
 require 'json'
 require 'faraday'
 require 'faraday_middleware'
-require "nokogiri"
+require 'nokogiri'
 require 'manifique/metadata'
+require 'manifique/errors'
 
 module Manifique
   class WebClient
@@ -160,8 +161,10 @@ module Manifique
       if res.status < 400
         res
       else
-        raise "Could not fetch #{url} successfully (#{res.status})"
+        raise Manifique::Error.new "Failed with HTTP status #{res.status}", "http_#{res.status}", url
       end
+    rescue Faraday::ConnectionFailed, Faraday::TimeoutError, Faraday::SSLError => e
+      raise Manifique::Error.new "Failed to connect", "connection_failed", url
     end
 
     def discover_web_manifest_url(html)
diff --git a/spec/manifique/web_client_spec.rb b/spec/manifique/web_client_spec.rb
index 95fe715..1599653 100644
--- a/spec/manifique/web_client_spec.rb
+++ b/spec/manifique/web_client_spec.rb
@@ -13,6 +13,13 @@ RSpec.describe Manifique::WebClient do
 
       stub_request(:get, "http://example.com/200_empty").
         to_return(body: "", status: 200, headers: {})
+
+      stub_request(:get, "http://example.com/failed").
+        to_raise(Faraday::ConnectionFailed)
+      stub_request(:get, "http://example.com/timeout").
+        to_raise(Faraday::TimeoutError)
+      stub_request(:get, "http://example.com/ssl_error").
+        to_raise(Faraday::SSLError)
     end
 
     context "unsuccessful requests" do
@@ -22,7 +29,12 @@ RSpec.describe Manifique::WebClient do
         it "raises an exception" do
           expect {
             client.send(:do_get_request, 'http://example.com/404')
-          }.to raise_error("Could not fetch http://example.com/404 successfully (404)")
+          }.to raise_error { |error|
+            expect(error).to be_a(Manifique::Error)
+            expect(error.message).to eq("Failed with HTTP status 404")
+            expect(error.type).to eq("http_404")
+            expect(error.url).to eq("http://example.com/404")
+          }
         end
       end
 
@@ -32,7 +44,39 @@ RSpec.describe Manifique::WebClient do
         it "raises an exception" do
           expect {
             client.send(:do_get_request, 'http://example.com/500')
-          }.to raise_error("Could not fetch http://example.com/500 successfully (500)")
+          }.to raise_error { |error|
+            expect(error).to be_a(Manifique::Error)
+            expect(error.message).to eq("Failed with HTTP status 500")
+            expect(error.type).to eq("http_500")
+            expect(error.url).to eq("http://example.com/500")
+          }
+        end
+      end
+
+      describe "failed connections" do
+        let(:client) { Manifique::WebClient.new }
+
+        it "raises an exception on connection failures" do
+          expect {
+            client.send(:do_get_request, 'http://example.com/failed')
+          }.to raise_error { |error|
+            expect(error).to be_a(Manifique::Error)
+            expect(error.message).to eq("Failed to connect")
+            expect(error.type).to eq("connection_failed")
+            expect(error.url).to eq("http://example.com/failed")
+          }
+        end
+
+        it "raises an exception on timouts" do
+          expect {
+            client.send(:do_get_request, 'http://example.com/timeout')
+          }.to raise_error("Failed to connect")
+        end
+
+        it "raises an exception on SSL errors" do
+          expect {
+            client.send(:do_get_request, 'http://example.com/ssl_error')
+          }.to raise_error("Failed to connect")
         end
       end
     end

From 3d9bdb1cab66d8e5b63abc68fae79111f7c627c1 Mon Sep 17 00:00:00 2001
From: Sebastian Kippe <sebastian@kip.pe>
Date: Tue, 9 Jun 2020 14:14:54 +0200
Subject: [PATCH 2/2] Update spec/manifique/web_client_spec.rb

Co-authored-by: Garret Alfert <garret@5apps.com>
---
 spec/manifique/web_client_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/manifique/web_client_spec.rb b/spec/manifique/web_client_spec.rb
index 1599653..51b9baf 100644
--- a/spec/manifique/web_client_spec.rb
+++ b/spec/manifique/web_client_spec.rb
@@ -67,7 +67,7 @@ RSpec.describe Manifique::WebClient do
           }
         end
 
-        it "raises an exception on timouts" do
+        it "raises an exception on timeouts" do
           expect {
             client.send(:do_get_request, 'http://example.com/timeout')
           }.to raise_error("Failed to connect")