Merge pull request #1872 from adjam/issue-1847-jruby-swallows-sax-exc…

…eptions

Rethrow exceptions caught by Java SAX handler

ext/java/nokogiri/internals/NokogiriHandler.java

@@ -246,27 +246,25 @@ public void endCDATA() {
         charactersBuilder.setLength(0);
     }
 
-    @Override
-    public void error(SAXParseException ex) {
+    void handleError(SAXParseException ex) {
         try {
             final String msg = ex.getMessage();
             call("error", runtime.newString(msg == null ? "" : msg));
             addError(new RaiseException(XmlSyntaxError.createError(runtime, ex), true));
-        } catch(RaiseException e) {
+        } catch( RaiseException e) {
             addError(e);
+            throw e;
         }
     }
 
     @Override
-    public void fatalError(SAXParseException ex) {
-        try {
-            final String msg = ex.getMessage();
-            call("error", runtime.newString(msg == null ? "" : msg));
-            addError(new RaiseException(XmlSyntaxError.createFatalError(runtime, ex), true));
+    public void error(SAXParseException ex) {
+        handleError(ex);
+    }
 
-        } catch(RaiseException e) {
-            addError(e);
-        }
+    @Override
+    public void fatalError(SAXParseException ex) {
+        handleError(ex);
     }
 
     @Override

test/xml/sax/test_document_error.rb

@@ -0,0 +1,55 @@
+require 'helper'
+
+module Nokogiri
+  module XML
+    module SAX
+
+      # raises an exception when underlying parser
+      # encounters an XML parsing error
+      class ThrowingErrorDocument < Document
+        def error(msg)
+          raise(StandardError, "parsing did not complete: #{msg}")
+        end
+      end
+
+      # only warns when underlying parser encounters
+      # an XML parsing error
+      class WarningErrorDocument < Document
+        def error(msg)
+          errors << msg
+         end
+
+        def errors
+          @errors ||= []
+        end
+      end
+
+      class TestErrorHandling < Nokogiri::SAX::TestCase 
+        def setup
+          super
+          @error_parser = Parser.new(ThrowingErrorDocument.new)
+          @warning_parser = Parser.new(WarningErrorDocument.new)
+        end
+
+        def test_error_throwing_document_raises_exception
+          begin
+            @error_parser.parse("<xml>") # no closing element
+            fail '#parse  should not complete successfully when document #error throws exception'
+          rescue StandardError => e
+            assert_match /parsing did not complete/, e.message
+          end
+        end
+
+        def test_warning_document_encounters_error_but_terminates_normally
+          begin
+            @warning_parser.parse("<xml>")
+            assert !@warning_parser.document.errors.empty?, 'error collector did not collect an error'
+          rescue StandardError => e
+            warn(e)
+            fail '#parse should complete successfully unless document #error throws exception (#{e}'
+          end
+        end
+      end
+    end
+  end
+end

test/xml/sax/test_push_parser.rb

@@ -73,6 +73,7 @@ def error msg
           rescue => e
             actual = e
           end
+          fail 'PushParser should throw error when fed ill-formed data' if actual.nil?
 
           assert_equal actual.message, "parse error"
         end