From b0297a33afd5324b1c678ab3da1d89e2505280ac Mon Sep 17 00:00:00 2001 From: Felipe Elia Date: Wed, 27 Sep 2023 17:22:12 -0300 Subject: [PATCH 1/4] New ElasticsearchErrorInterpreter class --- .../classes/ElasticsearchErrorInterpreter.php | 109 ++++++++++++++++++ includes/classes/IndexHelper.php | 24 +++- .../classes/StatusReport/FailedQueries.php | 66 +---------- 3 files changed, 137 insertions(+), 62 deletions(-) create mode 100644 includes/classes/ElasticsearchErrorInterpreter.php diff --git a/includes/classes/ElasticsearchErrorInterpreter.php b/includes/classes/ElasticsearchErrorInterpreter.php new file mode 100644 index 0000000000..b719cb2032 --- /dev/null +++ b/includes/classes/ElasticsearchErrorInterpreter.php @@ -0,0 +1,109 @@ + 'no such index [???]', + 'solution' => sprintf( + /* translators: 1. Index name; 2. Sync Page URL */ + __( 'It seems the %1$s index is missing. Delete all data and sync to fix the issue.', 'elasticpress' ), + '' . $matches[1] . '', + $sync_url + ), + ]; + } + + if ( preg_match( '/No mapping found for \[(.*?)\] in order to sort on/', $error, $matches ) ) { + return [ + 'error' => 'No mapping found for [???] in order to sort on', + 'solution' => sprintf( + /* translators: 1. Index name; 2. Sync Page URL */ + __( 'The field %1$s was not found. Make sure it is added to the list of indexed fields and run a new sync to fix the issue.', 'elasticpress' ), + '' . $matches[1] . '', + $sync_url + ), + ]; + } + + /* translators: 1. Field name; 2. Sync Page URL */ + $field_type_solution = __( 'It seems you saved a post without doing a full sync first because %1$s is missing the correct mapping type. Delete all data and sync to fix the issue.', 'elasticpress' ); + + if ( preg_match( '/Fielddata is disabled on text fields by default. Set fielddata=true on \[(.*?)\]/', $error, $matches ) ) { + return [ + 'error' => 'Fielddata is disabled on text fields by default. Set fielddata=true on [???]', + 'solution' => sprintf( $field_type_solution, $matches[1], $sync_url ), + ]; + } + + if ( preg_match( '/field \[(.*?)\] is of type \[(.*?)\], but only numeric types are supported./', $error, $matches ) ) { + return [ + 'error' => 'field [???] is of type [???], but only numeric types are supported.', + 'solution' => sprintf( $field_type_solution, $matches[1], $sync_url ), + ]; + } + + if ( preg_match( '/Alternatively, set fielddata=true on \[(.*?)\] in order to load field data by uninverting the inverted index./', $error, $matches ) ) { + return [ + 'error' => 'Alternatively, set fielddata=true on [???] in order to load field data by uninverting the inverted index.', + 'solution' => sprintf( $field_type_solution, $matches[1], $sync_url ), + ]; + } + + if ( preg_match( '/Limit of total fields \[(.*?)\] in index \[(.*?)\] has been exceeded/', $error, $matches ) ) { + return [ + 'error' => 'Limit of total fields [???] in index [???] has been exceeded', + 'solution' => sprintf( + /* translators: Elasticsearch or ElasticPress.io; 2. Link to article; 3. Link to article */ + __( 'Your website content has more public custom fields than %1$s is able to store. Check our articles about Elasticsearch field limitations and how to index just the custom fields you need and sync again.', 'elasticpress' ), + Utils\is_epio() ? __( 'ElasticPress.io', 'elasticpress' ) : __( 'Elasticsearch', 'elasticpress' ), + 'https://elasticpress.zendesk.com/hc/en-us/articles/360051401212-I-get-the-error-Limit-of-total-fields-in-index-has-been-exceeded-', + 'https://elasticpress.zendesk.com/hc/en-us/articles/360052019111' + ), + ]; + } + + if ( Utils\is_epio() ) { + return [ + 'error' => $error, + 'solution' => sprintf( + /* translators: ElasticPress.io My Account URL */ + __( 'We did not recognize this error. Please open an ElasticPress.io support ticket so we can troubleshoot further.', 'elasticpress' ), + 'https://www.elasticpress.io/my-account/' + ), + ]; + } + + return [ + 'error' => $error, + 'solution' => sprintf( + /* translators: New GitHub issue URL */ + __( 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors. ', 'elasticpress' ), + 'https://github.com/10up/ElasticPress/issues/new/choose' + ), + ]; + } +} diff --git a/includes/classes/IndexHelper.php b/includes/classes/IndexHelper.php index 3f7453f9ae..811b76fd54 100644 --- a/includes/classes/IndexHelper.php +++ b/includes/classes/IndexHelper.php @@ -13,7 +13,7 @@ namespace ElasticPress; -use ElasticPress\Utils as Utils; +use ElasticPress\Utils; /** * Index Helper Class. @@ -1031,6 +1031,10 @@ protected function output( $message_text, $type = 'info', $context = '' ) { 'status' => $type, ]; + if ( in_array( $type, [ 'warning', 'error' ], true ) ) { + $message['errors'] = $this->build_message_errors_data( $message_text ); + } + if ( is_callable( $this->args['output_method'] ) ) { call_user_func( $this->args['output_method'], $message, $this->args, $this->index_meta, $context ); } @@ -1466,6 +1470,24 @@ protected function flush_messages_queue() { } } + /** + * Get data for a given error message(s) + * + * @since 5.0.0 + * @param string|array $messages Messages + * @return array + */ + protected function build_message_errors_data( $messages ) : array { + $messages = (array) $messages; + $error_interpreter = new \ElasticPress\ElasticsearchErrorInterpreter(); + + $errors_data = []; + foreach ( $messages as $message ) { + $errors_data[] = $error_interpreter->maybe_suggest_solution_for_es( $message ); + } + return $errors_data; + } + /** * Return singleton instance of class. * diff --git a/includes/classes/StatusReport/FailedQueries.php b/includes/classes/StatusReport/FailedQueries.php index 9206dfb32d..ad642be7cb 100644 --- a/includes/classes/StatusReport/FailedQueries.php +++ b/includes/classes/StatusReport/FailedQueries.php @@ -158,78 +158,22 @@ public function analyze_log( $log ) { $error = Utils\get_elasticsearch_error_reason( $log ); $solution = ( ! empty( $error ) ) ? - $this->maybe_suggest_solution_for_es( $error ) : + ( new \ElasticPress\ElasticsearchErrorInterpreter() )->maybe_suggest_solution_for_es( $error )['solution'] : ''; return [ $error, $solution ]; } /** - * Given an Elasticsearch error, try to suggest a solution + * DEPRECATED. Given an Elasticsearch error, try to suggest a solution * + * @deprecated 5.0.0 * @param string $error The error * @return string */ protected function maybe_suggest_solution_for_es( $error ) { - $sync_url = Utils\get_sync_url(); - - if ( preg_match( '/no such index \[(.*?)\]/', $error, $matches ) ) { - return sprintf( - /* translators: 1. Index name; 2. Sync Page URL */ - __( 'It seems the %1$s index is missing. Delete all data and sync to fix the issue.', 'elasticpress' ), - '' . $matches[1] . '', - $sync_url - ); - } - - if ( preg_match( '/No mapping found for \[(.*?)\] in order to sort on/', $error, $matches ) ) { - return sprintf( - /* translators: 1. Index name; 2. Sync Page URL */ - __( 'The field %1$s was not found. Make sure it is added to the list of indexed fields and run a new sync to fix the issue.', 'elasticpress' ), - '' . $matches[1] . '', - $sync_url - ); - } - - /* translators: 1. Field name; 2. Sync Page URL */ - $field_type_solution = __( 'It seems you saved a post without doing a full sync first because %1$s is missing the correct mapping type. Delete all data and sync to fix the issue.', 'elasticpress' ); - - if ( preg_match( '/Fielddata is disabled on text fields by default. Set fielddata=true on \[(.*?)\]/', $error, $matches ) ) { - return sprintf( $field_type_solution, $matches[1], $sync_url ); - } - - if ( preg_match( '/field \[(.*?)\] is of type \[(.*?)\], but only numeric types are supported./', $error, $matches ) ) { - return sprintf( $field_type_solution, $matches[1], $sync_url ); - } - - if ( preg_match( '/Alternatively, set fielddata=true on \[(.*?)\] in order to load field data by uninverting the inverted index./', $error, $matches ) ) { - return sprintf( $field_type_solution, $matches[1], $sync_url ); - } - - if ( preg_match( '/Limit of total fields \[(.*?)\] in index \[(.*?)\] has been exceeded/', $error, $matches ) ) { - return sprintf( - /* translators: Elasticsearch or ElasticPress.io; 2. Link to article; 3. Link to article */ - __( 'Your website content has more public custom fields than %1$s is able to store. Check our articles about Elasticsearch field limitations and how to index just the custom fields you need and sync again.', 'elasticpress' ), - Utils\is_epio() ? __( 'ElasticPress.io', 'elasticpress' ) : __( 'Elasticsearch', 'elasticpress' ), - 'https://elasticpress.zendesk.com/hc/en-us/articles/360051401212-I-get-the-error-Limit-of-total-fields-in-index-has-been-exceeded-', - 'https://elasticpress.zendesk.com/hc/en-us/articles/360052019111' - ); - } - - // field limit - - if ( Utils\is_epio() ) { - return sprintf( - /* translators: ElasticPress.io My Account URL */ - __( 'We did not recognize this error. Please open an ElasticPress.io support ticket so we can troubleshoot further.', 'elasticpress' ), - 'https://www.elasticpress.io/my-account/' - ); - } + _deprecated_function( __METHOD__, '5.0.0', '\ElasticPress\ElasticsearchErrorInterpreter::maybe_suggest_solution_for_es()' ); - return sprintf( - /* translators: New GitHub issue URL */ - __( 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors. ', 'elasticpress' ), - 'https://github.com/10up/ElasticPress/issues/new/choose' - ); + return ( new \ElasticPress\ElasticsearchErrorInterpreter() )->maybe_suggest_solution_for_es( $error )['solution']; } } From 376bf0934456a86fee5efa21e287de6c1091714a Mon Sep 17 00:00:00 2001 From: Felipe Elia Date: Thu, 28 Sep 2023 15:07:43 -0300 Subject: [PATCH 2/4] Tests for ElasticsearchErrorInterpreter --- .../classes/ElasticsearchErrorInterpreter.php | 2 +- .../php/TestElasticsearchErrorInterpreter.php | 135 ++++++++++++++++++ 2 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 tests/php/TestElasticsearchErrorInterpreter.php diff --git a/includes/classes/ElasticsearchErrorInterpreter.php b/includes/classes/ElasticsearchErrorInterpreter.php index b719cb2032..1f022c5858 100644 --- a/includes/classes/ElasticsearchErrorInterpreter.php +++ b/includes/classes/ElasticsearchErrorInterpreter.php @@ -101,7 +101,7 @@ public function maybe_suggest_solution_for_es( $error ) { 'error' => $error, 'solution' => sprintf( /* translators: New GitHub issue URL */ - __( 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors. ', 'elasticpress' ), + __( 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors.', 'elasticpress' ), 'https://github.com/10up/ElasticPress/issues/new/choose' ), ]; diff --git a/tests/php/TestElasticsearchErrorInterpreter.php b/tests/php/TestElasticsearchErrorInterpreter.php new file mode 100644 index 0000000000..12a6271027 --- /dev/null +++ b/tests/php/TestElasticsearchErrorInterpreter.php @@ -0,0 +1,135 @@ +GitHub Issue so we can add it to our list of supported errors.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( $error, $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when an index is missing + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_no_index() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'no such index [elasticpresstest-post-1]'; + $solution = 'It seems the elasticpresstest-post-1 index is missing. Delete all data and sync to fix the issue.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'no such index [???]', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when sorting on a wrong field + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_order() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'No mapping found for [fieldname] in order to sort on'; + $solution = 'The field fieldname was not found. Make sure it is added to the list of indexed fields and run a new sync to fix the issue.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'No mapping found for [???] in order to sort on', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when using the wrong mapping (fielddata error) + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_wrong_mapping_fielddata() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'Fielddata is disabled on text fields by default. Set fielddata=true on [fieldname]'; + $solution = 'It seems you saved a post without doing a full sync first because fieldname is missing the correct mapping type. Delete all data and sync to fix the issue.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'Fielddata is disabled on text fields by default. Set fielddata=true on [???]', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when using the wrong mapping (numeric error) + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_wrong_mapping_numeric() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'field [fieldname] is of type [type], but only numeric types are supported.'; + $solution = 'It seems you saved a post without doing a full sync first because fieldname is missing the correct mapping type. Delete all data and sync to fix the issue.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'field [???] is of type [???], but only numeric types are supported.', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when using the wrong mapping (inverted error) + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_wrong_mapping_inverted() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'Alternatively, set fielddata=true on [fieldname] in order to load field data by uninverting the inverted index.'; + $solution = 'It seems you saved a post without doing a full sync first because fieldname is missing the correct mapping type. Delete all data and sync to fix the issue.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'Alternatively, set fielddata=true on [???] in order to load field data by uninverting the inverted index.', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } + + /** + * Test the `maybe_suggest_solution_for_es` method when the fields limit is reached + * + * @group elasticsearch-error-interpreter + */ + public function test_maybe_suggest_solution_for_es_limit_fields() { + $error_interpreter = new ElasticsearchErrorInterpreter(); + $sync_url = Utils\get_sync_url(); + + $error = 'Limit of total fields [1000] in index [elasticpresstest-post-1] has been exceeded'; + $solution = 'Your website content has more public custom fields than Elasticsearch is able to store. Check our articles about Elasticsearch field limitations and how to index just the custom fields you need and sync again.'; + $suggested = $error_interpreter->maybe_suggest_solution_for_es( $error ); + + $this->assertSame( 'Limit of total fields [???] in index [???] has been exceeded', $suggested['error'] ); + $this->assertSame( $solution, $suggested['solution'] ); + } +} From 97aafe80be7b92b9d4070371f99c5a92c86e898f Mon Sep 17 00:00:00 2001 From: Felipe Elia Date: Thu, 28 Sep 2023 15:13:27 -0300 Subject: [PATCH 3/4] Fix test --- tests/php/TestStatusReport.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/TestStatusReport.php b/tests/php/TestStatusReport.php index 8a6d611014..03c6018d35 100644 --- a/tests/php/TestStatusReport.php +++ b/tests/php/TestStatusReport.php @@ -357,7 +357,7 @@ function( $logs ) use ( $time_stamp, $random_no ) { ), 'recommended_solution' => array( 'label' => 'Recommended Solution', - 'value' => 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors. ', + 'value' => 'We did not recognize this error. Please consider opening a GitHub Issue so we can add it to our list of supported errors.', ), 'es_req' => array( 'label' => 'Elasticsearch Request', From 6f58ffd91007f942bd5d953503c2b39a4e433fef Mon Sep 17 00:00:00 2001 From: Felipe Elia Date: Thu, 28 Sep 2023 17:33:18 -0300 Subject: [PATCH 4/4] Aggregate different errors by type --- includes/classes/IndexHelper.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/includes/classes/IndexHelper.php b/includes/classes/IndexHelper.php index 811b76fd54..be1bd38d3c 100644 --- a/includes/classes/IndexHelper.php +++ b/includes/classes/IndexHelper.php @@ -1481,11 +1481,20 @@ protected function build_message_errors_data( $messages ) : array { $messages = (array) $messages; $error_interpreter = new \ElasticPress\ElasticsearchErrorInterpreter(); - $errors_data = []; + $errors_list = []; foreach ( $messages as $message ) { - $errors_data[] = $error_interpreter->maybe_suggest_solution_for_es( $message ); + $error = $error_interpreter->maybe_suggest_solution_for_es( $message ); + + if ( ! isset( $errors_list[ $error['error'] ] ) ) { + $errors_list[ $error['error'] ] = [ + 'solution' => $error['solution'], + 'count' => 1, + ]; + } else { + $errors_list[ $error['error'] ]['count']++; + } } - return $errors_data; + return $errors_list; } /**