Wrap painless explain error (#103151) (#103234)

In #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
elastic · Dec 9, 2023 · ba5839d · ba5839d
1 parent 7e48f4c
commit ba5839d
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git a/docs/changelog/103151.yaml 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
@@ -23,6 +23,7 @@ class ErrorCauseWrapper extends ElasticsearchException {
 
     private static final List<Class<? extends Error>> 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
@@ -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());
     }