Conversation
bcolsson
approved these changes
Jun 10, 2026
dschom
reviewed
Jun 11, 2026
| </div> | ||
| </div> | ||
|
|
||
| {error?.message && ( |
Contributor
There was a problem hiding this comment.
Small nit. It'd be good to also print the errno and code if they are available, since some auth ui errors have identical messages.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR redesigns the application error dialog to provide clearer user-facing messaging and to surface a Sentry “Error ID” that support/engineering can use to correlate user reports with Sentry events. It also threads Sentry event IDs through error capture paths (OAuth + app error boundaries) and expands unit/functional test coverage around those flows.
Changes:
- Redesign
AppErrorDialogUI/copy (including dark mode) and add optional “Error ID” + expandable “Error details”. - Capture and propagate Sentry event IDs through
AppErrorBoundary(fxa-react + fxa-settings) and OAuth error flows to avoid double-reporting. - Add/expand unit and functional tests for error boundaries, OAuth error pages, and a clientInfo loading race regression.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/models/integrations/oauth-web-integration.ts | Attach Sentry event ID to thrown OAuthError after capture. |
| packages/fxa-settings/src/models/integrations/oauth-web-integration.test.ts | Add coverage for “empty husk” clientInfo race behavior and related error capture. |
| packages/fxa-settings/src/models/hooks.test.ts | Add regression test ensuring useClientInfoState reports loading on first render when fetch will run. |
| packages/fxa-settings/src/lib/oauth/oauth-errors.ts | Extend AuthError/OAuthError to carry sentryEventId. |
| packages/fxa-settings/src/components/OAuthDataError/index.tsx | Capture/reuse Sentry event ID and display it on the OAuth error page. |
| packages/fxa-settings/src/components/OAuthDataError/index.test.tsx | Add unit tests for Sentry event ID capture/reuse and rendering. |
| packages/fxa-settings/src/components/ErrorBoundaries/index.tsx | Capture Sentry event ID in boundary and pass it to AppErrorDialog. |
| packages/fxa-settings/src/components/ErrorBoundaries/index.test.tsx | Add unit tests asserting error ID display and query-parameter heading. |
| packages/fxa-react/components/AppErrorDialog/index.tsx | Redesign dialog markup/styles, add error ID + details toggle, update copy/FTL IDs. |
| packages/fxa-react/components/AppErrorDialog/index.test.tsx | Update tests for new headings/error ID/details UI. |
| packages/fxa-react/components/AppErrorDialog/en.ftl | Add new localized strings for redesigned dialog (heading/message/error ID/details). |
| packages/fxa-react/components/AppErrorBoundary/index.tsx | Capture Sentry event ID and pass it into the dialog. |
| packages/functional-tests/tests/misc/fourOhFour.spec.ts | Remove old 404-only functional test file (renamed/expanded). |
| packages/functional-tests/tests/misc/errorViews.spec.ts | Add consolidated error views functional tests, including invalid query param rendering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| clientInfo: this.clientInfo || 'undefined', | ||
| }; | ||
| Sentry.captureException(err, { | ||
| err.sentryEventId = Sentry.captureException(err, { |
Comment on lines
+24
to
+32
| // Prefer the ID from the original capture site; capture here only for | ||
| // errors not already reported, so one error never emits two events. | ||
| const [sentryEventId] = useState( | ||
| () => | ||
| error.sentryEventId ?? | ||
| Sentry.captureException(new Error(error.message), { | ||
| tags: { source: 'OAuthDataError', errno: error.errno }, | ||
| }) | ||
| ); |
Comment on lines
+57
to
+61
| <FtlMsg id="app-error-id" vars={{ errorId: sentryEventId }}> | ||
| <p className="mt-6 text-center text-xs text-grey-400 dark:text-grey-300"> | ||
| {`Error ID: ${sentryEventId}`} | ||
| </p> | ||
| </FtlMsg> |
Comment on lines
+3
to
+8
| app-something-went-wrong-heading = Something went wrong | ||
| app-something-went-wrong-message = We’ve been notified of the issue. Refresh the page to try again. | ||
| # $errorId (String) - Unique identifier for the error report, used to look it up in our monitoring system | ||
| app-error-id = Error ID: { $errorId } | ||
| # Expandable toggle that reveals technical details about the error | ||
| app-error-details-summary = Error details |
Comment on lines
+46
to
+50
| <Localized id="app-something-went-wrong-message"> | ||
| <p className="mt-3 text-sm text-grey-500 dark:text-grey-200"> | ||
| We’ve been notified of the issue. Refresh the page to try again. | ||
| </p> | ||
| </Localized> |
Comment on lines
+44
to
55
| it('reveals the error message when the error details toggle is expanded', async () => { | ||
| const user = userEvent.setup(); | ||
| renderWithLocalizationProvider( | ||
| <AppErrorDialog error={new Error('Invalid redirect parameter')} /> | ||
| ); | ||
|
|
||
| expect(screen.getByText('Invalid redirect parameter')).not.toBeVisible(); | ||
|
|
||
| await user.click(screen.getByText('Error details')); | ||
|
|
||
| expect(screen.getByText('Invalid redirect parameter')).toBeVisible(); | ||
| }); |
Because: * The app error dialog needed to surface a Sentry error ID and present a clearer, redesigned UI for unexpected failures. This commit: * Redesigns AppErrorDialog to display the captured Sentry event ID. * Adds a functional test covering the app error view for an invalid query parameter and folds the 404 test into a shared errorViews spec. * Adds supporting model hook and OAuth web integration unit tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Because
This pull request
AppErrorDialog(fxa-react) with refreshed copy ("Something went wrong"), dark-mode styling, an expandable "Error details" toggle, and a displayed Error ID.AppErrorBoundarycomponents (fxa-reactandfxa-settings/ErrorBoundaries) and threads it into the dialog.OAuthDataErrorto surface the Error ID and reuse an ID already captured at the throw site, so one error never emits two Sentry events.sentryEventIdto theAuthErrortype andOAuthErrorclass (oauth-errors.ts);oauth-web-integration.tsnow stamps it on captured errors.en.ftlstrings:app-something-went-wrong-*,app-error-id,app-error-details-summary.AppErrorDialog,ErrorBoundaries,OAuthDataError, auseClientInfoStaterace regression test, andoauth-web-integration"empty clientInfo" coverage.fourOhFour.spec.ts→errorViews.spec.tsand adds a functional test asserting the app error view renders for an invalid query parameter.Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13938
Checklist
Other information
How to test:
/?redirect_to=javascript:alert(1)— the redesigned "Bad Request" error view should render instead of the signin form.nx test-unit fxa-settingsandnx test-unit fxa-react; functional:errorViews.spec.ts(verified passing locally).I was reallyl trying to keep this PR small...but the tests kind of ballooned since its adding missing coverage for these files.