balrog should return empty snippets instead of 500s

RESOLVED FIXED

Status

Release Engineering
Balrog: Backend
P3
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: bhearsum, Assigned: bhearsum)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [balrog])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
We did this for 404s, we should do it for 500s too. The trick here is making sure that we log enough information to the application log before we eat the error.
Product: mozilla.org → Release Engineering
(Assignee)

Comment 1

4 years ago
mass component change
Component: General Automation → Balrog: Backend
(Assignee)

Comment 2

4 years ago
This should be simple to implement. We should need to wrap the entirety of ClientRequestView.get (https://github.com/mozilla/balrog/blob/master/auslib/web/views/client.py#L32) in a try/except. In the except block we should make a call to log.exception() and then return an empty snippet. Might be good to factor out what an empty snippet looks like into the business logic.
(Assignee)

Updated

4 years ago
Blocks: 933414
(Assignee)

Comment 3

4 years ago
mass priority change after component move
Priority: -- → P3
(Assignee)

Comment 4

4 years ago
I started trying to implement this and got stuck trying to figure out a way to do it where we still get exceptions logged to Sentry. What I have is here: https://github.com/bhearsum/balrog/commit/6874e5a8325ff61db4476c43b8fc0c62a5766d12
(Assignee)

Comment 5

4 years ago
(In reply to Ben Hearsum [:bhearsum] from comment #4)
> I started trying to implement this and got stuck trying to figure out a way
> to do it where we still get exceptions logged to Sentry. What I have is
> here:
> https://github.com/bhearsum/balrog/commit/
> 6874e5a8325ff61db4476c43b8fc0c62a5766d12

Looks like we can force it to send the exception: http://raven.readthedocs.org/en/latest/config/flask.html#usage

But we need a place to put the Sentry middleware object. Right now it's only available in balrog.wsgi/admin.wsgi...
(Assignee)

Comment 6

4 years ago
Created attachment 8366888 [details] [diff] [review]
send exceptions to sentry then nomnomnom

Turns out that Sentry supports lazy initialization, so we can just instantiate empty Sentry middleware in the library and initialize later. Hooray!
Assignee: nobody → bhearsum
Status: NEW → ASSIGNED
Attachment #8366888 - Flags: review?(nthomas)
Comment on attachment 8366888 [details] [diff] [review]
send exceptions to sentry then nomnomnom

Review of attachment 8366888 [details] [diff] [review]:
-----------------------------------------------------------------

Very whizzybang! I crash!
Attachment #8366888 - Flags: review?(nthomas) → review+

Comment 8

3 years ago
Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/e96baa04e52a8b2302cc81ac4827d144d002f9d1
bug 885173: balrog should return empty snippets instead of 500s. r=nthomas
(Assignee)

Comment 9

3 years ago
Comment on attachment 8366888 [details] [diff] [review]
send exceptions to sentry then nomnomnom

Pushed. Going to wait until there's a few patches in the queue to ask for deployment.
Attachment #8366888 - Flags: checked-in+
(Assignee)

Updated

3 years ago
Depends on: 976081
(Assignee)

Comment 10

3 years ago
Deployed to prod.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.