FIX: return an empty result if response from Amazon is missing expected attributes (#13173)

FIX: return an empty result if response from Amazon is missing expected attributes (#13173)

  • FIX: return an empty result if response from Amazon is missing attributes

Check we have the basic attributes requires to construct a Onebox for Amazon.

This is an attempt to handle scenarios where we receive a valid 200-status response from an Amazon request that does not include the data we’re expecting.

  • Update lib/onebox/engine/amazon_onebox.rb

Co-authored-by: Régis Hanol regis@hanol.fr

Co-authored-by: Régis Hanol regis@hanol.fr

diff --git a/lib/onebox/engine/allowlisted_generic_onebox.rb b/lib/onebox/engine/allowlisted_generic_onebox.rb
index f4d5d3b..e264abe 100644
--- a/lib/onebox/engine/allowlisted_generic_onebox.rb
+++ b/lib/onebox/engine/allowlisted_generic_onebox.rb
@@ -66,6 +66,10 @@ module Onebox
         to_html
       end
 
+      def verified_data
+        data
+      end
+
       def data
         @data ||= begin
           html_entities = HTMLEntities.new
diff --git a/lib/onebox/engine/amazon_onebox.rb b/lib/onebox/engine/amazon_onebox.rb
index 4ea26aa..7f377ca 100644
--- a/lib/onebox/engine/amazon_onebox.rb
+++ b/lib/onebox/engine/amazon_onebox.rb
@@ -46,6 +46,37 @@ module Onebox
         end
       end
 
+      def to_html(ignore_errors = false)
+        unless ignore_errors
+          verified_data # forces a check for missing fields
+          return '' unless errors.empty?
+        end
+
+        super()
+      end
+
+      def placeholder_html
+        to_html(true)
+      end
+
+      def verified_data
+        @verified_data ||= begin
+          result = data
+
+          required_tags = [:title, :description]
+          required_tags.each do |tag|
+            if result[tag].blank?
+              errors[tag] ||= []
+              errors[tag] << 'is blank'
+            end
+          end
+
+          result
+        end
+
+        @verified_data
+      end
+
       private
 
       def has_cached_body
diff --git a/lib/onebox/preview.rb b/lib/onebox/preview.rb
index ada48e1..9c856ed 100644
--- a/lib/onebox/preview.rb
+++ b/lib/onebox/preview.rb
@@ -40,6 +40,11 @@ module Onebox
       engine.data
     end
 
+    def verified_data
+      return {} unless engine
+      engine.verified_data
+    end
+
     def options
       OpenStruct.new(@options)
     end
diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index 239f6b2..69bb14c 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -464,7 +464,7 @@ module Oneboxer
         unless error_keys.length == 1 && skip_if_only_error.include?(error_keys.first)
           missing_attributes = error_keys.map(&:to_s).sort.join(I18n.t("word_connector.comma"))
           error_message = I18n.t("errors.onebox.missing_data", missing_attributes: missing_attributes, count: error_keys.size)
-          args = r.data.merge(error_message: error_message)
+          args = r.verified_data.merge(error_message: error_message)
 
           if result[:preview].blank?
             result[:preview] = preview_error_onebox(args)
diff --git a/spec/fixtures/onebox/amazon-error.response b/spec/fixtures/onebox/amazon-error.response
new file mode 100644
index 0000000..78060d6
--- /dev/null
+++ b/spec/fixtures/onebox/amazon-error.response
@@ -0,0 +1,38 @@
+<!doctype html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <meta http-equiv="x-ua-compatible" content="ie=edge">
+    <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
+    <style>
+    html, body {
+        padding: 0;
+        margin:0
+    }
+    </style>
+</head>
+<body>
+    <form id="b" accept-charset="utf-8" action="/s" method="GET" role="search">
+        <a href="/">
+            <img id="c" src="https://images-na.ssl-images-amazon.com/images/G/01/error/logo._TTD_.png" alt="Amazon.com">
+        </a>
+        <div id="e">
+            <input id="f" name="field-keywords" placeholder="Search">
+            <input id="g" type="submit" value="Go">
+        </div>
+    </form>
+    <div id="h">
+        <div>
+            <a href="/">
+                <img src="https://images-na.ssl-images-amazon.com/images/G/01/error/title._TTD_.png" alt="Sorry! We couldn&#39;t find that page. Try searching or go to Amazon&#39;s home page.">
+            </a>
+        </div>
+        <a href="/dogsofamazon" target="_blank">
+            <img id="d" alt="Dogs of Amazon">
+            <script>
+            document.getElementById("d").src = "https://images-na.ssl-images-amazon.com/images/G/01/error/" + (Math.floor(Math.random() * 200) + 1) + "._TTD_.jpg";
+            </script>
+        </a>
+    </div>
+</body>
+</html>
diff --git a/spec/lib/onebox/engine/amazon_onebox_spec.rb b/spec/lib/onebox/engine/amazon_onebox_spec.rb
index 44479d1..7d5973b 100644
--- a/spec/lib/onebox/engine/amazon_onebox_spec.rb
+++ b/spec/lib/onebox/engine/amazon_onebox_spec.rb
@@ -54,7 +54,7 @@ describe Onebox::Engine::AmazonOnebox do
         check_link("es", "https://www.amazon.es/familia-Pascual-Duarte-Camilo-Jos%C3%A9-ebook/dp/B00EJRTKTW/")
       end
 
-      it "matches brasilian domains" do
+      it "matches brazilian domains" do
         check_link("com.br", "https://www.amazon.com.br/A-p%C3%A1tria-chuteiras-Nelson-Rodrigues-ebook/dp/B00J2B414Y/")
       end
 
@@ -193,4 +193,28 @@ describe Onebox::Engine::AmazonOnebox do
       end
     end
   end
+
+  context "non-standard response from Amazon" do
+    let(:link) { "https://www.amazon.com/dp/B0123ABCD3210" }
+    let(:onebox) { described_class.new(link) }
+
+    before do
+      stub_request(:get, "https://www.amazon.com/dp/B0123ABCD3210")
+        .to_return(status: 200, body: onebox_response("amazon-error"))
+    end
+
+    it "returns a blank result" do
+      expect(onebox.to_html).to eq("")
+    end
+
+    it "produces a placeholder" do
+      expect(onebox.placeholder_html).to include('<aside class="onebox amazon">')
+    end
+
+    it "returns errors" do
+      onebox.to_html
+      expect(onebox.errors).to eq({ title: ["is blank"], description: ["is blank"] })
+    end
+  end
+
 end

GitHub sha: 461a2c33

This commit appears in #13173 which was approved by ZogStriP. It was merged by jbrw.