Return 400 for user-facing planner validation errors - #22452
Return 400 for user-facing planner validation errors#22452finnegancarroll wants to merge 2 commits into
Conversation
PR Code Suggestions ✨Latest suggestions up to aeaf843 Explore these optional code suggestions:
Previous suggestionsSuggestions up to commit 5ca119e
Suggestions up to commit 91fbad3
Suggestions up to commit 91fbad3
Suggestions up to commit 67ba41d
Suggestions up to commit 9716136
|
|
❌ Gradle check result for 028a643: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
PR Reviewer Guide 🔍(Review updated until commit 7f95ec2)Here are some key observations to aid the review process:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #22452 +/- ##
============================================
- Coverage 71.47% 71.45% -0.02%
+ Complexity 76978 76936 -42
============================================
Files 6156 6156
Lines 358443 358443
Branches 52246 52246
============================================
- Hits 256192 256143 -49
+ Misses 81951 81919 -32
- Partials 20300 20381 +81 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
028a643 to
9716136
Compare
|
Persistent review updated to latest commit 9716136 |
de97832 to
67ba41d
Compare
|
Persistent review updated to latest commit 67ba41d |
67ba41d to
91fbad3
Compare
|
Persistent review updated to latest commit 91fbad3 |
|
Persistent review updated to latest commit 91fbad3 |
91fbad3 to
5ca119e
Compare
|
Persistent review updated to latest commit 5ca119e |
|
❌ Gradle check result for 5ca119e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
5ca119e to
b0da0d5
Compare
|
Persistent review updated to latest commit b0da0d5 |
|
❌ Gradle check result for b0da0d5: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
b0da0d5 to
94a46cb
Compare
|
Persistent review updated to latest commit f41d522 |
f41d522 to
a87d57c
Compare
a87d57c to
aeaf843
Compare
|
Persistent review updated to latest commit aeaf843 |
aeaf843 to
c946cc2
Compare
|
Persistent review updated to latest commit c946cc2 |
|
Persistent review updated to latest commit c946cc2 |
c946cc2 to
6f3dea1
Compare
|
Persistent review updated to latest commit 6f3dea1 |
|
❕ Gradle check result for 6f3dea1: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
2b2be24 to
f3f273f
Compare
|
Persistent review updated to latest commit f3f273f |
f3f273f to
c54a98e
Compare
|
Persistent review updated to latest commit c54a98e |
|
❌ Gradle check result for c54a98e: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Changes IllegalStateException to IllegalArgumentException for two specific planner error paths that represent user-caused issues: 1. Field not found in field storage: user's query references a field that doesn't exist in the index mapping (e.g., typo in field name). 2. Unrecognized scalar function in filter: user used an unsupported UDF in a WHERE clause. All other planner IllegalStateExceptions remain as-is since they represent engine misconfiguration or bugs (no user action can trigger them). Signed-off-by: Finn Carroll <carrofin@amazon.com>
c54a98e to
59dc795
Compare
|
Persistent review updated to latest commit 59dc795 |
|
❌ Gradle check result for 59dc795: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
…+ Jenkins timeout) Signed-off-by: Finn Carroll <carrofin@amazon.com>
|
Persistent review updated to latest commit 7f95ec2 |
Description
Changes two specific
IllegalStateExceptionthrows toIllegalArgumentExceptionin the analytics engine planner. These are error paths triggered by user-caused issues that should return HTTP 400 (actionable error) rather than 500 (redacted internal error).Changes
FieldStorageResolver.javaOpenSearchFilterRule.java