Detect errors in data layer and handle them when routing #104920

p-jackson · 2025-07-25T05:04:14Z

Proposed Changes

More generally, this PR suggests an error handling strategy.

Detect errors in the data layer
Handle those errors in the UI, it could be
- router
- suspense boundary
- error boundary

It seems like React has moved in the direction of propagating errors through the component tree using exceptions. So perhaps we just embrace that.

More specifically, this detects and show a specific error UI when we detect the case of a Jurassic Ninja site being missing

Why make this change?

I think of the "500" error page as a last resort. It gives the user a way to recover (by navigating to /sites) but we should actually handle expected error cases with something else.

Most errors probably won't propagate all the way up to such a high level in the router. But this same exception throwing strategy should work for more localised <ErrorBoundary> components too.

Testing Instructions

Hopefully you have an old Jurassic Ninja site that has been cleaned up.
Navigate to /v2/sites/old.jurassic.ninja
You should not see the "500" error page.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
- For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2025-07-25T05:04:26Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-151221

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-151221&env=jetpack

Automattic for Agencies live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-151221&env=a8c-for-agencies

p-jackson · 2025-07-25T05:05:59Z

client/dashboard/data/error.ts

@@ -0,0 +1,13 @@
+export class DashboardDataError extends Error {


By making our own type we can do instanceof DashboardDataError

matticbot · 2025-07-25T05:06:35Z

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

blaze-dashboard
command-palette-wp-admin
help-center
notifications
odyssey-stats

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/error-message-when-jurassic-ninja-site-gone on your sandbox.

p-jackson · 2025-07-25T05:08:39Z

client/dashboard/data/error.ts

@@ -0,0 +1,13 @@
+export class DashboardDataError extends Error {
+	constructor(
+		public code: string,


Leaving the code as string seems ok to me. Right? We don't have a need to do exhaustive matching—we don't need to be sure the component at sites/site/error.tsx handles every since code—so I think there isn't much use in having a giant type definition that includes every single error case that every single API can return.

Throw error with generic string, catch error with generic string. Seems greppable :)

p-jackson · 2025-07-25T05:09:20Z

client/dashboard/data/error.ts

+		// Fix prototype chain (important for instanceof to work reliably)
+		Object.setPrototypeOf( this, new.target.prototype );


AI suggested this. Apparently in some transpiling situations can break the prototype chain.

p-jackson · 2025-07-25T05:10:44Z

client/dashboard/data/site.ts

+	} catch ( error ) {
+		if ( error instanceof Error && error.message.includes( '[-32710]' ) ) {
+			throw new DashboardDataError( 'inaccessible_jetpack', error );
+		}
+		throw error;


My thinking here is that the error handling is fine tuned to what v2 needs. Hence why it is a dashboard data error. We should only feel we need to detect the error situations we care about.

p-jackson · 2025-07-25T05:11:52Z

client/dashboard/sites/site/error.tsx

+	let site = queryClient.getQueryData< Site >( siteBySlugQuery( siteSlug ).queryKey );
+	if ( ! site ) {
+		const allSites = queryClient.getQueryData< Site[] >( sitesQuery().queryKey );
+		site = allSites?.find( ( s ) => s.slug === siteSlug );
+	}


Being careful to only try showing the display name if it happens to already be available. We don't want to cause more errors in the error handler!

I feel this is a bit unnecessary. If the site is down, I don't think it's that helpful to show the site name; the site URL makes more sense to highlight.

Yes perhaps you're right. I'm imagining that there might be other errors we could catch that are site related, that wouldn't be as catastrophic as this one. And in that case it'd be useful to salvage whatever Site data we have.

But we should cross that bridge when we come to it. In this case where the JP site has just disappeared it is a bit more catastrophic.

Removed in 6135394

fushar

Tested; the logic looks sound, thanks!

fushar · 2025-07-28T02:26:50Z

client/dashboard/data/site.ts

+			{ fields: JOINED_SITE_FIELDS, options: JOINED_SITE_OPTIONS }
+		);
+	} catch ( error ) {
+		if ( error instanceof Error && error.message.includes( '[-32710]' ) ) {


Would it be more robust to check for error.error === 'parse_error' instead of checking for substring?

Ohh I guess because there's more kind of errors that have the same parse_error category

Hmmm, actually I think -32710 does just mean the same thing as parse_error.

fbhepr%2Skers%2Sjcpbz%2Sjc%2Qpbagrag%2Szh%2Qcyhtvaf%2Swrgcnpx%2Spyvrag%2Spynff.ncv%2Qreebe.cuc%3Se%3Q5o221o91%23128-og

So I'll remove the sub-string match.

It'd be nice to have a more specific error than just "parse error" tbh. What's happening here is that we're trying to call the JP remove site, the API doesn't return, and rather than interpret that as an error that the JP site wasn't available, the error that gets propagated is that the response body was empty or something. So not ideal.

Done in 853f435

I added some ergonomics for parsing the error object, since we can't be sure what the type is.

wpcom-xhr-request throws a type called WPError—the JS equivalent of the PHP class.

wp-calypso/packages/wpcom-xhr-request/src/index.js

Line 90 in 6135394

const wpe = WPError( { path, method }, statusCode, body );

I made a isWpError function to give us the same ergonomics as the PHP is_wp_error function.

fushar

a8ci18n · 2025-07-28T08:22:35Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/19608909

Some locales (Hebrew) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @p-jackson for including a screenshot in the description! This is really helpful for our translators.

Detect errors in data layer and handle them when routing

01e9316

p-jackson requested a review from a team as a code owner July 25, 2025 05:04

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 25, 2025

p-jackson self-assigned this Jul 25, 2025

p-jackson commented Jul 25, 2025

View reviewed changes

fushar approved these changes Jul 28, 2025

View reviewed changes

p-jackson added 2 commits July 28, 2025 15:40

Check for parse_error, which means the same thing as the error code

853f435

Remove attempt to show disconnected site's display name

6135394

fushar approved these changes Jul 28, 2025

View reviewed changes

p-jackson merged commit eae8f27 into trunk Jul 28, 2025
11 checks passed

p-jackson deleted the update/error-message-when-jurassic-ninja-site-gone branch July 28, 2025 05:41

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 28, 2025

gabrielcaires pushed a commit that referenced this pull request Aug 1, 2025

Detect errors in data layer and handle them when routing (#104920)

ac6318b

		@@ -0,0 +1,13 @@
		export class DashboardDataError extends Error {

		// Fix prototype chain (important for instanceof to work reliably)
		Object.setPrototypeOf( this, new.target.prototype );

Detect errors in data layer and handle them when routing #104920

Detect errors in data layer and handle them when routing #104920

Uh oh!

Conversation

p-jackson commented Jul 25, 2025

Proposed Changes

Why make this change?

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions bot commented Jul 25, 2025 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matticbot commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fushar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fushar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

a8ci18n commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 25, 2025 •

edited by matticbot

Loading

matticbot commented Jul 25, 2025 •

edited

Loading

a8ci18n commented Jul 28, 2025 •

edited

Loading