Skip to content
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

Fix the output of put-mapping errors #3464

Merged
merged 2 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions includes/classes/Command.php
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ private function put_mapping_helper( $args, $assoc_args ) {
sprintf(
/* translators: Error message */
esc_html__( 'Mapping failed: %s', 'elasticpress' ),
$result->get_error_message()
Utils\get_elasticsearch_error_reason( $result->get_error_message() )
)
);
}
Expand Down Expand Up @@ -330,7 +330,7 @@ private function put_mapping_helper( $args, $assoc_args ) {
sprintf(
/* translators: Error message */
esc_html__( 'Mapping failed: %s', 'elasticpress' ),
$result->get_error_message()
Utils\get_elasticsearch_error_reason( $result->get_error_message() )
)
);
}
Expand Down Expand Up @@ -371,7 +371,7 @@ private function put_mapping_helper( $args, $assoc_args ) {
sprintf(
/* translators: Error message */
esc_html__( 'Mapping failed: %s', 'elasticpress' ),
$result->get_error_message()
Utils\get_elasticsearch_error_reason( $result->get_error_message() )
)
);
}
Expand Down
7 changes: 5 additions & 2 deletions includes/classes/IndexHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -1337,8 +1337,11 @@ protected function on_error_update_and_clean( $error, $context = 'sync' ) {

switch ( $context ) {
case 'mapping':
/* translators: Error message */
$message = sprintf( esc_html__( 'Mapping failed: %s', 'elasticpress' ), $error['message'] );
$message = sprintf(
/* translators: Error message */
esc_html__( 'Mapping failed: %s', 'elasticpress' ),
Utils\get_elasticsearch_error_reason( $error['message'] )
);
$message .= "\n";
$message .= esc_html__( 'Mapping has failed, which will cause ElasticPress search results to be incorrect. Please click `Delete all Data and Start a Fresh Sync` to retry mapping.', 'elasticpress' );
break;
Expand Down
10 changes: 1 addition & 9 deletions includes/classes/StatusReport/FailedQueries.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,7 @@ protected function maybe_clear_logs() {
* @return array The error in index 0, solution in index 1
*/
public function analyze_log( $log ) {
$error = '';

if ( ! empty( $log['result']['error'] ) && ! empty( $log['result']['error']['root_cause'][0]['reason'] ) ) {
$error = $log['result']['error']['root_cause'][0]['reason'];
}

if ( ! empty( $log['result']['errors'] ) && ! empty( $log['result']['items'] ) && ! empty( $log['result']['items'][0]['index']['error']['reason'] ) ) {
$error = $log['result']['items'][0]['index']['error']['reason'];
}
$error = Utils\get_elasticsearch_error_reason( $log );

$solution = ( ! empty( $error ) ) ?
$this->maybe_suggest_solution_for_es( $error ) :
Expand Down
31 changes: 31 additions & 0 deletions includes/utils.php
Original file line number Diff line number Diff line change
Expand Up @@ -787,3 +787,34 @@ function generate_request_id() : string {
*/
return apply_filters( 'ep_request_id', get_request_id_base() . $uuid );
}

/**
* Given an Elasticsearch response, try to find an error message.
*
* @since 4.6.0
* @param mixed $response The Elasticsearch response
* @return string
*/
function get_elasticsearch_error_reason( $response ) : string {
if ( is_string( $response ) ) {
return $response;
}

if ( ! is_array( $response ) ) {
return var_export( $response, true ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions
}

if ( ! empty( $response['reason'] ) ) {
return $response['reason'];
}

if ( ! empty( $response['result']['error'] ) && ! empty( $response['result']['error']['root_cause'][0]['reason'] ) ) {
return $response['result']['error']['root_cause'][0]['reason'];
}

if ( ! empty( $response['result']['errors'] ) && ! empty( $response['result']['items'] ) && ! empty( $response['result']['items'][0]['index']['error']['reason'] ) ) {
return $response['result']['items'][0]['index']['error']['reason'];
}

return '';
}
60 changes: 60 additions & 0 deletions tests/php/TestUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -329,4 +329,64 @@ public function testGetPostMapCapabilities() {

$this->assertSame( $expected, Utils\get_post_map_capabilities() );
}

/**
* Test the `get_elasticsearch_error_reason` function
*
* @since 4.6.0
*/
public function testGetElasticsearchErrorReason() {
// Strings should be returned without any change
$this->assertSame( 'Some message', Utils\get_elasticsearch_error_reason( 'Some message' ) );

// Objects will be returned after passing through var_export()
$object = (object) [ 'attribute' => 'this will be an object' ];
$return = Utils\get_elasticsearch_error_reason( $object );
$this->assertIsString( $return );
$this->assertStringContainsString( 'attribute', $return );
$this->assertStringContainsString( 'this will be an object', $return );

// `reason` in the array root
$reason_root = [ 'reason' => 'Error reason' ];
$this->assertSame( 'Error reason', Utils\get_elasticsearch_error_reason( $reason_root ) );

// array with `error`
$reason_in_single_error_array = [
'result' => [
'error' => [
'root_cause' => [
[
'reason' => 'Error reason',
],
],
],
],
];
$this->assertSame( 'Error reason', Utils\get_elasticsearch_error_reason( $reason_in_single_error_array ) );

// array with `errors`
$reason_in_errors_array = [
'result' => [
'errors' => [
'some error',
],
'items' => [
[
'index' => [
'error' => [
'reason' => 'Error reason',
],
],
],
],
],
];
$this->assertSame( 'Error reason', Utils\get_elasticsearch_error_reason( $reason_in_errors_array ) );

// For something that is an array but does not have a format of an error, return an empty string
$not_an_error = [
'results' => [ 1, 2, 3 ],
];
$this->assertSame( '', Utils\get_elasticsearch_error_reason( $not_an_error ) );
}
}