site-kit-wp: Prevent dashboard from crashing when async SVG fails to load
Bug Description
There have been a couple of users who reported a Loading chunk 33 failed
dashboard error, both since Site Kit 1.79.1.
Loading chunk 33 failed.
(error: http://domainname.com/wp-content/plugins/google-site-kit/dist/assets/js/33-a742fa632cd94e4fa105.js)
in Unknown
in LazyContentSVG
in Suspense
in ContentSVG
in div
in div
in div
in Cell
in div
in ForwardRef
in ForwardRef
in ContentAutoUpdate
in div
in ForwardRef
in section
in AdSenseConnectCTA
in div
in div
in Widget
in WithWidgetSlug(Widget)
in AdSenseConnectCTAWidget
in WidgetRenderer
in div
in Cell
in WidgetCellWrapper
in div
in ForwardRef
in div
in div
in ForwardRef
in WidgetAreaRenderer
in div
in WidgetContextRenderer
in DashboardMainApp
in DashboardEntryPoint
in RestoreSnapshots
in ErrorHandler
in StrictMode
in Root
Additional Context
- SK 1.79.1
- No SH info so far
- One site does seem to have mismatching URLs, evident from checking their public REST endpoints
- Awaiting further details
Impacted sites
- https://wordpress.org/support/topic/loading-chunk-33-failed/ | Open | No SH info
- https://wordpress.org/support/topic/site-kit-encountered-an-error-74/ | Open | No SH info
- https://wordpress.org/support/topic/site-kit-encountered-an-error-75/ | Open | SH info
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- Lazy-loaded graphics should be loaded in a common way that prevents a failed import from crashing the dashboard
- In this case, a suitable light-weight non-lazy-loaded fallback should be rendered in its place. For now this can simply be an error message styled similar to use in other places:
Error: Failed to load graphic.
- This should be an edge case so we don’t need to make it a polished experience.
Implementation Brief
- Create
assets/js/components/MediaErrorHandler.js
which exports the class basedMediaErrorHandler
component.- The component should have the same logic as
assets/js/components/ErrorHandler/index.js
with the following changes:- It should accept a required string prop
errorMessage
. - The
copied
state variable and associated functionality should be removed, leaving theerror
andinfo
state variables. - There should only be the
componentDidCatch
andrender
functions. - The
render
function should renderchildren
if there’s noerror
anderrorMessage
in case there’s one. - The
errorMessage
should be wrapped within ap
tag and should be styled using the same color as for error messages. (#7a1e00)
- It should accept a required string prop
- The component should have the same logic as
- Using
assets/js/modules/adsense/components/common/AdSenseConnectCTA/ContentSVG.js
,- Wrap the existing components within
MediaErrorHandler
witherrorMessage
prop set as per the AC.
- Wrap the existing components within
- Add stories for
MediaErrorHandler
withinassets/js/components/MediaErrorHandler.stories.js
- Note: To reproduce/simulate error, delete
0.js
or1.js
indist/assets/js/
Test Coverage
- Add tests for the
MediaErrorHandler
component.
QA Brief
- Ensure that you are using the development build of Site Kit.
- Ensure that Adsense is not set up and the Adsense Connect CTA widget is visible.
- Enter the file manager and go to the
wp-content/plugins/google-site-kit/dist/assets/js
folder. - Rename the
0.js
,1.js
, or2.js
file, or all of them to something like*.old.js
so that the application cannot find them. - Go to the Site Kit Dashboard.
- Verify that the dashboard doesn’t crash.
- Verify that the Adsense Connect CTA widget shows an error according to the ACs for the missing graphics.
- Verify the Components > MediaErrorHandler Storybook story.
Follow-up Update
- Follow the above first three steps.
- Rename the
3.js
file to something like3.old.js
so the application cannot find it. - Ensure the IdeaHub CTA widget shows an error according to the AC for the missing graphics. Refer the screenshot below:
Changelog entry
- Prevent dashboard from crashing when async SVG fails to load.
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 17
Commits related to this issue
- Merge pull request #6120 from google/fix/#5605-svg-loading-failed Add `MediaErrorHandler` — committed to google/site-kit-wp by techanvil 2 years ago
@aaemnnosttv @nfmohit I have created a follow-up PR that wraps the
LazyIdeaHubPromptSVG
with theMediaErrorHandler
component.Thanks, @asvinb. All looks good now, except we don’t need tracking for the new error handler. I have removed it from IB.
IB ✔️.
@asvinb, yes, I understand your intention, but I think we should still use a separate component and not change the
ErrorHandler
component to avoid making even more changes there in the future if we decide to add a fallback image or anything else to the error boundary for svg images.@aaemnnosttv Tagging you in here as we discussed this in today’s support/eng sync. Let’s see if we can make this a clearer message for users.