GitHub user dimas-b opened a pull request:
https://github.com/apache/tinkerpop/pull/899
TINKERPOP-2005: Intermittent NullPointerException in response handling
Address
[TINKERPOP-2005](https://issues.apache.org/jira/browse/TINKERPOP-2005) by
ignoring more than one "final" response messages from `AbstractEvalOpProcessor`.
Add `isFinalResponse()` getter to `ResponseStatusCode`
Introduce `ResponseHandlerContext` to allow tracking the final response
status per request message.
Update `AbstractOpProcessor`, `AbstractEvalOpProcessor` and related classes
to write response messages through `ResponseHandlerContext` methods as
opposed to `ChannelHandlerContext` methods.
----
In addition to the failure mode described in
[TINKERPOP-2005](https://issues.apache.org/jira/browse/TINKERPOP-2005)
non-timeout script evaluation exceptions in AbstractEvalOpProcessor may occur
after the script has started executing. In this situation it it critical to
prevent potentially writing multiple final (e.g. error vs. success) responses
back to the client. This failure mode is hard to replicate in a pure Gremlin
server, however the new `AbstractEvalOpProcessorTest` class provides coverage
for it.
----
Testing performed:
* Full suite of unit tests (i.e. `mvn clean install`)
* Full suite of `gremlin-server` integration tests (`mvn verify -pl
gremlin-server -DskipIntegrationTests=false`)
* Manual testing according to comment under
[TINKERPOP-2005](https://issues.apache.org/jira/browse/TINKERPOP-2005).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dimas-b/tinkerpop TINKERPOP-2005-alt
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/899.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 #899
----
commit f592e3446c84e9398e242a072cdfec64025e9566
Author: Dmitri Bourlatchkov <dmitri.bourlatchkov@...>
Date: 2018-07-27T19:54:06Z
TINKERPOP-2005 Reject multiple final responses in AbstractEvalOpProcessor
Add isFinalResponse() getter to ResponseStatusCode
Introduce ResponseHandlerContext to allow tracking the final response
status per request message.
Update AbstractOpProcessor, AbstractEvalOpProcessor and related classes
to write response messages through ResponseHandlerContext methods as
opposed to ChannelHandlerContext methods.
commit b7a44953c8ac62f308b78709ab2565a78eddb1af
Author: Dmitri Bourlatchkov <dmitri.bourlatchkov@...>
Date: 2018-07-30T16:38:21Z
TINKERPOP-2005 Handle evaluation excetions in AbstractEvalOpProcessor
Some script evaluation exceptions in AbstractEvalOpProcessor may occur
after the script has started executing. In this situation it it critical
to prevent potentially writing multiple final (e.g. error vs. success)
responses back to the client.
Exceptions that used to escape from evalOpInternal(...) would be
converted to error response messages by OpExecutorHandler, which could
coincide with a successful response from a quick script.
This change makes AbstractEvalOpProcessor do the error message
writing for the same type of exceptions that are handled by
OpExecutorHandler. However, AbstractEvalOpProcessor makes sure that
at most one final reponse message is sent by using
ResponseHandlerContext.
Also ResponseHandlerContext.writeAndFlush(...) methods will no longer
throw exceptions for attempts to send multiple final messages. This is
again to avoid multiple error response messages sent from
OpExecutorHandler.
commit 4ff4b88a5b663f86917299691edc6f84a6bd1c67
Author: Dmitri Bourlatchkov <dmitri.bourlatchkov@...>
Date: 2018-07-30T19:57:14Z
TINKERPOP-2005 Encapsulate test eval timeout overrides in a method
This is to avoid directly exposing other settings whose changes may
not be effective until the server is restarted.
----
---
