feat(l10n): report whole-bundle l10n load failures to Sentry - #21044
Open
LZoog wants to merge 1 commit into
Open
feat(l10n): report whole-bundle l10n load failures to Sentry#21044LZoog wants to merge 1 commit into
LZoog wants to merge 1 commit into
Conversation
Because: - A build regression shipped a manifest with no main.ftl entries, and every locale silently fell back to English for several releases. - Nothing surfaced it: the client logs one console warning per string and emits no telemetry, so it had to be spotted by eye. This commit: - Adds a reportBundleError prop to AppLocalizationProvider, fired when a bundle has no manifest entry, returns a non-OK status, or the manifest itself cannot be loaded. - Sends those to Sentry tagged area:l10n, deduped by message and capped per page load. Individual missing string ids stay with Fluent's reporter. - Buffers reports until sentryMetrics.enable() runs, since bundles resolve before App mounts and Sentry drops events until then. - Returns an empty bundle rather than a non-OK response body, which Fluent would otherwise parse as junk. - Fixes the test manifest fixture, which was invalid JSON with keys that never matched the computed path. Closes #FXA-14362
5 tasks
Contributor
Author
|
This was pulled out of #21043 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Sentry reporting for whole-bundle localization failures while respecting metrics consent.
Changes:
- Detects manifest, mapping, HTTP, and fetch failures.
- Buffers, deduplicates, and caps Sentry reports.
- Adds localization and reporter tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/fxa-settings/src/lib/l10n-error-reporter.ts |
Buffers and submits capped Sentry reports. |
packages/fxa-settings/src/lib/l10n-error-reporter.test.ts |
Tests buffering, deduplication, and limits. |
packages/fxa-settings/src/index.tsx |
Connects localization failures to the reporter. |
packages/fxa-settings/src/contexts/DynamicLocalizationContext.tsx |
Forwards the error callback. |
packages/fxa-settings/src/components/App/index.tsx |
Flushes reports after enabling Sentry. |
packages/fxa-react/lib/AppLocalizationProvider.tsx |
Detects and reports bundle-loading failures. |
packages/fxa-react/lib/AppLocalizationProvider.test.tsx |
Tests manifest and bundle failures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| // The locale is read now rather than at flush time, since a language switch | ||
| // can happen in between. | ||
| const report = { error, locale: document.documentElement.lang }; |
Comment on lines
+51
to
+53
| // A well-formed manifest, served under its own baseDir so it does not | ||
| // collide with the invalid-manifest fixture above. `farewells` is | ||
| // deliberately absent from it, and `notfound` maps to a path that 404s. |
| sentryMetrics.enable(); | ||
| // l10n bundles are fetched before this component can mount, so any | ||
| // failure there is buffered until Sentry will accept it. | ||
| flushL10nErrorReports(); |
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
reportBundleErrorprop toAppLocalizationProvider, fired when a bundle has no manifest entry, returns a non-OK status, or the manifest itself cannot be loaded.area: 'l10n'with the failing locale, deduped by message and capped at 10 per page load.sentryMetrics.enable()runs, flushing from the same effect inAppthat enables it.Issue that this pull request solves
Closes: FXA-14362
Checklist
Put an
xin the boxes that applyHow to review (Optional)
l10n-error-reporter.ts, and the three report sites inAppLocalizationProvider.tsx.AppLocalizationProvider.tsx→l10n-error-reporter.ts→App/index.tsx→index.tsx.DynamicLocalizationContext.tsxis a two-line change plus a whole-file reformat from the pre-commit prettier hook.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
beforeSenddrops events while Sentry is disabled, andAppLocalizationProviderresolves bundles beforeAppcan mount, so without it the feature would report nothing.fetch-mockdoes not serve — the existing tests passed either way beforehand, so a green suite did not prove the mapping was being consulted.