[Java] Log invocation binding failures (#3160)

clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java

@@ -167,6 +167,10 @@ public class HubConnection {
             for (HubMessage message : messages) {
                 logger.debug("Received message of type {}.", message.getMessageType());
                 switch (message.getMessageType()) {
+                    case INVOCATION_BINDING_FAILURE:
+                        InvocationBindingFailureMessage msg = (InvocationBindingFailureMessage)message;
+                        logger.error("Failed to bind arguments received in invocation '{}' of '{}'.", msg.getInvocationId(), msg.getTarget(), msg.getException());
+                        break;
                     case INVOCATION:
                         InvocationMessage invocationMessage = (InvocationMessage) message;
                         List<InvocationHandler> handlers = this.handlers.get(invocationMessage.getTarget());

clients/java/signalr/src/main/java/com/microsoft/signalr/HubMessageType.java

@@ -10,7 +10,8 @@ enum HubMessageType {
     STREAM_INVOCATION(4),
     CANCEL_INVOCATION(5),
     PING(6),
-    CLOSE(7);
+    CLOSE(7),
+    INVOCATION_BINDING_FAILURE(-1);
 
     public int value;
     HubMessageType(int id) { this.value = id; }

clients/java/signalr/src/main/java/com/microsoft/signalr/InvocationBindingFailureMessage.java

@@ -0,0 +1,33 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+package com.microsoft.signalr;
+
+class InvocationBindingFailureMessage extends HubMessage {
+    private final String invocationId;
+    private final String target;
+    private final Exception exception;
+
+    public InvocationBindingFailureMessage(String invocationId, String target, Exception exception) {
+        this.invocationId = invocationId;
+        this.target = target;
+        this.exception = exception;
+    }
+
+    public String getInvocationId() {
+        return invocationId;
+    }
+
+    public String getTarget() {
+        return target;
+    }
+
+    public Exception getException() {
+        return exception;
+    }
+
+    @Override
+    public HubMessageType getMessageType() {
+        return HubMessageType.INVOCATION_BINDING_FAILURE;
+    }
+}

clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java

@@ -52,6 +52,7 @@ class JsonHubProtocol implements HubProtocol {
                 ArrayList<Object> arguments = null;
                 JsonArray argumentsToken = null;
                 Object result = null;
+                Exception argumentBindingException = null;
                 JsonElement resultToken = null;
                 JsonReader reader = new JsonReader(new StringReader(str));
                 reader.beginObject();
@@ -83,8 +84,26 @@ class JsonHubProtocol implements HubProtocol {
                             break;
                         case "arguments":
                             if (target != null) {
-                                List<Class<?>> types = binder.getParameterTypes(target);
+                                boolean startedArray = false;
-                                arguments = bindArguments(reader, types);
+                                try {
+                                    List<Class<?>> types = binder.getParameterTypes(target);
+                                    startedArray = true;
+                                    arguments = bindArguments(reader, types);
+                                } catch (Exception ex) {
+                                    argumentBindingException = ex;
+
+                                    // Could be at any point in argument array JSON when an error is thrown
+                                    // Read until the end of the argument JSON array
+                                    if (!startedArray) {
+                                        reader.beginArray();
+                                    }
+                                    while (reader.hasNext()) {
+                                        reader.skipValue();
+                                    }
+                                    if (reader.peek() == JsonToken.END_ARRAY) {
+                                        reader.endArray();
+                                    }
+                                }
                             } else {
                                 argumentsToken = (JsonArray)jsonParser.parse(reader);
                             }
@@ -104,13 +123,21 @@ class JsonHubProtocol implements HubProtocol {
                 switch (messageType) {
                     case INVOCATION:
                         if (argumentsToken != null) {
-                            List<Class<?>> types = binder.getParameterTypes(target);
+                            try {
-                            arguments = bindArguments(argumentsToken, types);
+                                List<Class<?>> types = binder.getParameterTypes(target);
+                                arguments = bindArguments(argumentsToken, types);
+                            } catch (Exception ex) {
+                                argumentBindingException = ex;
+                            }
                         }
-                        if (arguments == null) {
+                        if (argumentBindingException != null) {
-                            hubMessages.add(new InvocationMessage(invocationId, target, new Object[0]));
+                            hubMessages.add(new InvocationBindingFailureMessage(invocationId, target, argumentBindingException));
                         } else {
-                            hubMessages.add(new InvocationMessage(invocationId, target, arguments.toArray()));
+                            if (arguments == null) {
+                                hubMessages.add(new InvocationMessage(invocationId, target, new Object[0]));
+                            } else {
+                                hubMessages.add(new InvocationMessage(invocationId, target, arguments.toArray()));
+                            }
                         }
                         break;
                     case COMPLETION:
@@ -183,7 +210,11 @@ class JsonHubProtocol implements HubProtocol {
             throw new RuntimeException(String.format("Invocation provides %d argument(s) but target expects %d.", argCount, paramCount));
         }
 
+        // Do this at the very end, because if we throw for any reason above, we catch at the call site
+        // And manually consume the rest of the array, if we called endArray before throwing the RuntimeException
+        // Then we can't correctly consume the rest of the json object
         reader.endArray();
+
         return arguments;
     }
 }

clients/java/signalr/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java

@@ -81,12 +81,7 @@ class OkHttpWebSocketWrapper extends WebSocketWrapper {
 
         @Override
         public void onMessage(WebSocket webSocket, String message) {
-            try {
+            onReceive.invoke(message);
-                onReceive.invoke(message);
-            } catch (Exception e) {
-                // TODO Auto-generated catch block
-                e.printStackTrace();
-            }
         }
 
         @Override

clients/java/signalr/src/test/java/com/microsoft/signalr/HubConnectionTest.java

@@ -904,7 +904,7 @@ class HubConnectionTest {
     }
 
     @Test
-    public void errorWhenReceivingInvokeWithIncorrectArgumentLength()  {
+    public void doesNotErrorWhenReceivingInvokeWithIncorrectArgumentLength()  {
         MockTransport mockTransport = new MockTransport();
         HubConnection hubConnection = TestUtils.createHubConnection("http://example.com", mockTransport);
         hubConnection.on("Send", (s) -> {
@@ -913,8 +913,8 @@ class HubConnectionTest {
 
         hubConnection.start().timeout(1, TimeUnit.SECONDS).blockingAwait();
 
-        RuntimeException exception = assertThrows(RuntimeException.class, () -> mockTransport.receiveMessage("{\"type\":1,\"target\":\"Send\",\"arguments\":[]}" + RECORD_SEPARATOR));
+        mockTransport.receiveMessage("{\"type\":1,\"target\":\"Send\",\"arguments\":[]}" + RECORD_SEPARATOR);
-        assertEquals("Invocation provides 0 argument(s) but target expects 1.", exception.getMessage());
+        hubConnection.stop();
     }
 
     @Test

clients/java/signalr/src/test/java/com/microsoft/signalr/JsonHubProtocolTest.java

@@ -220,30 +220,64 @@ class JsonHubProtocolTest {
     }
 
     @Test
-    public void errorWhileParsingTooManyArgumentsWithOutOfOrderProperties() {
+    public void invocationBindingFailureWhileParsingTooManyArgumentsWithOutOfOrderProperties() {
         String stringifiedMessage = "{\"arguments\":[42, 24],\"type\":1,\"target\":\"test\"}\u001E";
         TestBinder binder = new TestBinder(new InvocationMessage(null, "test", new Object[] { 42 }));
 
-        RuntimeException exception = assertThrows(RuntimeException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder));
+        HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder);
-        assertEquals("Invocation provides 2 argument(s) but target expects 1.", exception.getMessage());
+        assertEquals(1, messages.length);
+        assertEquals(InvocationBindingFailureMessage.class, messages[0].getClass());
+        InvocationBindingFailureMessage message = (InvocationBindingFailureMessage)messages[0];
+        assertEquals("Invocation provides 2 argument(s) but target expects 1.", message.getException().getMessage());
     }
 
     @Test
-    public void errorWhileParsingTooManyArguments() {
+    public void invocationBindingFailureWhileParsingTooManyArguments() {
         String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[42, 24]}\u001E";
         TestBinder binder = new TestBinder(new InvocationMessage(null, "test", new Object[] { 42 }));
 
-        RuntimeException exception = assertThrows(RuntimeException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder));
+        HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder);
-        assertEquals("Invocation provides 2 argument(s) but target expects 1.", exception.getMessage());
+        assertEquals(1, messages.length);
+        assertEquals(InvocationBindingFailureMessage.class, messages[0].getClass());
+        InvocationBindingFailureMessage message = (InvocationBindingFailureMessage) messages[0];
+        assertEquals("Invocation provides 2 argument(s) but target expects 1.", message.getException().getMessage());
     }
 
     @Test
-    public void errorWhileParsingTooFewArguments() {
+    public void invocationBindingFailureWhileParsingTooFewArguments() {
         String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[42]}\u001E";
         TestBinder binder = new TestBinder(new InvocationMessage(null, "test", new Object[] { 42, 24 }));
 
-        RuntimeException exception = assertThrows(RuntimeException.class, () -> jsonHubProtocol.parseMessages(stringifiedMessage, binder));
+        HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder);
-        assertEquals("Invocation provides 1 argument(s) but target expects 2.", exception.getMessage());
+        assertEquals(1, messages.length);
+        assertEquals(InvocationBindingFailureMessage.class, messages[0].getClass());
+        InvocationBindingFailureMessage message = (InvocationBindingFailureMessage) messages[0];
+        assertEquals("Invocation provides 1 argument(s) but target expects 2.", message.getException().getMessage());
+    }
+
+    @Test
+    public void invocationBindingFailureWhenParsingIncorrectType() {
+        String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[\"true\"]}\u001E";
+        TestBinder binder = new TestBinder(new InvocationMessage(null, "test", new Object[] { 42 }));
+
+        HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder);
+        assertEquals(1, messages.length);
+        assertEquals(InvocationBindingFailureMessage.class, messages[0].getClass());
+        InvocationBindingFailureMessage message = (InvocationBindingFailureMessage) messages[0];
+        assertEquals("java.lang.NumberFormatException: For input string: \"true\"", message.getException().getMessage());
+    }
+
+    @Test
+    public void invocationBindingFailureStillReadsJsonPayloadAfterFailure() {
+        String stringifiedMessage = "{\"type\":1,\"target\":\"test\",\"arguments\":[\"true\"],\"invocationId\":\"123\"}\u001E";
+        TestBinder binder = new TestBinder(new InvocationMessage(null, "test", new Object[] { 42 }));
+
+        HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder);
+        assertEquals(1, messages.length);
+        assertEquals(InvocationBindingFailureMessage.class, messages[0].getClass());
+        InvocationBindingFailureMessage message = (InvocationBindingFailureMessage) messages[0];
+        assertEquals("java.lang.NumberFormatException: For input string: \"true\"", message.getException().getMessage());
+        assertEquals("123", message.getInvocationId());
     }
 
     @Test