[Java] Catch errors from user callbacks (#25513)

* [Java] Catch errors from user callbacks

* test logger

* rebase

* fb

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

@@ -223,7 +223,11 @@ public class HubConnection implements AutoCloseable {
                         List<InvocationHandler> handlers = this.handlers.get(invocationMessage.getTarget());
                         if (handlers != null) {
                             for (InvocationHandler handler : handlers) {
-                                handler.getAction().invoke(invocationMessage.getArguments());
+                                try {
+                                    handler.getAction().invoke(invocationMessage.getArguments());
+                                } catch (Exception e) {
+                                    logger.error("Invoking client side method '{}' failed:", invocationMessage.getTarget(), e);
+                                }
                             }
                         } else {
                             logger.warn("Failed to find handler for '{}' method.", invocationMessage.getTarget());

src/SignalR/clients/java/signalr/test/build.gradle

@@ -5,7 +5,7 @@ dependencies {
     compile 'org.junit.jupiter:junit-jupiter-params:5.3.1'
     runtime 'org.junit.jupiter:junit-jupiter-engine:5.3.1'
     implementation 'com.google.code.gson:gson:2.8.5'
-    testCompile 'org.slf4j:slf4j-jdk14:1.7.25'
+    compile 'ch.qos.logback:logback-classic:1.2.3'
     implementation project(':core')
     implementation project(':messagepack')
     compile project(':messagepack')

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

@@ -18,6 +18,7 @@ import java.util.concurrent.atomic.AtomicReference;
 
 import org.junit.jupiter.api.Test;
 
+import ch.qos.logback.classic.spi.ILoggingEvent;
 import io.reactivex.Completable;
 import io.reactivex.Observable;
 import io.reactivex.Single;
@@ -2565,6 +2566,37 @@ class HubConnectionTest {
         assertEquals((short) 5, (short) person.getT());
     }
 
+    @Test
+    public void throwFromOnHandlerRunsAllHandlers() {
+        AtomicReference<String> value1 = new AtomicReference<>();
+        AtomicReference<String> value2 = new AtomicReference<>();
+
+        try (TestLogger logger = new TestLogger()) {
+            MockTransport mockTransport = new MockTransport();
+            HubConnection hubConnection = TestUtils.createHubConnection("http://example.com", mockTransport);
+
+            hubConnection.on("inc", (param1) -> {
+                value1.set(param1);
+                throw new RuntimeException("throw from on handler");
+            }, String.class);
+            hubConnection.on("inc", (param1) -> {
+                value2.set(param1);
+            }, String.class);
+
+            hubConnection.start().timeout(1, TimeUnit.SECONDS).blockingAwait();
+            mockTransport.receiveMessage("{\"type\":1,\"target\":\"inc\",\"arguments\":[\"Hello World\"]}" + RECORD_SEPARATOR);
+
+            // Confirming that our handler was called and the correct message was passed in.
+            assertEquals("Hello World", value1.get());
+            assertEquals("Hello World", value2.get());
+
+            hubConnection.stop().timeout(1, TimeUnit.SECONDS).blockingAwait();
+
+            ILoggingEvent log = logger.assertLog("Invoking client side method 'inc' failed:");
+            assertEquals("throw from on handler", log.getThrowableProxy().getMessage());
+        }
+    }
+
     @Test
     public void receiveHandshakeResponseAndMessage() {
         AtomicReference<Double> value = new AtomicReference<Double>(0.0);

src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/TestLogger.java

@@ -0,0 +1,45 @@
+// 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;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.slf4j.LoggerFactory;
+
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+
+class TestLogger implements AutoCloseable {
+    private Logger logger;
+    private ListAppender<ILoggingEvent> appender;
+
+    public TestLogger() {
+        this("com.microsoft.signalr.HubConnection");
+    }
+
+    public TestLogger(String category) {
+        this.logger = (Logger)LoggerFactory.getLogger(category);
+        this.appender = new ListAppender<ILoggingEvent>();
+        this.appender.start();
+        this.logger.addAppender(this.appender);
+    }
+
+    public ILoggingEvent assertLog(String logMessage) {
+        for (ILoggingEvent log : appender.list) {
+            if (log.getFormattedMessage().startsWith(logMessage)) {
+                return log;
+            }
+        }
+
+        assertTrue(false, String.format("Log message '%s' not found", logMessage));
+        return null;
+    }
+
+    @Override
+    public void close() {
+        this.logger.detachAppender(this.appender);
+    }
+
+}