From 9eb9de6c8c089853c4041885c2230a7ac0acda1b Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 11 Sep 2020 11:44:00 -0700 Subject: [PATCH] [Java] Catch errors from user callbacks (#25513) * [Java] Catch errors from user callbacks * test logger * rebase * fb --- .../com/microsoft/signalr/HubConnection.java | 6 ++- .../clients/java/signalr/test/build.gradle | 2 +- .../microsoft/signalr/HubConnectionTest.java | 32 +++++++++++++ .../com/microsoft/signalr/TestLogger.java | 45 +++++++++++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/TestLogger.java diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java index 25b6139127..8ff26762f6 100644 --- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java +++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/HubConnection.java @@ -223,7 +223,11 @@ public class HubConnection implements AutoCloseable { List 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()); diff --git a/src/SignalR/clients/java/signalr/test/build.gradle b/src/SignalR/clients/java/signalr/test/build.gradle index 75a451b929..2fefd54f64 100644 --- a/src/SignalR/clients/java/signalr/test/build.gradle +++ b/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') diff --git a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java index de8db977a9..5839dd2f66 100644 --- a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/HubConnectionTest.java +++ b/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 value1 = new AtomicReference<>(); + AtomicReference 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 value = new AtomicReference(0.0); diff --git a/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/TestLogger.java b/src/SignalR/clients/java/signalr/test/src/main/java/com/microsoft/signalr/TestLogger.java new file mode 100644 index 0000000000..2bc1a58eae --- /dev/null +++ b/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 appender; + + public TestLogger() { + this("com.microsoft.signalr.HubConnection"); + } + + public TestLogger(String category) { + this.logger = (Logger)LoggerFactory.getLogger(category); + this.appender = new ListAppender(); + 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); + } + +} \ No newline at end of file