Avoid giving 400 or 500 upon client error
What does this MR do and why?
Avoid giving 400 or 500 upon client error because that can make us disabling ourselves.
We have several places that can still give 400 or 500:
-
triage/rack/authenticator.rb
- This will give 400 when the token isn't included. Assuming this should never happen, we should be ok with this
- When the token is wrong, it'll give 401, which usually means a mis-configuration. In this case, we need to fix the configuration anyway, it should be fine to keep it. Note that when the webhook got disabled, we'll need a group owner of
gitlab-org
andgitlab-com
to help us re-enable it though!
-
triage/rack/webhook_event.rb
- This will give 400 when the JSON payload is malformed. Assuming this should never happen, this should be ok
-
triage/rack/error_handler.rb
- This will give 500 when there's any exception that we didn't rescue in
triage/rack/processor.rb
, which I think it's fine, too, because it might be possible that we indeed cannot recover from this case. Any known errors should be handled intriage/rack/processor.rb
instead, and in that case, always respond with 200 so we never get disabled due to know client/API issue.
- This will give 500 when there's any exception that we didn't rescue in
Expected impact & dry-runs
These are strongly recommended to assist reviewers and reduce the time to merge your change.
Action items
-
(If applicable) Add documentation to the handbook pages for Triage Operations => - (If applicable) Identify the affected groups and how to communicate to them:
-
/cc @ person_or_group
=> -
Relevant Slack channels => -
Engineering week-in-review
-