Harden pre-publish projection and client-metadata error handling#149
Draft
pfefferle wants to merge 6 commits into
Draft
Harden pre-publish projection and client-metadata error handling#149pfefferle wants to merge 6 commits into
pfefferle wants to merge 6 commits into
Conversation
- Pre-publish controller: catch any Throwable from project() (it runs the_content over raw editor markup), log the post ID, and return a WP_Error 500 instead of fatalling the keystroke-driven endpoint. - Pre-publish panel: capture and log the fetch error, and show a distinct message for permission failures vs transient ones rather than collapsing every failure into one generic line. - Client-metadata controller: also route an invalid-filter return through debug_log(), so operators get a signal without WP_DEBUG (the existing _doing_it_wrong() is silent in production).
github-actions
Bot
added
[Status] In Progress
[Tests] Includes Tests
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens error handling for the editor pre-publish preview (server projection + panel UI) and improves operational visibility when the OAuth client-metadata filter returns invalid data.
Changes:
- Wrap pre-publish projection in
try/catch(\Throwable)to return a structuredWP_Error(and ensure the HTTP-blocking filter is always removed). - Improve the pre-publish panel’s fetch error handling by logging the underlying error and showing a distinct message for authorization failures.
- Log invalid
atmosphere_client_metadatafilter results viadebug_log()in addition to_doing_it_wrong(), and add a regression test for projection throws.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/rest/admin/class-test-pre-publish-controller.php | Adds coverage ensuring projection exceptions return a structured REST error instead of fatalling. |
| src/pre-publish-panel/plugin.js | Preserves the caught error, logs it, and displays more specific user-facing error messages. |
| includes/rest/class-client-metadata-controller.php | Adds debug_log() signal when filtered client-metadata is invalid in production. |
| includes/rest/admin/class-pre-publish-controller.php | Catches projection exceptions, logs context, returns WP_Error, and preserves finally cleanup. |
| build/pre-publish-panel/plugin.js | Updates built panel bundle to reflect source changes. |
| build/pre-publish-panel/plugin.asset.php | Updates build asset version hash. |
| .github/changelog/fix-pre-publish-error-handling | Adds patch changelog entry describing improved preview error messaging. |
Files not reviewed (1)
- build/pre-publish-panel/plugin.js: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+101
to
+106
| const errorStatus = error?.data?.status; | ||
| const isAuth = | ||
| 401 === errorStatus || | ||
| 403 === errorStatus || | ||
| 'rest_forbidden' === error?.code; | ||
|
|
A 403 with code rest_cookie_invalid_nonce is transient, so key the auth message off the REST error code and exclude the nonce case; it now shows the retriable message instead of "you don't have permission".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes:
Follow-up to #148, addressing @kraftbj/@jeherve review notes that are about the pre-publish panel / client-metadata rather than the reactions block, kept out of that PR to avoid scope creep.
get_preview):Post::project()runsthe_contentover raw, unsaved editor markup, so a malformed block or a misbehaving content/shortcode filter could throw and fatal the keystroke-driven endpoint into an opaque 500. It's now wrapped intry/catch(\Throwable)/finally— on a throw it logs the post ID viadebug_log()and returns a structuredWP_Error(500); thefinallystill removes the HTTP-blocking filter on every path.plugin.js): the fetch.catchpreviously collapsed every failure into one generic message and threw the error away. It now logs the error and shows a distinct message for permission failures (401/403 orrest_forbidden) vs transient ones.atmosphere_client_metadatafilter return was only surfaced via_doing_it_wrong(), which is silent in production. It now also routes throughdebug_log(), so operators can opt into the signal via theatmosphere_debug_logfilter without enablingWP_DEBUGsite-wide.Other information:
Testing instructions:
composer lint && npm run lint:js && npm run buildnpm run env-test— 720 tests pass, incl. the newtest_preview_returns_error_when_projection_throws(a throwingthe_contentfilter →WP_Error500, not a fatal).Manual:
Changelog entry
Entry included at
.github/changelog/fix-pre-publish-error-handling(Patch / Fixed).