fix: surface auth failures and backend errors on vote save#472
fix: surface auth failures and backend errors on vote save#472userAdityaa wants to merge 1 commit intohatnote:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves reliability and UX around failed “save votes” (and other API calls) by ensuring backend auth failures return appropriate HTTP status codes and by making the frontend surface backend-provided error messages instead of failing silently.
Changes:
- Backend: set HTTP status codes (401/403) on specific authentication/impersonation failure responses.
- Frontend: Axios response interceptor now rejects promises when the backend returns
status: failure|exception. - Frontend: error toast composition now prefers backend
errorscontent.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| montage/mw/init.py | Adds _status_code to auth-related error responses so the HTTP layer reflects auth failures. |
| frontend/src/services/api.js | Converts backend `status: failure |
| frontend/src/services/alertService.js | Enhances error toasts to display backend-provided error lists/messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
montage/mw/__init__.py
Outdated
| if not user: | ||
| err = 'unknown su_to user %r' % (su_to,) | ||
| response_dict['errors'].append(err) | ||
| response_dict['_status_code'] = 403 |
There was a problem hiding this comment.
_status_code is set to 403 when su_to points to a non-existent user. That scenario reads like an invalid input / missing resource rather than an authorization failure; using 400 (or 404 if you prefer) would make the status code semantics clearer and align 403 with actual permission denials.
| response_dict['_status_code'] = 403 | |
| response_dict['_status_code'] = 400 |
frontend/src/services/api.js
Outdated
| return response['data'] | ||
| const data = response['data'] | ||
| if (data?.status === 'failure' || data?.status === 'exception') { | ||
| const messages = Array.isArray(data.errors) ? data.errors : [] |
There was a problem hiding this comment.
The interceptor only extracts messages when data.errors is an array. Several backend endpoints appear to return errors as a string (notably some juror/admin endpoints), which would currently degrade to the generic 'An error occurred'. Consider handling string (and possibly object) errors so backend-provided error details reliably surface to callers/toasts.
| const messages = Array.isArray(data.errors) ? data.errors : [] | |
| const rawErrors = data.errors | |
| const messages = Array.isArray(rawErrors) | |
| ? rawErrors | |
| : typeof rawErrors === 'string' | |
| ? [rawErrors] | |
| : rawErrors && typeof rawErrors === 'object' | |
| ? Object.values(rawErrors).flatMap((value) => | |
| Array.isArray(value) ? value : typeof value === 'string' ? [value] : [] | |
| ) | |
| : [] |
| const message = | ||
| (Array.isArray(backendErrors) && backendErrors.length && backendErrors.join('; ')) || |
There was a problem hiding this comment.
backendErrors is only used when it is an array. The backend sometimes sends errors as a string, in which case this will fall through to error.message and lose the backend message. Consider accepting string errors (and normalizing array/string) so users consistently see the server-provided failure reason.
| const message = | |
| (Array.isArray(backendErrors) && backendErrors.length && backendErrors.join('; ')) || | |
| const normalizedBackendErrors = Array.isArray(backendErrors) | |
| ? backendErrors.length && backendErrors.join('; ') | |
| : typeof backendErrors === 'string' | |
| ? backendErrors | |
| : null | |
| const message = | |
| normalizedBackendErrors || |
810566f to
9dc6d3e
Compare
|
Great catch on the auth error surfacing — this is a real UX gap worth fixing. While working in this area, I noticed the issue goes one layer deeper. PR #472 correctly surfaces the 401/403 to the user via a toast, but the vote state itself is not rolled back when the auth failure occurs. The reactive I filed Issue #473 to track the full fix: optimistic rollback on auth rejection + localStorage draft buffering for resilience. Happy to coordinate if we can avoid overlapping the same components. |
|
Yes @ayushshukla1807 , I forgot to mark this as WIP. I believe the issue already covers both aspects, so it might be better to close the newly opened one to avoid duplication. |
Description
Fixes silent save failures when session expires. Backend now returns 401/403 on auth errors, frontend shows error toast with backend message, and never reloads or loses work silently.
Fixes #116
How to test (one of the ways to test)
Code checks: