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


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 based MediaErrorHandler 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 the error and info state variables.
      • There should only be the componentDidCatch and render functions.
      • The render function should render children if there’s no error and errorMessage in case there’s one.
      • The errorMessage should be wrapped within a p tag and should be styled using the same color as for error messages. (#7a1e00)
  • Using assets/js/modules/adsense/components/common/AdSenseConnectCTA/ContentSVG.js,
    • Wrap the existing components within MediaErrorHandler with errorMessage prop set as per the AC.
  • Add stories for MediaErrorHandler within assets/js/components/MediaErrorHandler.stories.js
  • Note: To reproduce/simulate error, delete 0.js or 1.js in dist/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, or 2.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 like 3.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:

Screenshot 2022-11-15 at 11 49 16 AM

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

Most upvoted comments

@aaemnnosttv @nfmohit I have created a follow-up PR that wraps the LazyIdeaHubPromptSVG with the MediaErrorHandler component.

Screenshot 2022-11-15 at 11 49 16 AM

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.