Commit 8273de31 authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Matthias Käppler

Handle some WebHookService errors

`WebHookWorker` is currently experiencing ~12k unhandled errors per
week.

This MR handles the following errors:

- `EOFError`: `EOFError` can be due to using port 443 without SSL, or
   transient network failures. The SSL configuration problem is unlikely
   because `HTTParty` handles setting this, so it's like this error is
   due to the remote server terminating the connection.
- `ActiveRecord::RecordNotFound`
- `Errno::ENETUNREACH`: handled the same as `Errno::EHOSTUNREACH` is, in
  `Gitlab::HTTP`.

https://gitlab.com/gitlab-org/gitlab/-/issues/337708

Changelog: fixed
parent 3c4d5efb
...@@ -14,9 +14,10 @@ class WebHookWorker ...@@ -14,9 +14,10 @@ class WebHookWorker
worker_has_external_dependencies! worker_has_external_dependencies!
def perform(hook_id, data, hook_name) def perform(hook_id, data, hook_name)
hook = WebHook.find(hook_id) hook = WebHook.find_by_id(hook_id)
data = data.with_indifferent_access return unless hook
data = data.with_indifferent_access
WebHookService.new(hook, data, hook_name, jid).execute WebHookService.new(hook, data, hook_name, jid).execute
end end
end end
......
...@@ -14,7 +14,7 @@ module Gitlab ...@@ -14,7 +14,7 @@ module Gitlab
Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP::ReadTotalTimeout Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP::ReadTotalTimeout
].freeze ].freeze
HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [ HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [
SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError, EOFError, SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError,
Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH,
Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep
].freeze ].freeze
......
...@@ -15,6 +15,10 @@ RSpec.describe WebHookWorker do ...@@ -15,6 +15,10 @@ RSpec.describe WebHookWorker do
subject.perform(project_hook.id, data, hook_name) subject.perform(project_hook.id, data, hook_name)
end end
it 'does not error when the WebHook record cannot be found' do
expect { subject.perform(non_existing_record_id, data, hook_name) }.not_to raise_error
end
it_behaves_like 'worker with data consistency', it_behaves_like 'worker with data consistency',
described_class, described_class,
data_consistency: :delayed data_consistency: :delayed
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment