943 seo data optimization error reporting refactoring#23402
Open
pls78 wants to merge 12 commits into
Open
Conversation
pls78
added
the
changelog: enhancement
Coverage Report for CI Build 908Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Warning No base build found for commit Coverage: 45.965%Details
Uncovered Changes
Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Added the getAlreadyIndexedStateName function because the Tools page and the FTC use a different string. All the rest is 100% untouched
Nothing changed, I just moved out the re-used code
… id and type to be consumed by the fronted
…rminal which indexable cannot be created
pls78
force-pushed
the
943-seo-data-optimization-error-reporting-refactoring
branch
from
600e5a2 to
3ecba73
Compare
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves SEO data optimization/indexing failure reporting by propagating the failing object’s identity (type + ID) from the PHP indexable build layer through to WP-CLI output and REST/React error UI, and refactors the JS indexation engine into a shared base component.
Changes:
- Introduces
Indexing_Failed_Exceptionto wrap unexpected build failures while carrying failing object metadata, and emits a newwpseo_indexable_indexing_failedaction. - Extends REST (
Indexing_Route) and WP-CLI (Index_Command) error handling to surface the failing object (type + ID) in responses/output. - Refactors JS indexation into
AbstractIndexationand centralizes error-details rendering inIndexingErrorContent, with updated unit/Jest coverage.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Routes/Indexing_Route_Test.php | Adds unit coverage for turning Indexing_Failed_Exception into a REST WP_Error. |
| tests/Unit/Exceptions/Indexable/Indexing_Failed_Exception_Test.php | Adds unit coverage for the new exception’s stored metadata and message. |
| tests/Unit/Commands/Index_Command_Test.php | Adds unit coverage for WP-CLI output including failing object details. |
| tests/Unit/Builders/Indexable_Builder/Save_Indexable_Test.php | Updates builder tests for the new constructor dependency (logger) and improves phpdoc typing. |
| tests/Unit/Builders/Indexable_Builder/Maybe_Build_Author_Indexable_Test.php | Updates builder tests for the new constructor dependency (logger). |
| tests/Unit/Builders/Indexable_Builder/Is_Type_With_No_Id_Test.php | Updates builder tests for the new constructor dependency (logger) and corrects provider docblock. |
| tests/Unit/Builders/Indexable_Builder/Ensure_Indexable_Test.php | Updates builder tests for the new constructor dependency (logger). |
| tests/Unit/Builders/Indexable_Builder/Build_Test.php | Adds coverage for logging + action firing + wrapped exception rethrow on unexpected build errors. |
| tests/Unit/Builders/Indexable_Builder/Abstract_Indexable_Builder_TestCase.php | Adds a logger mock to the shared builder test setup; improves phpdoc typing. |
| src/routes/indexing-route.php | Converts Indexing_Failed_Exception into WP_Error with stack trace and failing object metadata. |
| src/exceptions/indexable/indexing-failed-exception.php | Adds the new exception class to carry failing object identity and wrap the underlying throwable. |
| src/commands/index-command.php | Enhances WP-CLI indexing to halt with a clearer error including failing object info. |
| src/builders/indexable-builder.php | Logs unexpected build failures, fires wpseo_indexable_indexing_failed, and rethrows as Indexing_Failed_Exception. |
| packages/js/tests/indexation.test.js | Adds Jest coverage for showing the failing object in the error details UI. |
| packages/js/src/first-time-configuration/tailwind-components/steps/indexation/indexing-error.js | Reuses shared IndexingErrorContent for consistent error-details rendering. |
| packages/js/src/first-time-configuration/tailwind-components/steps/indexation/indexation.js | Refactors FTC indexation UI to extend shared AbstractIndexation. |
| packages/js/src/errors/RequestError.js | Extends RequestError to carry failing object metadata. |
| packages/js/src/components/IndexingErrorContent.js | Introduces shared error-details renderer, including a “Failing object” line. |
| packages/js/src/components/IndexingError.js | Refactors to use shared IndexingErrorContent. |
| packages/js/src/components/Indexation.js | Refactors Tools-page indexation UI to extend AbstractIndexation. |
| packages/js/src/components/AbstractIndexation.js | Introduces shared indexation request/state engine and shared propTypes/defaultProps. |
| composer.json | Updates YoastCS error threshold value. |
Comment on lines
+49
to
+59
| parent::__construct( | ||
| \sprintf( | ||
| /* translators: 1: indexable object type; 2: object ID; 3: underlying error message. */ | ||
| 'Yoast SEO could not build the %1$s indexable for object %2$d: %3$s', | ||
| $object_type, | ||
| $object_id, | ||
| $previous->getMessage(), | ||
| ), | ||
| 0, | ||
| $previous, | ||
| ); |
Comment on lines
+519
to
+537
| /** | ||
| * Tests that a failing object reported through an Indexing_Failed_Exception is turned into a | ||
| * WP_Error carrying that object's id and type. | ||
| * | ||
| * @covers ::run_indexation_action | ||
| * | ||
| * @return void | ||
| */ | ||
| public function test_index_posts_when_indexing_failed_exception_occurs() { | ||
| $exception = new Indexing_Failed_Exception( 123, 'post', 'post', new Exception( 'The underlying error.' ) ); | ||
|
|
||
| $this->post_indexation_action->expects( 'index' )->once()->andThrow( $exception ); | ||
|
|
||
| $this->indexing_helper->expects( 'indexing_failed' )->once()->withNoArgs(); | ||
|
|
||
| Mockery::mock( WP_Error::class ); | ||
|
|
||
| $this->assertInstanceOf( WP_Error::class, $this->instance->index_posts() ); | ||
| } |
Contributor
There was a problem hiding this comment.
I'll leave this to @pls78 whether he wants to deal with that or not
Comment on lines
141
to
143
| } | ||
|
|
||
| Indexation.propTypes = { | ||
| indexingActions: PropTypes.object, | ||
| preIndexingActions: PropTypes.object, | ||
| indexingStateCallback: PropTypes.func, | ||
| }; | ||
|
|
||
| Indexation.defaultProps = { | ||
| indexingActions: {}, | ||
| preIndexingActions: {}, | ||
| indexingStateCallback: () => {}, | ||
| }; | ||
|
|
||
| export default Indexation; |
Contributor
There was a problem hiding this comment.
I'll leave this to @pls78 whether he wants to deal with that or not
leonidasmi
reviewed
Jul 9, 2026
leonidasmi
left a comment
leonidasmi
left a comment
Contributor
There was a problem hiding this comment.
CR: 🏗️
A couple of remarks aside from the inline ones:
object_idcan be null for id-less types. Forhome-page,date-archive,post-type-archive, andsystem-page indexablesobject_id is null,- so the exception message renders "for object 0" and the CLI prints "Could not optimize system-page #0 while indexing general objects". I think we should improve that, for better UX (maybe the message could fall back to object_sub_type for those types?)
- it also means that we should update the
$object_iddocblocks (in exception constructor and the action doc) to beint|null
- There are cases where building an indexable means building an additional indexable during the initial build (eg. when building a post indexable, we also do
$this->maybe_build_author_indexable( $indexable->author_id ).- Which means that if there's something wrong with building the second/inner indexable, we will initiate the improvements of this PR twice. Specifically:
- a second log entry, a second action fire (now with the post's identity and the already-wrapped exception as $exception), and a message like "could not build the post indexable for object 5: Yoast SEO could not build the user indexable for object 3: …".
- There's an easy fix where we can add a
catch ( Indexing_Failed_Exception $exception )beforecatch ( Throwable $exception ), in the builder - if we decide to do that, let's also add test instructions for that case
- Which means that if there's something wrong with building the second/inner indexable, we will initiate the improvements of this PR twice. Specifically:
Also, let's add test instructions for the Logger addition, since support might use this PR for future reference and I suspect that they are gonna use our Logger for troubleshooting :)
Comment on lines
+442
to
+458
| /** | ||
| * Fires when an indexable could not be built because of an unexpected error. | ||
| * | ||
| * This action lets third parties observe build failures themselves. | ||
| * | ||
| * @param int $object_id The object ID of the indexable that failed to build. | ||
| * @param string $object_type The object type of the indexable that failed to build. | ||
| * @param string|null $object_sub_type The object sub type of the indexable that failed to build. | ||
| * @param Throwable $exception The error that caused the failure. | ||
| */ | ||
| \do_action( | ||
| 'wpseo_indexable_indexing_failed', | ||
| $indexable->object_id, | ||
| $indexable->object_type, | ||
| $indexable->object_sub_type, | ||
| $exception, | ||
| ); |
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.
Context
When an unexpected error occurred while building an indexable during SEO data optimization, the failure surfaced as an opaque error: the user had no indication of which object could not be optimized. This PR carries the failing object's identity (type + ID) from the indexable builder all the way through to the WP-CLI output and the REST/React error UI, and gives third parties a hook to react to the failure.
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Indexing_Failed_Exception(extendingIndexable_Exception) wraps the underlying throwable and carries the object'sobject_id,object_type, andobject_sub_type.wpseo_indexable_indexing_failedaction so third parties can react to a failed indexable build.AbstractIndexationcomponent so the Tools page and the first-time configuration share a single code path, with the render layer kept in thin subclasses.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR changes what the user sees when an indexable cannot be built during SEO data optimization. To test it you need to (a) make sure indexing actually runs, (b) force a build failure, then (c) check the CLI output and the React error UI. Finally, confirm the normal (happy-path) optimization still works once the forced failure is removed.
Setup
Force a build failure (temporary code change)
src/builders/indexable-post-builder.phpand, as the very first line ofpublic function build( $post_id, $indexable )(around line 115), add:Indexable_Builder::build()catches, logs, fireswpseo_indexable_indexing_failedfor, and re-throws asIndexing_Failed_Exception— exactly the path this PR adds, with object typepostand a real post ID.Check the React error UI (Tools page)
post #{id}(the ID of the post that failed). The Request URL, Request method, Status code, Error message, and a collapsible "Error stack trace" should also be present.Check the WP-CLI output
wp yoast index(in this worktree-based Docker setup, run it inside the cli container or viawp-env run cli wp yoast index).Could not optimize post #{id} while indexing posts: Simulated indexing failureand stops with an error, instead of an opaque failure.(Optional) Third-party hook + log
WP_DEBUG_LOGenabled, the builder's own logger also writes the failure todebug.log.Confirm the happy path still works (no regression)
throw(and the optional hook).AbstractIndexation) as the Tools page.Relevant test scenarios
{object_type} #{object_id}for whichever indexable fails — to see a non-post type, move the temporarythrowinto the matching builder (e.g.indexable-term-builder.phpforterm #{id}).Test instructions for QA when the code is in the RC
Forcing a build failure requires a temporary code change, so QA can focus on the happy path plus a visual check of the error UI:
wp yoast indexWP-CLI command still indexes content normally and reports completion.throwinindexable-post-builder.php), confirm the Tools-page error alert's "Error details" shows a "Failing object" line aspost #{id}, and thatwp yoast indexprintsCould not optimize post #{id} while indexing posts: …. Remove the temporary change afterward.Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Indexable_Builder) — now logs, fireswpseo_indexable_indexing_failed, and re-throws asIndexing_Failed_Exceptionon unexpected errors.wp yoast indexWP-CLI command and the indexing REST route — both now handle the new exception.AbstractIndexationengine, so the happy-path indexing flow should be regression-tested in both places.Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand committed the results, if my PR introduces or edits images or SVGs.Innovation
innovationlabel.Fixes #943