Site Editor: Throws fatal error when enabled on an atomic site

jeyip commented 3 years ago

Description

After provisioning an atomic site that is full site editing (FSE) enabled, the site and post editor stop loading upon revisiting either page.

Steps to reproduce the behavior

Create FSE enabled site https://horizon.wordpress.com/new?flags=gutenboarding/site-editor
Upgrade site to atomic (upgrade plan + install plugin)
Wait a few moments for site to provision
Visit the site editor or page editor
Return to the admin dashboard
Sometimes the editors load properly for a short period of time, but after a few more minutes, revisiting the page or site editor consistently throws a critical error

What I expected to happen

The site editor and page editor load properly for newly created atomic sites

What actually happened

The site throws a critical error

PHP Error Logs

PHP Fatal error:  Uncaught Error: Call to a member function get_raw_data() on array in /wordpress/plugins/gutenberg/10.2.0/lib/class-wp-theme-json.php:1097

Stack trace:
#0 /wordpress/plugins/gutenberg/10.2.0/lib/class-wp-theme-json-resolver.php(431): WP_Theme_JSON->merge(Array)
#1 /wordpress/plugins/gutenberg/10.2.0/lib/global-styles.php(206): WP_Theme_JSON_Resolver::get_merged_data(Array, 'user')
#2 /wordpress/plugins/gutenberg/10.2.0/lib/full-site-editing/edit-site-page.php(110): gutenberg_experimental_global_styles_settings(Array)
#3 /wordpress/core/5.7/wp-includes/class-wp-hook.php(292): gutenberg_edit_site_init('toplevel_page_g...')
#4 /wordpress/core/5.7/wp-includes/class-wp-hook.php(316): WP_Hook->apply_filters(NULL, Array)
#5 /wordpress/core/5.7/wp-includes/plugin.php(484): WP_Hook->do_action(Array)
#6 /wordpress/core/5.7/wp-admin/admin-header.php(102): do_action('admin_enqueue_s...', 'toplevel_page_g...')
#7 /wordpress/core/5.7/wp-admin/admin.php(239): require_once('/wordpress/core...')
#8 {main}
  thrown in /wordpress/plugins/gutenberg/10.2.0/lib/class-wp-theme-json.php on line 1097

Context

Browser / OS version

Chrome Latest / MacOS Big Sur 11.0.1

Is this specific to the applied theme? Which one?

This only occurs when block themes are enabled (e.g. tt1-blocks and seedlet-blocks)

Does this happen on simple or atomic sites or both?

Isolated to atomic sites

jeyip commented 3 years ago

Apparently the global styles feature also relies on a custom post type that uses theme stylesheet pathing (similar to how FSE template / parts do).

Stack Trace

The PHP error logs point us here where get_user_data() is invoked ( Link )
get_user_data() is defined here in the file ( Link )
get_user_data() calls get_user_data_from_custom_post_type() ( Link )
get_user_data_from_custom_post_type() finds and returns a CPT related to Global Styles ( Link )
get_user_data_from_custom_post_type() attempts to query for an existing CPT by name. ( Link )
The queried name includes the current theme's stylesheet. See the call to wp_get_theme()->get_stylesheet() for $post_name_filter
Before an atomic migration, the stylesheet for block-themes is prefixed by pub (Ex. pub/tt1-blocks). This is because themes are nested in a pub subdirectory. After an atomic migration, however, the theme's stylesheet changes. Block themes are relocated outside of the pub subdirectory, and the stylesheet is no longer prefixed by pub (Ex. tt1-blocks)
The query does not return a CPT
Since a CPT is not returned, json decoding fails. A syntax error is thrown in get_user_data() ( Link )
Because of the syntax error, get_user_data() returns an array ( Link )
The original call to get_raw_data() is passed an array instead of a WP_Theme_Json object ( Link )

oandregal commented 3 years ago

https://github.com/WordPress/gutenberg/pull/30088 will help fixing this as well.

oandregal commented 3 years ago

https://github.com/WordPress/gutenberg/pull/30088 landed as part of Gutenberg 10.3 and should prevent the site editor from crashing. A different issue, though, is that when users are moved to atomic their user styles aren't going to be pulled because of:

Before an atomic migration, the stylesheet for block-themes is prefixed by pub (Ex. pub/tt1-blocks). This is because themes are nested in a pub subdirectory. After an atomic migration, however, the theme's stylesheet changes. Block themes are relocated outside of the pub subdirectory, and the stylesheet is no longer prefixed by pub (Ex. tt1-blocks)

As you pointed out, the user data is stored as a CPT (post_type = global_styles and post_name = wp-global-styles-<theme-stylesheet> see). I wonder how do you do it for user templates, I presume it has the same issue (IIRC, the idea of creating a CPT for user data in global styles was inspired by the templates, but I haven't kept up to how they work).

vindl commented 3 years ago

As you pointed out, the user data is stored as a CPT (post_type = global_styles and post_name = wp-global-styles- see). I wonder how do you do it for user templates, I presume it has the same issue (IIRC, the idea of creating a CPT for user data in global styles was inspired by the templates, but I haven't kept up to how they work).

For templates and template parts we are saving the theme info in the wp_theme taxonomy. Would it make sense to align the approaches?

We are going to update these values during atomic transfers/reversals (https://github.com/Automattic/wp-calypso/issues/50659) so it would be much easier if we could query all of the CPTs by taxonomy.

oandregal commented 3 years ago

Thanks for the pointer, Marko. I saw you created https://github.com/WordPress/gutenberg/issues/30191 and I added it to the task list for 5.8.

Automattic / wp-calypso