Raise more specific error when wrapper not found

alphagov · Mar 26, 2024 · 81e2c93 · 81e2c93
1 parent 9be1a24
commit 81e2c93
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# Unreleased
+
+* Raise more specific error when wrapper not found
+
 # 18.4.0
 
 * Drop support for Ruby 3.0. Ruby 3.2 is now required.

diff --git a/lib/slimmer.rb b/lib/slimmer.rb
@@ -51,4 +51,5 @@ module Processors
   class CouldNotRetrieveTemplate < StandardError; end
   class TemplateNotFoundException < CouldNotRetrieveTemplate; end
   class IntermittentRetrievalError < CouldNotRetrieveTemplate; end
+  class SourceWrapperNotFoundError < StandardError; end
 end
diff --git a/lib/slimmer/processors/body_inserter.rb b/lib/slimmer/processors/body_inserter.rb
@@ -10,6 +10,8 @@ def filter(src, dest)
       source_markup = src.at_css(@source_selector)
       destination_markup = dest.at_css(@destination_selector)
 
+      raise(Slimmer::SourceWrapperNotFoundError, "Source wrapper div not found", caller) if source_markup.nil?
+
       css_classes = []
       css_classes << source_markup.attributes["class"].to_s.split(/ +/) if source_markup.has_attribute?("class")
       css_classes << destination_markup.attributes["class"].to_s.split(/ +/) if destination_markup.has_attribute?("class")

diff --git a/lib/slimmer/skin.rb b/lib/slimmer/skin.rb
@@ -122,6 +122,12 @@ def success(source_request, response, body)
 
       template_name = response.headers[Headers::TEMPLATE_HEADER] || "gem_layout"
       process(processors, body, template(template_name), source_request.env)
+    rescue SourceWrapperNotFoundError => e
+      message = "#{e.message} "\
+                "at: #{source_request.base_url}#{source_request.path} "\
+                "length: #{body.to_s.length}\n"\
+                "#{body}"
+      raise SourceWrapperNotFoundError, message, caller
     end
   end
 end
diff --git a/test/processors/body_inserter_test.rb b/test/processors/body_inserter_test.rb
@@ -1,6 +1,18 @@
 require "test_helper"
 
 class BodyInserterTest < Minitest::Test
+  def test_should_raise_source_wrapper_div_not_found_error_when_wrapper_not_found
+    template = as_nokogiri %(
+      <html><body><div><div id="wrapper"></div></div></body></html>
+    )
+    source = as_nokogiri %(
+      <html><body><nav></nav><div><p>this should be moved</p></div></body></html>
+    )
+    assert_raises(Slimmer::SourceWrapperNotFoundError) do
+      Slimmer::Processors::BodyInserter.new.filter(source, template)
+    end
+  end
+
   def test_should_replace_contents_of_wrapper_in_template
     template = as_nokogiri %(
       <html><body><div><div id="wrapper"></div></div></body></html>

diff --git a/test/typical_usage_test.rb b/test/typical_usage_test.rb
@@ -79,6 +79,33 @@ def test_should_set_correct_content_length_header
     end
   end
 
+  class EmptyResponseTest < SlimmerIntegrationTest
+    def test_should_raise_source_wrapper_div_not_found_error
+      assert_raises(Slimmer::SourceWrapperNotFoundError) do
+        given_response 200, ""
+      end
+    end
+  end
+
+  class ResponseWithNoWrapperTest < SlimmerIntegrationTest
+    def test_should_raise_source_wrapper_div_not_found_error
+      assert_raises(Slimmer::SourceWrapperNotFoundError) do
+        given_response 200, %(
+          <html>
+          <head><title>The title of the page</title>
+          <meta name="something" content="yes">
+          <script src="blah.js"></script>
+          <link href="app.css" rel="stylesheet" type="text/css">
+          </head>
+          <body class="body_class">
+          <div>The body of the page</div>
+          </body>
+          </html>
+        )
+      end
+    end
+  end
+
   class NormalResponseTest < SlimmerIntegrationTest
     def setup
       super