From 3a1b0dc6c5732a7c2675f0a50837c8ca41cf19de Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Fri, 8 Dec 2023 22:03:28 -0800 Subject: [PATCH] Wrap painless explain error (#103151) In https://github.com/elastic/elasticsearch/pull/100872 Painless errors were wrapped so as to avoid throwing Errors outside scripting. However, one case was missed: PainlessExplainError which is used by Debug.explain. This commit adds the explain error to those that painless wraps. closes #103018 --- docs/changelog/103151.yaml | 6 ++++++ .../painless/ErrorCauseWrapper.java | 1 + .../org/elasticsearch/painless/DebugTests.java | 16 ++++++++++------ 3 files changed, 17 insertions(+), 6 deletions(-) create mode 100644 docs/changelog/103151.yaml diff --git a/docs/changelog/103151.yaml b/docs/changelog/103151.yaml new file mode 100644 index 0000000000000..bd9eea97cac6d --- /dev/null +++ b/docs/changelog/103151.yaml @@ -0,0 +1,6 @@ +pr: 103151 +summary: Wrap painless explain error +area: Infra/Scripting +type: bug +issues: + - 103018 diff --git a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java index aeaf44bfd014c..308d6223c666e 100644 --- a/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java +++ b/modules/lang-painless/src/main/java/org/elasticsearch/painless/ErrorCauseWrapper.java @@ -23,6 +23,7 @@ class ErrorCauseWrapper extends ElasticsearchException { private static final List> wrappedErrors = List.of( PainlessError.class, + PainlessExplainError.class, OutOfMemoryError.class, StackOverflowError.class, LinkageError.class diff --git a/modules/lang-painless/src/test/java/org/elasticsearch/painless/DebugTests.java b/modules/lang-painless/src/test/java/org/elasticsearch/painless/DebugTests.java index 87b199cd1b43f..48da785e801d3 100644 --- a/modules/lang-painless/src/test/java/org/elasticsearch/painless/DebugTests.java +++ b/modules/lang-painless/src/test/java/org/elasticsearch/painless/DebugTests.java @@ -20,6 +20,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; @@ -30,29 +31,32 @@ public class DebugTests extends ScriptTestCase { public void testExplain() { // Debug.explain can explain an object Object dummy = new Object(); - PainlessExplainError e = expectScriptThrows( - PainlessExplainError.class, - () -> exec("Debug.explain(params.a)", singletonMap("a", dummy), true) - ); + var wrapper = expectScriptThrows(ErrorCauseWrapper.class, () -> exec("Debug.explain(params.a)", singletonMap("a", dummy), true)); + assertThat(wrapper.realCause.getClass(), equalTo(PainlessExplainError.class)); + var e = (PainlessExplainError) wrapper.realCause; assertSame(dummy, e.getObjectToExplain()); assertThat(e.getHeaders(painlessLookup), hasEntry("es.to_string", singletonList(dummy.toString()))); assertThat(e.getHeaders(painlessLookup), hasEntry("es.java_class", singletonList("java.lang.Object"))); assertThat(e.getHeaders(painlessLookup), hasEntry("es.painless_class", singletonList("java.lang.Object"))); // Null should be ok - e = expectScriptThrows(PainlessExplainError.class, () -> exec("Debug.explain(null)")); + wrapper = expectScriptThrows(ErrorCauseWrapper.class, () -> exec("Debug.explain(null)")); + assertThat(wrapper.realCause.getClass(), equalTo(PainlessExplainError.class)); + e = (PainlessExplainError) wrapper.realCause; assertNull(e.getObjectToExplain()); assertThat(e.getHeaders(painlessLookup), hasEntry("es.to_string", singletonList("null"))); assertThat(e.getHeaders(painlessLookup), not(hasKey("es.java_class"))); assertThat(e.getHeaders(painlessLookup), not(hasKey("es.painless_class"))); // You can't catch the explain exception - e = expectScriptThrows(PainlessExplainError.class, () -> exec(""" + wrapper = expectScriptThrows(ErrorCauseWrapper.class, () -> exec(""" try { Debug.explain(params.a) } catch (Exception e) { return 1 }""", singletonMap("a", dummy), true)); + assertThat(wrapper.realCause.getClass(), equalTo(PainlessExplainError.class)); + e = (PainlessExplainError) wrapper.realCause; assertSame(dummy, e.getObjectToExplain()); }