-
Notifications
You must be signed in to change notification settings - Fork 25.4k
Change GeneralScriptException to 400 #133659
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
base: main
Are you sure you want to change the base?
Conversation
Errors in scripts are generally due to user errors in the script itself. ScriptException has a 400 status code reflecting that fact, but GeneralScriptException uses the default 500 status code. This commit fixes GeneralScriptException to match the 400 status code of other scripting errors.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
Hi @rjernst, I've created a changelog YAML for you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For posterity, we caught this in a scenario where the cause was a circuit breaking exception, whose status code was ignored.
org.elasticsearch.script.GeneralScriptException: Failed to compile inline script [if (doc.containsKey('live_start') && !doc['live_start'].empty && doc.containsKey('live_end') && !doc['live_end'].empty) { long now = 1751987618176L; boolean afterStart = now >= doc['live_start'].value.toInstant().toEpochMilli(); boolean beforeEnd = now < doc['live_end'].value.toInstant().toEpochMilli(); return afterStart && beforeEnd ? 1 : 0; } return 0;] using lang [painless]
at [email protected]/org.elasticsearch.script.ScriptCache.compile(ScriptCache.java:120)
at [email protected]/org.elasticsearch.script.ScriptService.compile(ScriptService.java:634)
at [email protected]/org.elasticsearch.index.query.SearchExecutionContext.compile(SearchExecutionContext.java:581)
at [email protected]/org.elasticsearch.search.sort.ScriptSortBuilder.fieldComparatorSource(ScriptSortBuilder.java:331)
at [email protected]/org.elasticsearch.search.sort.ScriptSortBuilder.build(ScriptSortBuilder.java:251)
at [email protected]/org.elasticsearch.search.sort.SortBuilder.buildSort(SortBuilder.java:161)
at [email protected]/org.elasticsearch.search.sort.SortBuilder.buildSort(SortBuilder.java:153)
at [email protected]/org.elasticsearch.search.SearchService.parseSource(SearchService.java:1574)
at [email protected]/org.elasticsearch.search.SearchService.createContext(SearchService.java:1335)
at [email protected]/org.elasticsearch.search.SearchService.executeQueryPhase(SearchService.java:869)
at [email protected]/org.elasticsearch.search.SearchService.lambda$executeQueryPhase$7(SearchService.java:710)
at [email protected]/org.elasticsearch.action.ActionRunnable$3.accept(ActionRunnable.java:79)
at [email protected]/org.elasticsearch.action.ActionRunnable$3.accept(ActionRunnable.java:76)
at [email protected]/org.elasticsearch.action.ActionRunnable$4.doRun(ActionRunnable.java:101)
at [email protected]/org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:27)
at [email protected]/org.elasticsearch.common.util.concurrent.TimedRunnable.doRun(TimedRunnable.java:35)
at [email protected]/org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingAbstractRunnable.doRun(ThreadContext.java:1044)
at [email protected]/org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:27)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1095)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:619)
at java.base/java.lang.Thread.run(Thread.java:1447)
Caused by: org.elasticsearch.common.breaker.CircuitBreakingException: [script] Too many dynamic script compilations within, max: [150/5m]; please use indexed, or scripts with parameters instead; this limit can be changed by the [script.max_compilations_rate] setting
at [email protected]/org.elasticsearch.script.ScriptCache.checkCompilationLimit(ScriptCache.java:178)
at [email protected]/org.elasticsearch.script.ScriptCache.lambda$compile$0(ScriptCache.java:107)
at [email protected]/org.elasticsearch.common.cache.Cache.computeIfAbsent(Cache.java:440)
at [email protected]/org.elasticsearch.script.ScriptCache.compile(ScriptCache.java:91)
... 20 more
Another path in that specific case may have been to surface the status code of the root cause perhaps (in that case 429 then), but I am not familiar enough with script exceptions to tell which way is better.
Errors in scripts are generally due to user errors in the script itself. ScriptException has a 400 status code reflecting that fact, but GeneralScriptException uses the default 500 status code. This commit fixes GeneralScriptException to match the 400 status code of other scripting errors.