Merge pull request #12 from 5apps/feature/custom_exceptions

Improve handling of failed requests
Update spec/manifique/web_client_spec.rb
2020-06-09 14:16:02 +02:00 · 2020-06-09 14:14:54 +02:00 · 2020-06-09 10:59:14 +02:00 · 2020-06-09 10:48:26 +02:00 · 2020-05-26 16:56:44 +02:00 · 2020-05-26 16:27:55 +02:00
6 changed files with 128 additions and 11 deletions
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,6 @@
 sudo: false
 language: ruby
 rvm:
-  - 2.3.1
-before_install: gem install bundler -v 1.16.2
+  - 2.6.6
+cache: bundler
+before_install: gem install bundler
--- a/README.md
+++ b/README.md
@@ -1,3 +1,5 @@
+[![Build Status](https://secure.travis-ci.org/5apps/manifique.png?branch=master)](http://travis-ci.org/5apps/manifique)
+
 # Manifique

 Manifique fetches metadata of Web applications, like e.g. name, description,
@@ -84,4 +86,4 @@ _TODO add information about dual/commercial licensing_

 Everyone interacting in this project’s codebase, issue trackers, and chat
 rooms is expected to follow the [code of
-conduct](https://github.com/skddc/manifique/blob/master/CODE_OF_CONDUCT.md).
+conduct](https://github.com/5apps/manifique/blob/master/CODE_OF_CONDUCT.md).
--- a/lib/manifique/errors.rb
+++ 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
--- 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
@@ -103,7 +104,7 @@ module Manifique
        icon_links.each do |link|
          icon = {}
          icon["src"] = link.attributes["href"].value rescue nil
-          next if icon["src"].to_s.empty?
+          next unless is_adequate_src(icon["src"])
          icon["sizes"] = link.attributes["sizes"].value rescue nil
          icon["type"]  = link.attributes["type"].value rescue get_icon_type(icon["src"])
          @metadata.icons.push icon
@@ -117,7 +118,7 @@ module Manifique
        icon_links.each do |link|
          icon = { "purpose" => "apple-touch-icon" }
          icon["src"] = link.attributes["href"].value rescue nil
-          next if icon["src"].to_s.empty?
+          next unless is_adequate_src(icon["src"])
          icon["sizes"] = link.attributes["sizes"].value rescue nil
          icon["type"]  = link.attributes["type"].value rescue get_icon_type(icon["src"])
          @metadata.icons.push icon
@@ -130,7 +131,7 @@ module Manifique
      if mask_icon_link = @html.at_css("link[rel=mask-icon]")
        icon = { "purpose" => "mask-icon" }
        icon["src"] = mask_icon_link.attributes["href"].value rescue nil
-        return if icon["src"].to_s.empty?
+        return unless is_adequate_src(icon["src"])
        icon["type"]  = link.attributes["type"].value rescue get_icon_type(icon["src"])
        icon["color"] = mask_icon_link.attributes["color"].value rescue nil
        @metadata.icons.push icon
@@ -138,6 +139,14 @@ module Manifique
      end
    end

+    def is_data_url?(src)
+      !!src.match(/^data:/)
+    end
+
+    def is_adequate_src(src)
+      !src.to_s.empty? && !is_data_url?(src)
+    end
+
    def get_icon_type(src)
      extension = src.match(/\.([a-zA-Z]+)$/)[1]
      return "image/#{extension}"
@@ -152,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)
@@ -161,6 +172,5 @@ module Manifique
    rescue
      false
    end
-
  end
 end
--- a/spec/fixtures/kommit.html
+++ b/spec/fixtures/kommit.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html lang="en">
+
+<head>
+	<title>Kommit</title>
+	<meta name="description" content="Augment your memory" />
+	<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+	<link rel="icon" href="data:;base64,iVBORw0KGgo=">
+	<link rel="canonical" href="/" />
+	<link rel="alternate" hreflang="en" href="/en/" />
+	<link rel="apple-touch-icon" href="https://abcdefg.s3.amazonaws.com/public/icon.jpg">
+</head>
+<head>
+
+<body>
+</body>
+</html>
--- 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 timeouts" 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
@@ -211,6 +255,38 @@ RSpec.describe Manifique::WebClient do
        end
      end
    end
+
+    context "with data URL icons" do
+      before do
+        index_html = File.read(File.join(__dir__, "..", "fixtures", "kommit.html"));
+        stub_request(:get, "https://kosmos.social/").
+          to_return(body: index_html, status: 200, headers: {
+            "Content-Type": "text/html; charset=utf-8"
+          })
+      end
+
+      subject { web_client.fetch_metadata }
+
+      it "returns a metadata object" do
+        expect(subject).to be_kind_of(Manifique::Metadata)
+      end
+
+      it "loads properties from parsed HTML" do
+        expect(subject.name).to eq("Kommit")
+        expect(subject.description).to eq("Augment your memory")
+      end
+
+      it "ignores data URL icons" do
+        expect(subject.icons.length).to eq(1)
+      end
+
+      it "loads icons from link[rel=apple-touch-icon] elements" do
+        apple_touch_icons = subject.icons.select{|i| i["purpose"] == "apple-touch-icon"}
+        expect(apple_touch_icons.length).to eq(1)
+        expect(apple_touch_icons.first["type"]).to eq("image/jpg")
+        expect(apple_touch_icons.first["sizes"]).to be_nil
+      end
+    end
  end

 end
Author	SHA1	Message	Date
Sebastian Kippe	eed9ee4583	Merge pull request #12 from 5apps/feature/custom_exceptions Improve handling of failed requests	2020-06-09 14:16:02 +02:00
Sebastian Kippe	3d9bdb1cab	Update spec/manifique/web_client_spec.rb Co-authored-by: Garret Alfert <garret@5apps.com>	2020-06-09 14:14:54 +02:00
Sebastian Kippe	fa9f0b785e	Add CI badge to README	2020-06-09 10:59:14 +02:00
Sebastian Kippe	4c36da46fe	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	2020-06-09 10:48:26 +02:00
Sebastian Kippe	c5d5c123a6	Merge pull request #10 from 5apps/bugfix/ignore_inadequate_icon_sources Ignore inadequate icon sources	2020-05-26 16:56:44 +02:00
Sebastian Kippe	8826b1d672	Guard against empty strings As suggested by @galfert	2020-05-26 16:27:55 +02:00
Sebastian Kippe	5280a24f4b	Fix link	2020-05-26 12:37:11 +02:00
Sebastian Kippe	5de24e9fc4	Merge pull request #11 from 5apps/chore/travis Update Travis config	2020-05-26 12:35:12 +02:00
Sebastian Kippe	33134c12f4	Update Travis config	2020-05-26 12:32:19 +02:00
Sebastian Kippe	7c89900c70	Ignore inadequate icon sources Icons with data URLs as source are throwing exceptions when trying to parse their type via the file extension. This fix ignores all icons with data URLs to begin with.	2020-05-26 11:30:21 +02:00