Merge remote-tracking branch 'upstream/pull/5226'

diff --git a/app/jobs/trace_importer_job.rb b/app/jobs/trace_importer_job.rb
index 940dd6c7907a39972ff2f261d2091438e1d7c565..48285cbc1ebd60273f108cdf526f8eba5e7a6b8c 100644 (file)
--- a/app/jobs/trace_importer_job.rb
+++ b/app/jobs/trace_importer_job.rb
@@ -10,6 +10,10 @@ class TraceImporterJob < ApplicationJob
       UserMailer.gpx_failure(trace, "0 points parsed ok. Do they all have lat,lng,alt,timestamp?").deliver
       trace.destroy
     end
+  rescue XML::Error => e
+    logger.info e.to_s
+    UserMailer.gpx_failure(trace, e).deliver
+    trace.destroy
   rescue StandardError => e
     logger.info e.to_s
     e.backtrace.each { |l| logger.info l }

diff --git a/lib/gpx.rb b/lib/gpx.rb
index 3e1cb9afa6d9cf6da4ca51d80255576c3f25f309..1bf7764ec5e3052f878c394f4ae75f9e1c9e879c 100644 (file)
--- a/lib/gpx.rb
+++ b/lib/gpx.rb
@@ -57,7 +57,7 @@ module GPX
         when "application/x-bzip" then io = Bzip2::FFI::Reader.open(@file)
         end
 
-        parse_file(XML::Reader.io(io), &block)
+        parse_file(XML::Reader.io(io, :options => XML::Parser::Options::NOERROR), &block)
       end
     end
 

diff --git a/test/gpx/fixtures/jpg.gpx b/test/gpx/fixtures/jpg.gpx
new file mode 100644 (file)
index 0000000..12dc8c4
Binary files /dev/null and b/test/gpx/fixtures/jpg.gpx differ

diff --git a/test/gpx/fixtures/jpg.gz.gpx b/test/gpx/fixtures/jpg.gz.gpx
new file mode 100644 (file)
index 0000000..2742808
Binary files /dev/null and b/test/gpx/fixtures/jpg.gz.gpx differ

diff --git a/test/jobs/trace_importer_job_test.rb b/test/jobs/trace_importer_job_test.rb
index e1ddcc9679cbd1161a366bffd20c6fdc6fbc4cb8..c6d44ecce22185fc3269db737261ba86625b3993 100644 (file)
--- a/test/jobs/trace_importer_job_test.rb
+++ b/test/jobs/trace_importer_job_test.rb
@@ -52,6 +52,38 @@ class TraceImporterJobTest < ActiveJob::TestCase
     email = ActionMailer::Base.deliveries.last
     assert_equal trace.user.email, email.to[0]
     assert_match(/failure/, email.subject)
+    assert_no_match(/Start tag expected/, email.text_part.body.to_s, "should not include parser error")
+    assert_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should include stack backtrace")
+
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_parse_error_notification
+    trace = create(:trace, :inserted => false, :fixture => "jpg")
+    Rails.logger.silence do
+      TraceImporterJob.perform_now(trace)
+    end
+
+    email = ActionMailer::Base.deliveries.last
+    assert_equal trace.user.email, email.to[0]
+    assert_match(/failure/, email.subject)
+    assert_match(/Start tag expected/, email.text_part.body.to_s, "should include parser error")
+    assert_no_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should not include stack backtrace")
+
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def test_gz_parse_error_notification
+    trace = create(:trace, :inserted => false, :fixture => "jpg.gz")
+    Rails.logger.silence do
+      TraceImporterJob.perform_now(trace)
+    end
+
+    email = ActionMailer::Base.deliveries.last
+    assert_equal trace.user.email, email.to[0]
+    assert_match(/failure/, email.subject)
+    assert_match(/Start tag expected/, email.text_part.body.to_s, "should include parser error")
+    assert_no_match(%r{jobs/trace_importer_job\.rb}, email.text_part.body.to_s, "should not include stack backtrace")
 
     ActionMailer::Base.deliveries.clear
   end