Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-2139

Errors during request serialization in WebSocketGremlinRequestEncoder/NioGremlinRequestEncoder are not reported to the client

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.4.1
    • Fix Version/s: 3.3.6, 3.4.1
    • Component/s: driver
    • Labels:
      None

      Description

      Running something like client.submit(TinkerFactory.createModern().traversal().addV("x").property("c", Color.RED)) will fail with the below error rather than reporting what the actual error was:

      io.netty.handler.codec.EncoderException: WebSocketGremlinRequestEncoder must produce at least one message.> is a io.netty.handler.codec.EncoderException
      

      This seems to be because WebSocketGremlinRequestEncoder#encode(..) just logs the error, but doesn't report it back to the client.

      Throwing an exception in encode() reports it correctly back to the client, but the session will be closed.

      Putting the below test method into GremlinDriverIntegrateTest reproduces the issue:

      @Test
      public void shouldReportErrorWhenRequestCantBeSerialized() throws ExecutionException, InterruptedException {
          final Cluster cluster = TestClientFactory.build().serializer(Serializers.GRAPHSON_V3D0).create();
          final Client client = cluster.connect().alias("g");
      
      
          try {
              final ResultSet results = client.submit(TinkerFactory.createModern().traversal().addV("x").property("c", Color.RED));
              results.all().get();
              fail("Should have thrown exception over bad serialization");
          } catch (Exception ex) {
              final Throwable inner = ExceptionUtils.getRootCause(ex);
              assertThat(inner, instanceOf(ResponseException.class));
              assertEquals(ResponseStatusCode.SERVER_ERROR_SERIALIZATION, ((ResponseException) inner).getResponseStatusCode());
              assertTrue(ex.getMessage().contains("An error occurred during serialization of this request"));
              assertTrue(ex.getMessage().contains("Serializer for type java.awt.Color not found"));
          }
      
          // should not die completely just because we had a bad serialization error.  that kind of stuff happens
          // from time to time, especially in the console if you're just exploring.
          assertEquals(2, client.submit("1+1").all().get().get(0).getInt());
      
          cluster.close();
      }

      Tested with 3.4.1 but it most likely fails with other versions as well.

      This is the diff I had tested it with:

      @@ -58,7 +61,9 @@ public final class WebSocketGremlinRequestEncoder extends MessageToMessageEncode
                       objects.add(new TextWebSocketFrame(textSerializer.serializeRequestAsString(requestMessage)));
                   }
               } catch (Exception ex) {
      -            logger.warn(String.format("An error occurred during serialization of this request [%s] - it could not be sent to the server.", requestMessage), ex);
      +            String errorMsg = String.format("An error occurred during serialization of this request [%s] - it could not be sent to the server - Reason: %s", requestMessage, ex);
      +            logger.warn(errorMsg, ex);
      +            throw new ResponseException(ResponseStatusCode.SERVER_ERROR_SERIALIZATION, errorMsg);
               }
      
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                spmallette stephen mallette
                Reporter:
                eduard.tudenhoefner Eduard Tudenhoefner
              • Votes:
                1 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: