4482 – Handle Archive.org HTML errors (#533)

vasconsaurus · web-flow · commit b90a9a5cc42d · 2025-03-19T14:20:40.000-03:00
Context - Sometimes ArchiveOrg will send a HTML response: - which are usually issues on their side like: “429 Too Many Requests” and “Item Not Avaiable”, - but also caused an error on our side, since we always expected a JSON. - We have two jobs that run for ArchiveOrg, both make requests, so this happens at different places/time, but need very similar error handling. - We also wanted to take this opportunity to try to centralize more our error handling. How - Handling HTML errors - I extracted a method json_parse we can use for our both requests. It rescues JSON::ParserError, and re-raises it as a more specific error or as itself. - Then handle_archiving_exceptions deals with it. - Other errors - To make this more cohesive, I updated this to have the same behavior as when we are dealing with HTML errors, - so we also re-raise the errors and handle_archiving_exceptions deals with it. - On handle_archiving_exceptions: - We had been dealing with ArchiveOrg errors directly in its module, but even if they are more ArchiveOrg related, I think it makes sense to centralize everything in handle_archiving_exceptions. - I did some work before where we moved error handling inside get_archive_org_status - 28941ec - I think this might not be needed, - but even if it is, I think it might make more sense for us to wrap this job with handle_archiving_exceptions? - It would be easier for us to avoid duplication. References: CV2-4482 PR 533
diff --git a/app/models/concerns/media_archive_org_archiver.rb b/app/models/concerns/media_archive_org_archiver.rb
@@ -25,36 +25,17 @@ def send_to_archive_org(url, key_id, _supported = nil)
 
         Rails.logger.info level: 'INFO', message: '[archive_org] Sent URL to archive', url: url, code: response.code, response: response.message
 
-        if response&.body&.include?("<html><body><h1>429 Too Many Requests</h1>") && response&.code == "500"
-          data = snapshot_data.to_h.merge({ error: { message: "(#{response.body}) #{response}", code: Lapis::ErrorCodes::const_get('ARCHIVER_ERROR') }})
-          Media.notify_webhook_and_update_cache('archive_org', url, data, key_id)
-          PenderSentry.notify(
-              Pender::Exception::RateLimitExceeded.new("429 Too Many Requests"),
-              url: url,
-              response_body: response.body
-            )
-          return
-        end
-
-        body = JSON.parse(response.body)
+        body = json_parse(response)
         if body['job_id']
           ArchiverStatusJob.perform_in(2.minutes, body['job_id'], url, key_id)
         else
           data = snapshot_data.to_h.merge({ error: { message: "(#{body['status_ext']}) #{body['message']}", code: Lapis::ErrorCodes::const_get('ARCHIVER_ERROR') }})
           Media.notify_webhook_and_update_cache('archive_org', url, data, key_id)
 
-         if body['message']&.include?('The same snapshot') || body['status_ext'] == 'error:too-many-daily-captures'
-            PenderSentry.notify(
-              Pender::Exception::TooManyCaptures.new(body['message']),
-              url: url,
-              response_body: body
-            )
+          if body['message']&.include?('The same snapshot') || body['status_ext'] == 'error:too-many-daily-captures'
+            raise Pender::Exception::TooManyCaptures, body['message']
           elsif body['status_ext'] == 'error:blocked-url'
-            PenderSentry.notify(
-              Pender::Exception::BlockedUrl.new(body['message']),
-              url: url,
-              response_body: body
-            )
+            raise Pender::Exception::BlockedUrl, body['message']
           else
             raise Pender::Exception::ArchiveOrgError, "(#{body['status_ext']}) #{body['message']}"
           end
@@ -78,20 +59,17 @@ def get_available_archive_org_snapshot(url, key_id)
     end
 
     def get_archive_org_status(job_id, url, key_id)
-      begin
-        http, request = Media.archive_org_request("https://web.archive.org/save/status/#{job_id}", 'Get')
-        response = http.request(request)
-        body = JSON.parse(response.body)
-        if body['status'] == 'success'
-         location = "https://web.archive.org/web/#{body['timestamp']}/#{url}"
-          data = { location: location }
-          Media.notify_webhook_and_update_cache('archive_org', url, data, key_id)
-        else
-          message = body['status'] == 'pending' ? 'Capture is pending' : "(#{body['status_ext']}) #{body['message']}"
-          raise Pender::Exception::RetryLater, message
-        end
-      rescue StandardError => error
-        raise Pender::Exception::RetryLater, error.message
+      http, request = Media.archive_org_request("https://web.archive.org/save/status/#{job_id}", 'Get')
+      response = http.request(request)
+      body = json_parse(response)
+
+      if body['status'] == 'success'
+        location = "https://web.archive.org/web/#{body['timestamp']}/#{url}"
+        data = { location: location }
+        Media.notify_webhook_and_update_cache('archive_org', url, data, key_id)
+      else
+        message = body['status'] == 'pending' ? 'Capture is pending' : "(#{body['status_ext']}) #{body['message']}"
+        raise Pender::Exception::RetryLater, message
       end
     end
 
@@ -106,5 +84,19 @@ def archive_org_request(request_url, verb)
       }
       [http, "Net::HTTP::#{verb}".constantize.new(uri, headers)]
     end
+
+    def json_parse(response)
+      begin
+        JSON.parse(response.body)
+      rescue JSON::ParserError => error
+        if error.message.include?("Too Many Requests")
+          raise Pender::Exception::RateLimitExceeded, error.message
+        elsif error.message.include?("Item Not Available")
+          raise Pender::Exception::ItemNotAvailable, error.message
+        else
+          raise JSON::ParserError, error.message
+        end
+      end
+    end
   end
 end
diff --git a/app/models/concerns/media_archiver.rb b/app/models/concerns/media_archiver.rb
@@ -95,15 +95,38 @@ def handle_archiving_exceptions(archiver, params)
         yield
       rescue Pender::Exception::RetryLater => error
         retry_archiving_after_failure(archiver, { message: error.message })
+      rescue Pender::Exception::BlockedUrl,
+             Pender::Exception::TooManyCaptures,
+             Pender::Exception::ItemNotAvailable,
+             Pender::Exception::RateLimitExceeded,
+             JSON::ParserError => error
+              post_error_tasks(archiver, params, error)
       rescue StandardError => error
-        error_type = 'ARCHIVER_ERROR'
-        params.merge!({code: Lapis::ErrorCodes::const_get(error_type), message: error.message})
-        data = { error: { message: params[:message], code: Lapis::ErrorCodes::const_get(error_type) }}
-        Media.notify_webhook_and_update_cache(archiver, params[:url], data, params[:key_id])
+        post_error_tasks(archiver, params, error, false)
         retry_archiving_after_failure(archiver, params)
       end
     end
 
+    def post_error_tasks(archiver, params, error, notify_sentry = true)
+      error_type = 'ARCHIVER_ERROR'
+      if notify_sentry then Media.notify_sentry(archiver, params[:url], error) end
+      data = Media.updated_errored_data(archiver, params, error, error_type = 'ARCHIVER_ERROR')
+      Media.notify_webhook_and_update_cache(archiver, params[:url], data, params[:key_id])
+    end
+
+    def notify_sentry(archiver, url, error)
+      PenderSentry.notify(
+        error.class.new("#{archiver}: #{error.message}"),
+        url: url,
+        response_body: error.message
+      )
+    end
+
+    def updated_errored_data(archiver, params, error, error_type = 'ARCHIVER_ERROR')
+      params.merge!({code: Lapis::ErrorCodes::const_get(error_type), message: error.message})
+      { error: { message: params[:message], code: Lapis::ErrorCodes::const_get(error_type) }}
+    end
+
     def retry_archiving_after_failure(archiver, params)
       Rails.logger.warn level: 'WARN', message: "#{params[:message]}", url: params[:url], archiver: archiver, error_code: params[:code], error_message: params[:message]
       raise Pender::Exception::RetryLater, "[#{archiver}]: #{params[:message]}"
diff --git a/lib/pender/exception/item_not_available.rb b/lib/pender/exception/item_not_available.rb
@@ -0,0 +1,5 @@
+module Pender
+  module Exception
+    class ItemNotAvailable < StandardError; end
+  end
+end
diff --git a/test/models/archiver_test.rb b/test/models/archiver_test.rb
@@ -159,7 +159,7 @@ def create_api_key_with_webhook_for_perma_cc
       end
   end
 
-  test "when Archive.org fails with Pender::Exception::ArchiveOrgError it should retry, update data with snapshot (if available) and error" do
+  test "when Archive.org fails with Pender::Exception::ArchiveOrgError it should not notify Sentry, it should retry, update data with snapshot (if available) and error" do
     api_key = create_api_key_with_webhook
     url = 'https://example.com/'
 
@@ -171,10 +171,19 @@ def create_api_key_with_webhook_for_perma_cc
     WebMock.stub_request(:post, /example.com\/webhook/).to_return(status: 200, body: '')
     WebMock.stub_request(:post, /web.archive.org\/save/).to_return_json(status: 500, body: { status_ext: '500', message: 'Random Error.', url: url})
 
+    sentry_call_count = 0
+    arguments_checker = Proc.new do |e|
+      sentry_call_count += 1
+    end
+
     media = create_media url: url, key: api_key
-    assert_raises StandardError do
-      media.as_json(archivers: 'archive_org')
+    PenderSentry.stub(:notify, arguments_checker) do
+      assert_raises StandardError do
+        media.as_json(archivers: 'archive_org')
+      end
     end
+
+    assert_equal 0, sentry_call_count
     media_data = Pender::Store.current.read(Media.get_id(url), :json)
     assert_equal '(500) Random Error.', media_data.dig('archives', 'archive_org', 'error', 'message') 
     assert_equal "https://web.archive.org/web/timestamp/#{url}", media_data.dig('archives', 'archive_org', 'location') 
@@ -729,25 +738,27 @@ def create_api_key_with_webhook_for_perma_cc
     assert_equal({ 'location' => 'http://perma.cc/perma-cc-guid-1'}, cached['perma_cc'])
   end
 
-  test "when Archive.org returns 429 Too Many Requests it should notify Sentry with RateLimitExceeded" do
+  test "when Archive.org status returns 429 Too Many Requests it should notify Sentry with RateLimitExceeded" do
     api_key = create_api_key_with_webhook
     url = 'https://example.com/'
 
     Media.any_instance.unstub(:archive_to_archive_org)
-    Media.stubs(:get_available_archive_org_snapshot).returns({ status_ext: 'error:rate-limit', message: '429 Too Many Requests', url: url })
-  
+
     WebMock.stub_request(:get, url).to_return(status: 200, body: '<html>A page</html>')
     WebMock.stub_request(:post, /safebrowsing\.googleapis\.com/).to_return(status: 200, body: '{}')
     WebMock.stub_request(:post, /example.com\/webhook/).to_return(status: 200, body: '')
-    WebMock.stub_request(:post, /archive.org\/save/).to_return(status: 500, body: '<html><body><h1>429 Too Many Requests</h1>')
+    # This response comes from ArchiveStatusJob, in order to call it we need to get a job_id
+    WebMock.stub_request(:post, /web.archive.org\/save/).to_return_json(body: {url: url, job_id: 'ebb13d31-7fcf-4dce-890c-c256e2823ca0' })
+    WebMock.stub_request(:get, /archive.org\/wayback/).to_return_json(body: {"archived_snapshots":{}}, headers: {})
+    WebMock.stub_request(:get, /archive.org\/save\/status/).to_return(body:  '429 Too Many Requests')
 
     m = Media.new url: url, key: api_key
 
     sentry_call_count = 0
     arguments_checker = Proc.new do |e|
       sentry_call_count += 1
       assert_instance_of Pender::Exception::RateLimitExceeded, e
-      assert_equal '429 Too Many Requests', e.message
+      assert_includes e.message, 'Too Many Requests'
     end
 
     PenderSentry.stub(:notify, arguments_checker) do
@@ -759,8 +770,75 @@ def create_api_key_with_webhook_for_perma_cc
     assert_equal 1, sentry_call_count
 
     media_data = Pender::Store.current.read(Media.get_id(url), :json)
-    expected_error_message = "<html><body><h1>429 Too Many Requests</h1>"
+    expected_error_message = "Too Many Requests"
     assert_includes media_data.dig('archives', 'archive_org', 'error', 'message'), expected_error_message
     assert_equal Lapis::ErrorCodes::const_get('ARCHIVER_ERROR'), media_data.dig('archives', 'archive_org', 'error', 'code')
-  end  
+  end
+
+  test "when Archive.org returns HTML response it should notify Sentry with JSON::ParserError" do
+    api_key = create_api_key_with_webhook
+    url = 'https://example.com/'
+
+    Media.any_instance.unstub(:archive_to_archive_org)
+
+    WebMock.stub_request(:get, url).to_return(status: 200, body: '<html>A page</html>')
+    WebMock.stub_request(:post, /safebrowsing\.googleapis\.com/).to_return(status: 200, body: '{}')
+    WebMock.stub_request(:post, /example.com\/webhook/).to_return(status: 200, body: '')
+    WebMock.stub_request(:post, /web.archive.org\/save/).to_return_json(body: '<html>A html response</html>' )
+    WebMock.stub_request(:get, /archive.org\/wayback/).to_return_json(body: {"archived_snapshots":{}}, headers: {})
+
+    m = Media.new url: url, key: api_key
+
+    sentry_call_count = 0
+    arguments_checker = Proc.new do |e|
+      sentry_call_count += 1
+      assert_instance_of JSON::ParserError, e
+    end
+
+    PenderSentry.stub(:notify, arguments_checker) do
+      assert_nothing_raised do
+        m.as_json(archivers: 'archive_org')
+      end
+    end
+
+    assert_equal 1, sentry_call_count
+
+    media_data = Pender::Store.current.read(Media.get_id(url), :json)
+    assert_equal Lapis::ErrorCodes::const_get('ARCHIVER_ERROR'), media_data.dig('archives', 'archive_org', 'error', 'code')
+  end
+
+  test "when Archive.org returns 'Item Not Available' response it should notify Sentry with Pender::Exception::ItemNotAvailable" do
+    api_key = create_api_key_with_webhook
+    url = 'https://example.com/'
+
+    Media.any_instance.unstub(:archive_to_archive_org)
+
+    WebMock.stub_request(:get, url).to_return(status: 200, body: '<html>A page</html>')
+    WebMock.stub_request(:post, /safebrowsing\.googleapis\.com/).to_return(status: 200, body: '{}')
+    WebMock.stub_request(:post, /example.com\/webhook/).to_return(status: 200, body: '')
+    WebMock.stub_request(:post, /web.archive.org\/save/).to_return_json(body: 'Item Not Available' )
+    WebMock.stub_request(:get, /archive.org\/wayback/).to_return_json(body: {"archived_snapshots":{}}, headers: {})
+
+    m = Media.new url: url, key: api_key
+
+    sentry_call_count = 0
+    arguments_checker = Proc.new do |e|
+      sentry_call_count += 1
+      assert_instance_of Pender::Exception::ItemNotAvailable, e
+      assert_includes e.message, 'Item Not Available'
+    end
+
+    PenderSentry.stub(:notify, arguments_checker) do
+      assert_nothing_raised do
+        m.as_json(archivers: 'archive_org')
+      end
+    end
+
+    assert_equal 1, sentry_call_count
+
+    media_data = Pender::Store.current.read(Media.get_id(url), :json)
+    expected_error_message = "Item Not Available"
+    assert_includes media_data.dig('archives', 'archive_org', 'error', 'message'), expected_error_message
+    assert_equal Lapis::ErrorCodes::const_get('ARCHIVER_ERROR'), media_data.dig('archives', 'archive_org', 'error', 'code')
+  end
 end
