You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2017/11/20 20:31:35 UTC
[GitHub] tinkerpop pull request #753: TINKERPOP-1811 Fixed bytecode deserialization e...
GitHub user spmallette opened a pull request:
https://github.com/apache/tinkerpop/pull/753
TINKERPOP-1811 Fixed bytecode deserialization error messaging
As the original issue description suggests the error message wasn't very helpful. Seems there was a mistype in the log message that prevented a better output and also some error handling that wasn't quite being dealt with properly.
Error now presents as:
```text
gremlin> bytecode = g.V().asAdmin().bytecode
==>[[], [V()]]
gremlin> bytecode.addStep("broken")
gremlin> bytecode
==>[[], [V(), broken()]]
gremlin> conn.submit(bytecode).next()
org.apache.tinkerpop.gremlin.driver.exception.ResponseException: Could not locate method: DefaultGraphTraversal.broken([])
Type ':help' or ':h' for help.
Display stack trace? [yN]n
```
On the server you now get this:
```text
[ERROR] TraversalOpProcessor - Could not deserialize the Traversal instance
java.lang.IllegalStateException: Could not locate method: DefaultGraphTraversal.broken([])
at org.apache.tinkerpop.gremlin.jsr223.JavaTranslator.invokeMethod(JavaTranslator.java:180)
at org.apache.tinkerpop.gremlin.jsr223.JavaTranslator.translate(JavaTranslator.java:91)
at org.apache.tinkerpop.gremlin.server.op.traversal.TraversalOpProcessor.iterateBytecodeTraversal(TraversalOpProcessor.java:367)
```
builds with `mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false`
VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1811
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/753.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #753
----
commit 95a6ea3f56db83449b62475045d08fc9452467e0
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-11-20T20:26:53Z
TINKERPOP-1811 Fixed bytecode deserialization error messaging
As the original issue description suggests the error message wasn't very helpful. Seems there was a mistype in the log message that prevented a better output and also some error handling that wasn't quite being dealt with properly.
----
---
[GitHub] tinkerpop issue #753: TINKERPOP-1811 Fixed bytecode deserialization error me...
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/753
Besides the suggestion from @dkuppitz , lgtm!
VOTE +1
---
[GitHub] tinkerpop pull request #753: TINKERPOP-1811 Fixed bytecode deserialization e...
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/753#discussion_r154010923
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java ---
@@ -173,6 +173,12 @@ private Object invokeMethod(final Object delegate, final Class returnType, final
for (int i = 0; i < arguments.length; i++) {
argumentsCopy[i] = translateObject(arguments[i]);
}
+
+ // without this initial check iterating an invalid methodName will lead to a null pointer and a less than
+ // great error message for the user.
+ if (!methodCache.containsKey(methodName))
+ throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() + "." + methodName + "(" + Arrays.toString(argumentsCopy) + ")");
--- End diff --
I agree, its more clear with empty parameters in the case of no method args.
---
[GitHub] tinkerpop pull request #753: TINKERPOP-1811 Fixed bytecode deserialization e...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/753
---
[GitHub] tinkerpop issue #753: TINKERPOP-1811 Fixed bytecode deserialization error me...
Posted by aolieman <gi...@git.apache.org>.
Github user aolieman commented on the issue:
https://github.com/apache/tinkerpop/pull/753
Thanks @spmallette , this should really help.
---
[GitHub] tinkerpop pull request #753: TINKERPOP-1811 Fixed bytecode deserialization e...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/753#discussion_r153933994
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/JavaTranslator.java ---
@@ -173,6 +173,12 @@ private Object invokeMethod(final Object delegate, final Class returnType, final
for (int i = 0; i < arguments.length; i++) {
argumentsCopy[i] = translateObject(arguments[i]);
}
+
+ // without this initial check iterating an invalid methodName will lead to a null pointer and a less than
+ // great error message for the user.
+ if (!methodCache.containsKey(methodName))
+ throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() + "." + methodName + "(" + Arrays.toString(argumentsCopy) + ")");
--- End diff --
Maybe I'm a bit too nit-picky here, but I think the empty square brackets (in case of no args) could confuse some people. I would prefer:
```
final String methodArgs = argumentsCopy.length() > 0 ? Arrays.toString(argumentsCopy) : "";
throw new IllegalStateException("Could not locate method: " + delegate.getClass().getSimpleName() +
"." + methodName + "(" + methodArgs + ")");
```
Besides that, VOTE: +1
---