Wrap groovy script exceptions in a serializable Exception object

Fixes #6598

src/main/java/org/elasticsearch/script/groovy/GroovyScriptCompilationException.java

@@ -0,0 +1,38 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.script.groovy;
+
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.rest.RestStatus;
+
+/**
+ * Exception used to wrap groovy script compilation exceptions so they are
+ * correctly serialized between nodes.
+ */
+public class GroovyScriptCompilationException extends ElasticsearchException {
+    public GroovyScriptCompilationException(String message) {
+        super(message);
+    }
+
+    @Override
+    public RestStatus status() {
+        return RestStatus.BAD_REQUEST;
+    }
+}

src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java

@@ -35,9 +35,11 @@
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.customizers.CompilationCustomizer;
 import org.codehaus.groovy.control.customizers.ImportCustomizer;
+import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.script.ExecutableScript;
 import org.elasticsearch.script.ScriptEngineService;
@@ -106,7 +108,14 @@ public boolean sandboxed() {
 
     @Override
     public Object compile(String script) {
-        return loader.parseClass(script, generateScriptName());
+        try {
+            return loader.parseClass(script, generateScriptName());
+        } catch (Throwable e) {
+            if (logger.isTraceEnabled()) {
+                logger.trace("exception compiling Groovy script:", e);
+            }
+            throw new GroovyScriptCompilationException(ExceptionsHelper.detailedMessage(e));
+        }
     }
 
     /**
@@ -129,7 +138,7 @@ public ExecutableScript executable(Object compiledScript, Map<String, Object> va
             if (vars != null) {
                 allVars.putAll(vars);
             }
-            return new GroovyScript(createScript(compiledScript, allVars));
+            return new GroovyScript(createScript(compiledScript, allVars), this.logger);
         } catch (Exception e) {
             throw new ScriptException("failed to build executable script", e);
         }
@@ -145,7 +154,7 @@ public SearchScript search(Object compiledScript, SearchLookup lookup, @Nullable
                 allVars.putAll(vars);
             }
             Script scriptObject = createScript(compiledScript, allVars);
-            return new GroovyScript(scriptObject, lookup);
+            return new GroovyScript(scriptObject, lookup, this.logger);
         } catch (Exception e) {
             throw new ScriptException("failed to build search script", e);
         }
@@ -180,14 +189,16 @@ public static final class GroovyScript implements ExecutableScript, SearchScript
         private final SearchLookup lookup;
         private final Map<String, Object> variables;
         private final UpdateableFloat score;
+        private final ESLogger logger;
 
-        public GroovyScript(Script script) {
-            this(script, null);
+        public GroovyScript(Script script, ESLogger logger) {
+            this(script, null, logger);
         }
 
-        public GroovyScript(Script script, SearchLookup lookup) {
+        public GroovyScript(Script script, SearchLookup lookup, ESLogger logger) {
             this.script = script;
             this.lookup = lookup;
+            this.logger = logger;
             this.variables = script.getBinding().getVariables();
             this.score = new UpdateableFloat(0);
             // Add the _score variable, which will be updated per-document by
@@ -237,7 +248,14 @@ public void setNextSource(Map<String, Object> source) {
 
         @Override
         public Object run() {
-            return script.run();
+            try {
+                return script.run();
+            } catch (Throwable e) {
+                if (logger.isTraceEnabled()) {
+                    logger.trace("exception running Groovy script", e);
+                }
+                throw new GroovyScriptExecutionException(ExceptionsHelper.detailedMessage(e));
+            }
         }
 
         @Override

src/main/java/org/elasticsearch/script/groovy/GroovyScriptExecutionException.java

@@ -0,0 +1,38 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.script.groovy;
+
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.rest.RestStatus;
+
+/**
+ * Exception used to wrap groovy script execution exceptions so they are
+ * correctly serialized between nodes.
+ */
+public class GroovyScriptExecutionException extends ElasticsearchException {
+    public GroovyScriptExecutionException(String message) {
+        super(message);
+    }
+
+    @Override
+    public RestStatus status() {
+        return RestStatus.BAD_REQUEST;
+    }
+}

src/test/java/org/elasticsearch/script/GroovyScriptTests.java

@@ -19,11 +19,20 @@
 
 package org.elasticsearch.script;
 
+import org.elasticsearch.ExceptionsHelper;
+import org.elasticsearch.action.index.IndexRequestBuilder;
+import org.elasticsearch.action.search.SearchPhaseExecutionException;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.test.ElasticsearchIntegrationTest;
 import org.junit.Test;
 
+import java.util.List;
+
+import static com.google.common.collect.Lists.newArrayList;
+import static org.elasticsearch.index.query.FilterBuilders.scriptFilter;
+import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures;
+import static org.hamcrest.Matchers.equalTo;
 
 /**
  * Various tests for Groovy scripting
@@ -47,4 +56,50 @@ public void assertScript(String script) {
                         "; 1\", \"type\": \"number\", \"lang\": \"groovy\"}}}").get();
         assertNoFailures(resp);
     }
+
+    @Test
+    public void testGroovyExceptionSerialization() throws Exception {
+        List<IndexRequestBuilder> reqs = newArrayList();
+        for (int i = 0; i < randomIntBetween(50, 500); i++) {
+            reqs.add(client().prepareIndex("test", "doc", "" + i).setSource("foo", "bar"));
+        }
+        indexRandom(true, false, reqs);
+        try {
+            client().prepareSearch("test").setQuery(constantScoreQuery(scriptFilter("1 == not_found").lang("groovy"))).get();
+            fail("should have thrown an exception");
+        } catch (SearchPhaseExecutionException e) {
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
+                    ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptExecutionException",
+                    ExceptionsHelper.detailedMessage(e).contains("GroovyScriptExecutionException"), equalTo(true));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained not_found",
+                    ExceptionsHelper.detailedMessage(e).contains("No such property: not_found"), equalTo(true));
+        }
+
+        try {
+            client().prepareSearch("test").setQuery(constantScoreQuery(
+                    scriptFilter("pr = Runtime.getRuntime().exec(\"touch /tmp/gotcha\"); pr.waitFor()").lang("groovy"))).get();
+            fail("should have thrown an exception");
+        } catch (SearchPhaseExecutionException e) {
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
+                    ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptCompilationException",
+                    ExceptionsHelper.detailedMessage(e).contains("GroovyScriptCompilationException"), equalTo(true));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained Method calls not allowed on [java.lang.Runtime]",
+                    ExceptionsHelper.detailedMessage(e).contains("Method calls not allowed on [java.lang.Runtime]"), equalTo(true));
+        }
+
+        try {
+            client().prepareSearch("test").setQuery(constantScoreQuery(
+                    scriptFilter("assert false").lang("groovy"))).get();
+            fail("should have thrown an exception");
+        } catch (SearchPhaseExecutionException e) {
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should not contained NotSerializableTransportException",
+                    ExceptionsHelper.detailedMessage(e).contains("NotSerializableTransportException"), equalTo(false));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained GroovyScriptExecutionException",
+                    ExceptionsHelper.detailedMessage(e).contains("GroovyScriptExecutionException"), equalTo(true));
+            assertThat(ExceptionsHelper.detailedMessage(e) + "should have contained an assert error",
+                    ExceptionsHelper.detailedMessage(e).contains("PowerAssertionError[assert false"), equalTo(true));
+        }
+    }
 }