-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add site editor initial redirect error handling #38655
Changes from all commits
112e4e2
ef9a6b3
b299d1c
22f17ae
ebcf6a4
48b8d81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { Button } from '@wordpress/components'; | ||
import { Warning } from '@wordpress/block-editor'; | ||
import { useCopyToClipboard } from '@wordpress/compose'; | ||
|
||
function CopyButton( { text, children } ) { | ||
const ref = useCopyToClipboard( text ); | ||
return ( | ||
<Button variant="secondary" ref={ ref }> | ||
{ children } | ||
</Button> | ||
); | ||
} | ||
|
||
export default function ErrorBoundaryWarning( { | ||
message, | ||
error, | ||
reboot, | ||
dashboardLink, | ||
} ) { | ||
const actions = []; | ||
|
||
if ( reboot ) { | ||
actions.push( | ||
<Button key="recovery" onClick={ reboot } variant="secondary"> | ||
{ __( 'Attempt Recovery' ) } | ||
</Button> | ||
); | ||
} | ||
|
||
if ( error ) { | ||
actions.push( | ||
<CopyButton key="copy-error" text={ error.stack }> | ||
{ __( 'Copy Error' ) } | ||
</CopyButton> | ||
); | ||
} | ||
talldan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if ( dashboardLink ) { | ||
actions.push( | ||
<Button | ||
key="back-to-dashboard" | ||
variant="secondary" | ||
href={ dashboardLink } | ||
> | ||
{ __( 'Back to dashboard' ) } | ||
</Button> | ||
); | ||
} | ||
|
||
return ( | ||
<Warning className="editor-error-boundary" actions={ actions }> | ||
{ message } | ||
</Warning> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,10 +20,17 @@ function getNeedsHomepageRedirect( params ) { | |
); | ||
} | ||
|
||
/** | ||
* Returns the postType and postId of the default homepage. | ||
* | ||
* @param {string} siteUrl The URL of the site. | ||
* @return {Object} An object containing the postType and postId properties | ||
* or `undefined` if a homepage could not be found. | ||
*/ | ||
async function getHomepageParams( siteUrl ) { | ||
const siteSettings = await apiFetch( { path: '/wp/v2/settings' } ); | ||
if ( ! siteSettings ) { | ||
return; | ||
throw new Error( '`getHomepageParams`: unable to load site settings.' ); | ||
} | ||
|
||
const { | ||
|
@@ -44,11 +51,27 @@ async function getHomepageParams( siteUrl ) { | |
// (packages/core-data/src/resolvers.js) | ||
const template = await window | ||
.fetch( addQueryArgs( siteUrl, { '_wp-find-template': true } ) ) | ||
.then( ( res ) => res.json() ) | ||
.then( ( { data } ) => data ); | ||
.then( ( response ) => { | ||
if ( ! response.ok ) { | ||
throw new Error( | ||
`\`getHomepageParams\`: HTTP status error, ${ response.status } ${ response.statusText }` | ||
); | ||
} | ||
|
||
return response.json(); | ||
} ) | ||
.then( ( { data } ) => { | ||
if ( data.message ) { | ||
throw new Error( | ||
`\`getHomepageParams\`: REST API error, ${ data.message }` | ||
); | ||
} | ||
|
||
return data; | ||
} ); | ||
|
||
if ( ! template?.id ) { | ||
return; | ||
throw new Error( '`getHomepageParams`: unable to find home template.' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Maybe we should make this message more user-friendly and display it instead of a generic error message?
What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, sounds good 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I've kept these error messages the same so that we can locate them in the code when the user uses 'Copy Error', but the visible message is now what you suggested. |
||
} | ||
|
||
return { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to add a fallback here. When I add an empty
front-page.php
to Twenty Twenty-two and open the Site Editor I see an error ("The editor is unable to find a block template for the homepage.") but pressing Copy Error does nothing.In React DevTools I can see that there is no
text
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So somewhere in the code isn't passing a proper error object? I can do some debugging, but if you have any clues let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No clues, sorry. It was pretty easy to reproduce, though. Add an empty
front-page.php
file to the top level oftwentytwentytwo
.