|
Description
Peter Vashchuk
2021-07-22 12:54:56 UTC
Created attachment 123046 [details]
Current "Unable to perform your search. Please try again." error
Created attachment 123181 [details] [review] Bug 28736: fix requirements for the correct error message to show up There is existing error message in the code stating: "Unable to understand your search query, please rephrase and try again." which fits perfectly but because it looks for "ParseException" in the warning output it doesn't show up on this page as it's actually "parse_exception". This patch makes that it's also checked if "parse_exception" is present in the warning output. To reproduce: 1) using ES search for something like "// ^ ! { } [ ] .. , <>" that will for sure break the syntax of ES. 2) after the search query fails note that the error is "Unable to perform your search. Please try again." 3) apply the patch 4) search for the same thing again 5) error message should be "Unable to understand your search query, please rephrase and try again." now. Created attachment 123404 [details] [review] Bug 28736: fix requirements for the correct error message to show up There is existing error message in the code stating: "Unable to understand your search query, please rephrase and try again." which fits perfectly but because it looks for "ParseException" in the warning output it doesn't show up on this page as it's actually "parse_exception". This patch makes that it's also checked if "parse_exception" is present in the warning output. To reproduce: 1) using ES search for something like "// ^ ! { } [ ] .. , <>" that will for sure break the syntax of ES. 2) after the search query fails note that the error is "Unable to perform your search. Please try again." 3) apply the patch 4) search for the same thing again 5) error message should be "Unable to understand your search query, please rephrase and try again." now. Signed-off-by: David Nind <david@davidnind.com> Created attachment 123985 [details] [review] Bug 28736: fix requirements for the correct error message to show up There is existing error message in the code stating: "Unable to understand your search query, please rephrase and try again." which fits perfectly but because it looks for "ParseException" in the warning output it doesn't show up on this page as it's actually "parse_exception". This patch makes that it's also checked if "parse_exception" is present in the warning output. To reproduce: 1) using ES search for something like "// ^ ! { } [ ] .. , <>" that will for sure break the syntax of ES. 2) after the search query fails note that the error is "Unable to perform your search. Please try again." 3) apply the patch 4) search for the same thing again 5) error message should be "Unable to understand your search query, please rephrase and try again." now. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 124005 [details] [review] Bug 28736: fix requirements for the correct error message to show up There is existing error message in the code stating: "Unable to understand your search query, please rephrase and try again." which fits perfectly but because it looks for "ParseException" in the warning output it doesn't show up on this page as it's actually "parse_exception". This patch makes that it's also checked if "parse_exception" is present in the warning output. Side note: "ParseException" reaction code was added here: e0f6c4dc Bug 12478: improve error reporting a bit Search::Elasticsearch seems propagates clean ES JSON answer, and in current ES version inside of $@ it contains "parse_exception" string in dumped JSON answer ("'type' => 'parse_exception'"). Old seeked phrase "ParseException" wasn't reproduced, only in ES logs ("Caused by: org.apache.lucene.queryparser.classic.ParseException: Cannot parse ..."). Check for both phrases won't complicate future changes, but this note added for reference and code cleanup if needed. To reproduce: 1) using ES search for something like "// ^ ! { } [ ] .. , <>" that will for sure break the syntax of ES. 2) after the search query fails note that the error is "Unable to perform your search. Please try again." 3) apply the patch 4) search for the same thing again 5) error message should be "Unable to understand your search query, please rephrase and try again." now. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Andrew Nugged <nugged@gmail.com> Created attachment 124006 [details] [review] Bug 28736: (QA follow-up) hints in explanation about broken syntax Add direct hint for the user what might be the issues with search phrase. In most cases we have unescaped or not removed special characters when user copy-pastes book name. If we explain that error happens because there is special search language, user will know what to research because we explicitly will tell that this is NOT the empty result, but search wasn't performed. This "sub process_error" code was added here:
e0f6c4dc Bug 12478: improve error reporting a bit
And it expected to get 'ParseException' inside of $@.
I didn't found any traces and explanations when we have text with "ParseException" from ES, is it older ES versions that had such word? (Search::Elasticsearch seems just propagates clean ES answer though)
Nowadays we have "parse_exception" in "$@" contents, in dumped JSON answer from ES it contains answer body with "'type' => 'parse_exception'" value.
I don't think the "ParseException" presence will make future updates more complicated but let's have this history-of-thoughts noted in the commit message,
Part2: QA-follow-up: in my experience, I had a lot of users thinking that "nothing was found", when ES error happened. I changed the phrase to be wordier but to explain that there MIGHT be results, but the user needs to review his request.
Created attachment 124007 [details] [review] Bug 28736: (QA follow-up) hints in explanation about broken syntax Add direct hint for the user what might be the issues with search phrase. In most cases we have unescaped or not removed special characters when user copy-pastes book name. If we explain that error happens because there is special search language, user will know what to research because we explicitly will tell that this is NOT the empty result, but search wasn't performed. Created attachment 124008 [details] [review] Bug 28736: (QA follow-up) hints in explanation about broken syntax Add direct hint for the user what might be the issues with search phrase. In most cases we have unescaped or not removed special characters when user copy-pastes book name. If we explain that error happens because there is special search language, user will know what to research because we explicitly will tell that this is NOT the empty result, but search wasn't performed. Note for RM: it's now about "human language", not Perl question – please confirm my last patch. I removed the last mine patch, I will create a separate ticket about that. (it's about "Part2: QA-follow-up:" in my message above. Putting back ticket to PQA Pushed to master for 21.11, thanks to everybody involved! |