GSA / notifications-admin

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

Exception Investigation: builtins:IndexError #1397

Closed ccostino closed 1 month ago

ccostino commented 6 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 a user permissions check failing based on one of the other messages seen with the transaction:

has_permissions user: <USER ID> service: <SERVICE ID> returning False

Error message: list index out of range Path: /services//notification/check Exception: app.notify_client:InviteTokenError

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/user.py", line 20, in wrap_func
File "/home/vcap/app/app/main/views/send.py", line 1018, in send_notification

Implementation Sketch and Acceptance Criteria

Security Considerations

terrazoon commented 3 months ago

The last occurrence of this was May 8th, so I suspect we fixed whatever it was. Going to review what deploys took place on May 8th and 9th.

terrazoon commented 3 months ago

Possibly fixed by this commit: https://github.com/GSA/notifications-admin/commit/83481a8b2869065c56d62940bf1f6bf3d64e08be

terrazoon commented 3 months ago

Okay, it was 'fixed' by this commit, but there's a story behind it.

When we moved phone numbers out of the db, all one-off sends had to be converted to jobs

The UI logic is messy here (Andrew has a ticket to separate 'create_job' from 'start_job' but right now they are still combined)

This means that we currently have to poll the backend and wait for the jobs table to be created, etc. This can create timing conditions where the notifications array is still empty but we were trying to redirect with notifications[0] ... leading to the IndexError.

When Bev removed notification_id from the redirect, it fixed the IndexError. What does the user, though, actually see if this error occurs and they are redirected to the jobs page? An empty job? Or is there sufficient time that the job populates.

This issue should be properly resolved when create_job and start_job are finally separated.

ccostino commented 3 months ago

@terrazoon this is still an open issue, right? We've not separated create_job and start_job yet if I remember correctly.

ccostino commented 3 months ago

Actually, given the other ticket in the blocked column, perhaps we need to write a new issue for the splitting apart of those two tasks instead?

ccostino commented 1 month ago

Given that this will likely be resolved when https://github.com/GSA/notifications-api/issues/912 is tackled, I'm closing this in favor of tracking any remaining work in that ticket instead.