Surface query errors as notices #421

chriszarate · 2025-03-13T21:18:30Z

Screen.Recording.2025-03-13.at.16.50.45.mov

The error duplication should be separately fixed by #416

github-actions · 2025-03-13T21:57:38Z

alecgeatches

This is great and sorely needed! I noticed that error responses in DataViews can obscure the error until the view is exited:

Screen.Recording.2025-03-14.at.10.57.03.AM.mov

Note: this request URL was modified to make it fail purposefully.

It'd be great if we had a way to also display errors in there, but either way this is an improvement as-is. Thank you!

chriszarate · 2025-03-14T17:15:49Z

The error is returned by the hook, so it should be possible, but I don't see any documented error handling in DataViews. @brookewp Anything come to mind?

brookewp · 2025-03-15T04:01:58Z

Ya, there isn't anything built into DataViews for this. How about adding a Notice as a child of our DataViewsModal component?

Screen.Recording.2025-03-14.at.8.53.42.PM.mov

jarekmorawski · 2025-03-18T09:11:10Z

These are all great ideas. Thanks for looking into them! The notice works well because it's very prominent, but what if the user is currently editing five different blocks? The error doesn't specify which block is affected, and while we could make it smart and actionable, I think we'll be better off simplifying the UI and displaying the error in the block placeholder.

The error state is shown in the block,
The error title will be the same for all connection errors (= "Cannot connect") unless there are any other error types that may require customized copy and CTA,
When the block is not focused, the block has a distinctive placeholder with an error icon,
When the user clicks the Show error details button, the placeholder expands and reveals a code block with the API response. The user can select its content and copy it like any other text string,
Clicking "Try again" will attempt to reconnect to the source,
The sidebar is unaffected and still displays the block's details without the settings and customization options.

Could this work?

chriszarate · 2025-03-18T21:01:38Z

Could this work?

Thanks so much for looking @jarekmorawski! I'll give it a shot.

brookewp · 2025-03-20T17:16:12Z

@chriszarate I've created a new PlaceholderError component based on @jarekmorawski's design above. I can directly commit the changes if preferred, but I figured I'd share a diff in case you want to take a look first.

I'm curious about your thoughts on implementing this. I would have thought to add it to edit.tsx, but the hook there is just looking at the display query key so it wouldn't catch all errors.

Here is a demo of what the component looks like:

Screen.Recording.2025-03-20.at.10.10.50.AM.mov

diff --git a/src/blocks/remote-data-container/components/placeholders/PlaceholderError.tsx b/src/blocks/remote-data-container/components/placeholders/PlaceholderError.tsx
new file mode 100644
index 00000000..f3209a6b
--- /dev/null
+++ b/src/blocks/remote-data-container/components/placeholders/PlaceholderError.tsx
@@ -0,0 +1,48 @@
+import { Button, Placeholder, __experimentalHStack as HStack, Icon } from '@wordpress/components';
+import { useState } from '@wordpress/element';
+import { __, sprintf } from '@wordpress/i18n';
+import { caution } from '@wordpress/icons';
+
+import { RemoteDataFetchError } from '../../hooks/useRemoteData';
+
+interface PlaceholderErrorProps {
+	blockName: string;
+	error: Error;
+	onTryAgain: () => void;
+}
+
+export function PlaceholderError( { blockName, error, onTryAgain }: PlaceholderErrorProps ) {
+	const [ showErrorDetails, setShowErrorDetails ] = useState( false );
+
+	let message = error.message;
+	if ( error instanceof RemoteDataFetchError && error.cause instanceof Error ) {
+		message = `${ message }: ${ error.cause.message }`;
+	}
+
+	return (
+		<Placeholder
+			className="remote-data-container-error"
+			icon={ caution }
+			label={ sprintf( __( 'Cannot connect to remote data source: (%s)' ), blockName ) }
+		>
+			<Icon className="remote-data-container-error__icon-overlay" icon={ caution } />
+			<HStack justify="flex-start">
+				<Button variant="secondary" onClick={ onTryAgain }>
+					{ __( 'Try again' ) }
+				</Button>
+				<Button variant="tertiary" onClick={ () => setShowErrorDetails( ! showErrorDetails ) }>
+					{ sprintf( __( '%s error details' ), showErrorDetails ? 'Hide' : 'Show' ) }
+				</Button>
+			</HStack>
+			{ showErrorDetails && (
+				<code
+					className={ `remote-data-container-error__content is-${
+						showErrorDetails ? 'open' : 'closed'
+					}` }
+				>
+					{ message }
+				</code>
+			) }
+		</Placeholder>
+	);
+}
diff --git a/src/blocks/remote-data-container/editor.scss b/src/blocks/remote-data-container/editor.scss
index 238fc2d4..90185279 100644
--- a/src/blocks/remote-data-container/editor.scss
+++ b/src/blocks/remote-data-container/editor.scss
@@ -4,6 +4,69 @@
 
 @import url(@wordpress/dataviews/build-style/style.css);
 
+// selected error placeholder
+.rdb-container.is-selected .components-placeholder.remote-data-container-error {
+
+	.components-placeholder__label > svg {
+		fill: var(--Gutenberg-Alert-Red, #cc1818);
+	}
+
+	.remote-data-container-error__content {
+		width: auto;
+		background: var(--Gutenberg-Gray-100, #f0f0f0);
+		color: var(--Gutenberg-Gray-900, #1e1e1e);
+		line-height: 1.5;
+		padding: 12px 16px;
+		border-radius: 2px;
+	}
+
+	// hide overlay icon
+	.remote-data-container-error__icon-overlay {
+		display: none;
+	}
+}
+
+// unselected error placeholder
+.rdb-container:not(.is-selected) .components-placeholder.remote-data-container-error {
+	background: var(--Gutenberg-Gray-100, #f0f0f0);
+	box-shadow: inset 0 0 1px 1px var(--Gutenberg-Alert-Red, #cc1818);
+
+	// hide content when block is not selected
+	*:not(.remote-data-container-error__icon-overlay):not(.remote-data-container-error__icon-overlay *) {
+		visibility: hidden;
+	}
+
+	// hide open error details when block is not selected for consistent sizing
+	.remote-data-container-error__content.is-open {
+		display: none;
+	}
+
+	// add strikethrough across placeholder
+	&::before {
+		content: "";
+		position: absolute;
+		top: 0;
+		left: 0;
+		width: 100%;
+		height: 100%;
+		background: var(--Gutenberg-Gray-200, #e0e0e0);
+		clip-path: polygon(1% 1%, 2% 1%, 99% 99%, 98% 99%);
+	}
+
+	// show overlay icon when block is not selected
+	.remote-data-container-error__icon-overlay {
+		visibility: visible;
+		width: 48px;
+		height: 48px;
+		background: var(--Gutenberg-Gray-100, #f0f0f0);
+		fill: var(--Gutenberg-Gray-200, #e0e0e0);
+		position: absolute;
+		top: 50%;
+		left: 50%;
+		transform: translate(-50%, -50%);
+	}
+}
+
 .remote-data-blocks-loading-overlay {
 	align-items: center;
 	background-color: rgba(255, 255, 255, 0.25);

chriszarate · 2025-03-20T18:09:49Z

@brookewp Push it up with thanks! I think edit.tsx can destructure error and pass it down

Signed-off-by: brookewp <[email protected]>

brookewp · 2025-03-20T18:46:33Z

Push it up with thanks!

Done! 23c9de0 (#421)

I think edit.tsx can destructure error and pass it down

When destructuring error here, I was seeing error was undefined (I broke the search query endpoint to trigger an error):

remote-data-blocks/src/blocks/remote-data-container/edit.tsx

Lines 42 to 47 in 500df71

    
           const { data, fetch, loading, reset } = useRemoteData( { 
        
           	blockName, 
        
           	externallyManagedRemoteData: remoteDataAttribute, 
        
           	externallyManagedUpdateRemoteData: updateRemoteData, 
        
           	queryKey: DISPLAY_QUERY_KEY, 
        
           } );

chriszarate · 2025-03-20T18:50:03Z

error is simple React state, can you trace why setError is not being called? Or can you push up your current state in a branch off this one?

brookewp · 2025-03-20T19:06:04Z

error is simple React state, can you trace why setError is not being called? Or can you push up your current state in a branch off this one?

I have a test branch here: test/improve/block-editor-query-notices

I'm looking into it, but I'm sure you'll be quicker than me.

brookewp · 2025-03-20T19:39:37Z

Btw, if it's helpful, setError is being called, but only the display query is passed to useRemoteData in edit.tsx. And the error I was testing was for the search query. Since the hook is called in the DataViewsModal, we see the console error.

Edit: I had added comments in my test branch to demonstrate this, but not sure if they stand out so wanted to mention:

remote-data-blocks/src/blocks/remote-data-container/edit.tsx

Lines 47 to 50 in 7486060

    
           // error is passed along with this query key 
        
           // queryKey: 'RemoteDataBlocks\\Config\\Query\\HttpQuery', 
        
           // error is undefined with this query key 
        
           queryKey: DISPLAY_QUERY_KEY,

So I'm not sure if we'd want modify edit to look at the other keys, or adjust how the error works in useRemoteData, or maybe use context... 🤔

chriszarate · 2025-03-20T19:54:15Z

Ah, I see. So we have the "render" usage of useRemoteData at the top level, then another use of useRemoteData in DataViewsModal further down the component tree. We need shared state between them to capture errors for all uses of useRemoteData.

We could use context. What about throwing and using an error boundary? Are we able to recover / try again?

brookewp · 2025-03-20T20:01:44Z

We could use context. What about throwing and using an error boundary? Are we able to recover / try again?

I can give that a try and report back

chriszarate added 3 commits March 13, 2025 17:13

Surface query errors as notices

d3562e8

Prettier!

26565fa

Cursor is killing me, I thought this was supposed to help

7eabd8f

chriszarate added the enhancement New feature or request label Mar 13, 2025

alecgeatches approved these changes Mar 14, 2025

View reviewed changes

Add new PlaceholderError component

23c9de0

Signed-off-by: brookewp <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Surface query errors as notices #421

Surface query errors as notices #421

chriszarate commented Mar 13, 2025

github-actions bot commented Mar 13, 2025

alecgeatches left a comment •

edited

Loading

chriszarate commented Mar 14, 2025

brookewp commented Mar 15, 2025

jarekmorawski commented Mar 18, 2025

chriszarate commented Mar 18, 2025

brookewp commented Mar 20, 2025

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

brookewp commented Mar 20, 2025 •

edited

Loading

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

Surface query errors as notices #421

Are you sure you want to change the base?

Surface query errors as notices #421

Conversation

chriszarate commented Mar 13, 2025

github-actions bot commented Mar 13, 2025

alecgeatches left a comment • edited Loading

Choose a reason for hiding this comment

chriszarate commented Mar 14, 2025

brookewp commented Mar 15, 2025

jarekmorawski commented Mar 18, 2025

chriszarate commented Mar 18, 2025

brookewp commented Mar 20, 2025

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

brookewp commented Mar 20, 2025 • edited Loading

chriszarate commented Mar 20, 2025

brookewp commented Mar 20, 2025

alecgeatches left a comment •

edited

Loading

brookewp commented Mar 20, 2025 •

edited

Loading