site-kit-wp: Retry all selectors for combined widgets in ReportError state.

Thanks for explaining this, @techanvil. Yes, I think we can go with what you suggested. @benbowler, could you please update your IB?

Thanks @benbowler, the IB LGTM ✅

@eugene-manuilov, in practical terms I think it would just mean passing an array of errors into WidgetReportError as ReportErrorActions looks up the selector name and args from the error object(s)…

Actually, looking into this has made me realise we should reconsider this issue a bit.

First off, I should point out that when I initially created the issue, I made a mistake in how I described it. It’s not the case that “the widgets in a widget area are combined due to all being in the ReportError state” - that suggests our widget combining logic is in play, when it’s not - ReportError is not one of the SPECIAL_WIDGET_STATES that allows it to be combined. Plus - the All Traffic widget is simply that - one widget, not a widget area. Looks like I managed to confuse myself while raising this one! So, sorry for the confusion.

Anyhow, rather than combining anything, we are simply rendering a WidgetReportError at the top level of each of the widgets mentioned in the AC when we see the first error that we want to handle.

https://github.com/google/site-kit-wp/blob/ab9d76bebe8a3368eaf73b194edf941fd42f2ec5/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidgetGA4/index.js#L301-L310

https://github.com/google/site-kit-wp/blob/ab9d76bebe8a3368eaf73b194edf941fd42f2ec5/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/index.js#L309-L318

So, with the widget errors clearly not being in a combined state, if we take the approach of retrying all of the widget’s report errors while only showing one of the errors, we can end up not showing some of the errors to the user, which is not really the intention here.

Note that ReportError is already designed to handle multiple errors, deduplicating them when duplicates are present:

https://github.com/google/site-kit-wp/blob/ab9d76bebe8a3368eaf73b194edf941fd42f2ec5/assets/js/components/ReportError.js#L92-L101

Its child ReportErrorActions will retry multiple errors:

https://github.com/google/site-kit-wp/blob/ab9d76bebe8a3368eaf73b194edf941fd42f2ec5/assets/js/components/ReportErrorActions.js#L103-L113

With that in mind, I think the thing to do here for the All Traffic widget is to pass all of the errors into WidgetReportError at that top level render. This will allow ReportError to show and retry multiple errors as currently designed, deduping as necessary.

diff --git a/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidgetGA4/index.js b/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidgetGA4/index.js
index 7e20a3d3c9..8e7338cb6f 100644
--- a/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidgetGA4/index.js
+++ b/assets/js/modules/analytics/components/dashboard/DashboardAllTrafficWidgetGA4/index.js
@@ -298,12 +298,18 @@ function DashboardAllTrafficWidgetGA4( props ) {
 		setValue,
 	] );
 
+	const retryableErrors = [
+		pieChartError,
+		totalUsersError,
+		userCountGraphError,
+	].filter( Boolean );
+
 	if ( pieChartError ) {
 		return (
 			<Widget>
 				<WidgetReportError
 					moduleSlug="analytics-4"
-					error={ pieChartError }
+					error={ retryableErrors }
 				/>
 			</Widget>
 		);

As for the Search Funnel widget, I don’t think we should do anything here at this point, because the top level error is for the Search Console module, while the error shown in the Overview is for the Analytics module, and we don’t currently have a design or pattern for showing errors for multiple modules at the same time. Maybe this could be something to address as a separate issue.

https://github.com/google/site-kit-wp/blob/ab9d76bebe8a3368eaf73b194edf941fd42f2ec5/assets/js/modules/search-console/components/dashboard/SearchFunnelWidgetGA4/Overview/OptionalCells.js#L85-L92

What do you think?

Hi @benbowler, just chipping in to say I think it would be preferable to take the approach of passing in options or a set of selector names and args to ReportErrorActions via WidgetReportError / ReportError, rather than the per-widget selector approach, as this would be more generic and reusable. We could then reuse this for the combined error states for the WP Dashboard / Admin Bar that are being discussed on https://github.com/google/site-kit-wp/issues/6377.