diff --git a/clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java b/clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java index 7c29721fc6..f9efa28c28 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java +++ b/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 handlers = this.handlers.get(invocationMessage.getTarget()); diff --git a/clients/java/signalr/src/main/java/com/microsoft/signalr/HubMessageType.java b/clients/java/signalr/src/main/java/com/microsoft/signalr/HubMessageType.java index 99de262769..588a82e6d1 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/signalr/HubMessageType.java +++ b/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; } diff --git a/clients/java/signalr/src/main/java/com/microsoft/signalr/InvocationBindingFailureMessage.java b/clients/java/signalr/src/main/java/com/microsoft/signalr/InvocationBindingFailureMessage.java new file mode 100644 index 0000000000..cda90f9c94 --- /dev/null +++ b/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; + } +} diff --git a/clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java b/clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java index 9922ca13e2..8ff1024b4b 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java +++ b/clients/java/signalr/src/main/java/com/microsoft/signalr/JsonHubProtocol.java @@ -52,6 +52,7 @@ class JsonHubProtocol implements HubProtocol { ArrayList 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> types = binder.getParameterTypes(target); - arguments = bindArguments(reader, types); + boolean startedArray = false; + try { + List> 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> types = binder.getParameterTypes(target); - arguments = bindArguments(argumentsToken, types); + try { + List> types = binder.getParameterTypes(target); + arguments = bindArguments(argumentsToken, types); + } catch (Exception ex) { + argumentBindingException = ex; + } } - if (arguments == null) { - hubMessages.add(new InvocationMessage(invocationId, target, new Object[0])); + if (argumentBindingException != null) { + 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; } } diff --git a/clients/java/signalr/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java b/clients/java/signalr/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java index b9d19c3e92..f62d0a140e 100644 --- a/clients/java/signalr/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java +++ b/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); - } catch (Exception e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } + onReceive.invoke(message); } @Override diff --git a/clients/java/signalr/src/test/java/com/microsoft/signalr/HubConnectionTest.java b/clients/java/signalr/src/test/java/com/microsoft/signalr/HubConnectionTest.java index bd14c0a7a5..6c29be1c82 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/signalr/HubConnectionTest.java +++ b/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)); - assertEquals("Invocation provides 0 argument(s) but target expects 1.", exception.getMessage()); + mockTransport.receiveMessage("{\"type\":1,\"target\":\"Send\",\"arguments\":[]}" + RECORD_SEPARATOR); + hubConnection.stop(); } @Test diff --git a/clients/java/signalr/src/test/java/com/microsoft/signalr/JsonHubProtocolTest.java b/clients/java/signalr/src/test/java/com/microsoft/signalr/JsonHubProtocolTest.java index 9c9bbdce25..8a8ad9f3cd 100644 --- a/clients/java/signalr/src/test/java/com/microsoft/signalr/JsonHubProtocolTest.java +++ b/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)); - assertEquals("Invocation provides 2 argument(s) but target expects 1.", exception.getMessage()); + HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + 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)); - assertEquals("Invocation provides 2 argument(s) but target expects 1.", exception.getMessage()); + HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + 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)); - assertEquals("Invocation provides 1 argument(s) but target expects 2.", exception.getMessage()); + HubMessage[] messages = jsonHubProtocol.parseMessages(stringifiedMessage, binder); + 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