Scope middleware rescue block to PDF generation

Limits the handling of exceptions to those occurring in the PDF generation logic,
handling errors from @app.call(env) has the side-effect of burying errors which may
be raised legitimately by other middlewares.

lib/pdfkit/middleware.rb

@@ -19,43 +19,42 @@ def _call(env)
       set_request_to_render_as_pdf(env) if render_as_pdf?
       status, headers, response = @app.call(env)
 
-      if rendering_pdf? && headers['Content-Type'] =~ /text\/html|application\/xhtml\+xml/
-        body = response.respond_to?(:body) ? response.body : response.join
-        body = body.join if body.is_a?(Array)
-
-        root_url = root_url(env)
-        protocol = protocol(env)
-        options = @options.merge(root_url: root_url, protocol: protocol)
-
-        if headers['PDFKit-javascript-delay']
-          options.merge!(javascript_delay: headers.delete('PDFKit-javascript-delay').to_i)
-        end
-
-        body = PDFKit.new(body, options).to_pdf
-        response = [body]
-
-        if headers['PDFKit-save-pdf']
-          File.open(headers['PDFKit-save-pdf'], 'wb') { |file| file.write(body) } rescue nil
-          headers.delete('PDFKit-save-pdf')
+      begin
+        if rendering_pdf? && headers['Content-Type'] =~ /text\/html|application\/xhtml\+xml/
+          body = response.respond_to?(:body) ? response.body : response.join
+          body = body.join if body.is_a?(Array)
+
+          root_url = root_url(env)
+          protocol = protocol(env)
+          options = @options.merge(root_url: root_url, protocol: protocol)
+
+          if headers['PDFKit-javascript-delay']
+            options.merge!(javascript_delay: headers.delete('PDFKit-javascript-delay').to_i)
+          end
+
+          body = PDFKit.new(body, options).to_pdf
+          response = [body]
+
+          if headers['PDFKit-save-pdf']
+            File.open(headers['PDFKit-save-pdf'], 'wb') { |file| file.write(body) } rescue nil
+            headers.delete('PDFKit-save-pdf')
+          end
+
+          unless @caching
+            # Do not cache PDFs
+            headers.delete('ETag')
+            headers.delete('Cache-Control')
+          end
+
+          headers['Content-Length'] = (body.respond_to?(:bytesize) ? body.bytesize : body.size).to_s
+          headers['Content-Type']   = 'application/pdf'
+          headers['Content-Disposition'] ||= @conditions[:disposition] || 'inline'
         end
-
-        unless @caching
-          # Do not cache PDFs
-          headers.delete('ETag')
-          headers.delete('Cache-Control')
-        end
-
-        headers['Content-Length'] = (body.respond_to?(:bytesize) ? body.bytesize : body.size).to_s
-        headers['Content-Type']   = 'application/pdf'
-        headers['Content-Disposition'] ||= @conditions[:disposition] || 'inline'
+      rescue StandardError => e
+        status = 500
+        response = [e.message]
       end
 
-      [status, headers, response]
-
-    rescue StandardError => e
-      status = 500
-      response = [e.message]
-
       [status, headers, response]
     end
 

spec/middleware_spec.rb

@@ -395,12 +395,27 @@ def mock_app(options = {}, conditions = {}, custom_headers = {})
       end
 
       describe "error handling" do
-        specify do
-          mock_app
-          allow(PDFKit).to receive(:new).and_raise(StandardError.new("Something went wrong"))
-          get 'http://www.example.org/public/test.pdf'
-          expect(last_response.status).to eq(500)
-          expect(last_response.body).to eq("Something went wrong")
+        let(:error) { StandardError.new("Something went wrong") }
+
+        context "errors raised by PDF generation" do
+          specify do
+            mock_app
+            allow(PDFKit).to receive(:new).and_raise(error)
+            get 'http://www.example.org/public/test.pdf'
+            expect(last_response.status).to eq(500)
+            expect(last_response.body).to eq(error.message)
+          end
+        end
+
+        context "errors raised upstream" do
+          specify do
+            mock_app
+            allow(@app).to receive(:call).and_raise(error)
+
+            expect {
+              get 'http://www.example.org/public/test.pdf'
+            }.to raise_error(error)
+          end
         end
       end
     end