1541581 - Loading /rules while logged out causes a 500

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (frontend), defect, P1)

Description

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: Screenshot

If you're not logged in while deep linking into /rules in Balrog admin, there's a request to
https://aus4-admin.mozilla.org/api/users/null
which causes a 500 response. There's a dialog the user has to dismiss (see screenshot). More details on the sentry issue with traceback etc.

Interestingly /releases and a few other endpoints I tried are OK, so there's something specific to rules here - possibly looking up signoff roles.

Has our model changed now and I should retrain myself to land at / first, or can we do something more like taskcluster and aggressively attempt to login ? Or maybe defer the user lookup until the scheduled change UI is used.

Comment 1

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #0)

Interestingly /releases and a few other endpoints I tried are OK, so there's
something specific to rules here - possibly looking up signoff roles.

Yup...that's what's causing it.

Has our model changed now and I should retrain myself to land at / first, or
can we do something more like taskcluster and aggressively attempt to login
? Or maybe defer the user lookup until the scheduled change UI is used.

I don't think it's reasonable to expect everyone to go to / first when we support deep linking into /rules. I haven't experienced this myself after logging in for the first time (it sounds like you are), but I think losing race is possible here, so I guess it's not too surprising.

Blocking the user lookup (and anything else that requires an access token) on the token renewal makes a lot of sense to me, shouldn't be too difficult, either.

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Looks like there's a difference between expired token and logged out. After waiting the two hours (was expecting 15 min in prod) I didn't hit the 500, whereas a manual logout does. Priority-- ?

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #2)

Looks like there's a difference between expired token and logged out. After
waiting the two hours (was expecting 15 min in prod) I didn't hit the 500,
whereas a manual logout does. Priority-- ?

Hm...this is probably because we remove "username" from localStorage when you logout, which won't happen if you wait for a timeout (you'll still have everything we put in there - the access token just won't be valid anymore). Maybe the most gracefully way to handle this is simply to not request user roles if the username is null? Perhaps we should hide all of the modification buttons, too, which should make it impossible for a user to get to a spot in the UI that requires roles.

Comment 4

[Nick Thomas [:nthomas] (UTC+12)]

That's probably the best way forward if we want to have a readonly/logged-out UI. Probably requires auditing/modifying each bit of UI though. I think I saw /releases/scheduled_changes looking up roles when there was a pending release change yesterday.

Comment 5

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #4)

That's probably the best way forward if we want to have a
readonly/logged-out UI. Probably requires auditing/modifying each bit of UI
though. I think I saw /releases/scheduled_changes looking up roles when
there was a pending release change yesterday.

We might be able to simplify this bit with a quick hack in the Permissions service to turn requests to /users/null into a no-op...in any case, I'll see that this gets fixed.

Comment 6

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/balrog/pull/891

Comment 7

[[e-mail] (:bhearsum)]

This is now in prod.