Improve an error message in Gremlin Server around large scripts. CTR
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/258dccb9 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/258dccb9 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/258dccb9 Branch: refs/heads/TINKERPOP-1642 Commit: 258dccb9c1dbe89c9cb439cc9c7fdbde73209b42 Parents: 591a65a Author: Stephen Mallette <[email protected]> Authored: Tue Mar 7 16:06:34 2017 -0500 Committer: Stephen Mallette <[email protected]> Committed: Tue Mar 7 16:06:34 2017 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../server/op/AbstractEvalOpProcessor.java | 20 +++++++++-- .../server/GremlinServerIntegrateTest.java | 35 ++++++++++++++++++-- 3 files changed, 51 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/258dccb9/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 8cc2adc..04541d6 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -36,6 +36,7 @@ TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET) * Fixed an optimization bug in `LazyBarrierStrategy` around appending barriers to the end of a `Traversal`. * Fixed an optimization bug in `PathRetractionStrategy` around appending barriers to the end of a `Traversal`. * `TraverserIterator` in GremlinServer is smart to try and bulk traversers prior to network I/O. +* Improved error handling of compilation failures for very large or highly parameterized script sent to Gremlin Server. * Fixed a bug in `RangeByIsCountStrategy` that changed the meaning of inner traversals. * Improved Gremlin-Python Driver implementation by adding a threaded client with basic connection pooling and support for pluggable websocket clients. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/258dccb9/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java index 48de830..1ba6e36 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java @@ -41,10 +41,13 @@ import org.apache.tinkerpop.gremlin.server.util.MetricManager; import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; import io.netty.channel.ChannelHandlerContext; +import org.codehaus.groovy.control.ErrorCollector; +import org.codehaus.groovy.control.MultipleCompilationErrorsException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.script.Bindings; +import javax.script.ScriptException; import javax.script.SimpleBindings; import java.util.Arrays; import java.util.HashSet; @@ -292,8 +295,21 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor { logger.warn(errorMessage, t); ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT).statusMessage(t.getMessage()).create()); } else { - logger.warn(String.format("Exception processing a script on request [%s].", msg), t); - ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION).statusMessage(t.getMessage()).create()); + // try to trap the specific jvm error of "Method code too large!" to re-write it as something nicer, + // but only re-write if it's the only error because otherwise we might lose some other important + // information related to the failure. at this point, there hasn't been a scenario that has + // presented itself where the "Method code too large!" comes with other compilation errors so + // it seems that this message trumps other compilation errors to some reasonable degree that ends + // up being favorable for this problem + if (t instanceof MultipleCompilationErrorsException && t.getMessage().contains("Method code too large!") && + ((MultipleCompilationErrorsException) t).getErrorCollector().getErrorCount() == 1) { + final String errorMessage = String.format("The Gremlin statement that was submitted exceed the maximum compilation size allowed by the JVM, please split it into multiple smaller statements - %s", msg); + logger.warn(errorMessage); + ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION).statusMessage(errorMessage).create()); + } else { + logger.warn(String.format("Exception processing a script on request [%s].", msg), t); + ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION).statusMessage(t.getMessage()).create()); + } } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/258dccb9/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java ---------------------------------------------------------------------- diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java index fe00a43..af25be5 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java @@ -140,6 +140,9 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration public Settings overrideSettings(final Settings settings) { final String nameOfTest = name.getMethodName(); switch (nameOfTest) { + case "shouldProvideBetterExceptionForMethodCodeTooLarge": + settings.maxContentLength = 4096000; + break; case "shouldRespectHighWaterMarkSettingAndSucceed": settings.writeBufferHighWaterMark = 64; settings.writeBufferLowWaterMark = 32; @@ -407,7 +410,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration cluster.close(); } } - + @Test public void shouldEnableSslAndClientCertificateAuth() { final Cluster cluster = TestClientFactory.build().enableSsl(true) @@ -421,7 +424,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration cluster.close(); } } - + @Test public void shouldEnableSslAndClientCertificateAuthAndFailWithoutCert() { final Cluster cluster = TestClientFactory.build().enableSsl(true).create(); @@ -455,7 +458,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration cluster.close(); } } - + @Test public void shouldRespectHighWaterMarkSettingAndSucceed() throws Exception { // the highwatermark should get exceeded on the server and thus pause the writes, but have no problem catching @@ -1097,4 +1100,30 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration assertEquals(3, g.V().promise(Traversal::toList).join().size()); } + + @Test + public void shouldProvideBetterExceptionForMethodCodeTooLarge() { + final int numberOfParameters = 4000; + final Map<String,Object> b = new HashMap<>(); + + // generate a script with a ton of bindings usage to generate a "code too large" exception + String script = "x = 0"; + for (int ix = 0; ix < numberOfParameters; ix++) { + if (ix > 0 && ix % 100 == 0) { + script = script + ";" + System.lineSeparator() + "x = x"; + } + script = script + " + x" + ix; + b.put("x" + ix, ix); + } + + final Cluster cluster = TestClientFactory.build().maxContentLength(4096000).create(); + final Client client = cluster.connect(); + + try { + client.submit(script, b).all().get(); + fail("Should have tanked out because of number of parameters used and size of the compile script"); + } catch (Exception ex) { + assertThat(ex.getMessage(), containsString("The Gremlin statement that was submitted exceed the maximum compilation size allowed by the JVM")); + } + } }
