GSA / notifications-admin

The UI of Notify.gov
https://notify.gov
Other
11 stars 2 forks source link

Exception Investigation: notifications_python_client.errors:HTTPError #1393

Closed ccostino closed 5 months ago

ccostino commented 7 months ago

This is one of the errors we've seen captured in New Relic that we'd like to dig into and understand, if not also resolve.

This one appears to be related to a Login.gov sign-in attempt failure because this was also captured as a part of the transaction:

API POST request on https://notify-api-production.apps.internal:61443/user/get-login-gov-user failed with 500 'Internal server error'

Error message: 500 - Internal server error Path: /sign-in Exception: notifications_python_client.errors:HTTPError

Traceback (most recent call last):
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/eventlet/greenthread.py", line 221, in main
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/gunicorn/workers/geventlet.py", line 157, in handle
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/gunicorn/workers/base_async.py", line 55, in handle
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/gunicorn/workers/base_async.py", line 108, in handle_request
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/newrelic/api/wsgi_application.py", line 669, in _nr_wsgi_application_wrapper_
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 2213, in __call__
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/request_helper.py", line 80, in __call__
File "/home/vcap/app/app/proxy_fix.py", line 11, in __call__
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/werkzeug/middleware/proxy_fix.py", line 182, in __call__
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/newrelic/api/wsgi_application.py", line 564, in _nr_wsgi_application_wrapper_
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 2190, in wsgi_app
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1484, in full_dispatch_request
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1469, in dispatch_request
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/newrelic/hooks/framework_flask.py", line 82, in _nr_wrapper_handler_
File "/home/vcap/app/app/utils/__init__.py", line 113, in decorated_function
File "/home/vcap/app/app/main/views/sign_in.py", line 93, in sign_in
File "/home/vcap/app/app/notify_client/user_api_client.py", line 49, in get_user_by_uuid_or_email
File "/home/vcap/app/app/notify_client/__init__.py", line 59, in post
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 44, in post
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 60, in request
File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_python_client/base.py", line 99, in _perform_request

Implementation Sketch and Acceptance Criteria

Security Considerations

terrazoon commented 6 months ago

I think this is no longer occurring.

Back in march, the sign in code was optimistic. We assumed that, if given an email_address and a login.gov uuid, we could look up the user in the database. Or if the user wasn't there, they query would return a null result set in some well-behaved way.

But if you look at a similar method in user_api_client (get_user_by_email_or_none) we should have explicitly accounted for an HTTPError if we tried to look up something that didn't exist.

We are now accounting for, in a slightly different way. The backend does a try/except and returns None, and if the front end gets None, it does an abort(401). So there should be no stack trace for this, and it should not be reproducible.

ecayer commented 5 months ago

Sounds good! Closing (pairing with @ccostino :-) )