diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs index 29c2984930..fecd94f8bd 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/DefaultHubDispatcherBenchmark.cs @@ -17,6 +17,7 @@ using Microsoft.AspNetCore.Sockets; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.SignalR.Microbenchmarks { @@ -38,6 +39,8 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks _dispatcher = new DefaultHubDispatcher( serviceScopeFactory, new HubContext(new DefaultHubLifetimeManager(NullLogger>.Instance)), + Options.Create(new HubOptions()), + Options.Create(new HubOptions()), new Logger>(NullLoggerFactory.Instance)); var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default); diff --git a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionContextBenchmark.cs b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionContextBenchmark.cs index b76ba0bb07..354e66d545 100644 --- a/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionContextBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionContextBenchmark.cs @@ -56,7 +56,8 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks { _pipe.AddReadResult(new ValueTask(_handshakeRequestResult)); - await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _successHubProtocolResolver, _userIdProvider); + await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _successHubProtocolResolver, + _userIdProvider, enableDetailedErrors: true); } [Benchmark] @@ -64,7 +65,8 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks { _pipe.AddReadResult(new ValueTask(_handshakeRequestResult)); - await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _failureHubProtocolResolver, _userIdProvider); + await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _failureHubProtocolResolver, + _userIdProvider, enableDetailedErrors: true); } } diff --git a/clients/ts/FunctionalTests/ts/HubConnectionTests.ts b/clients/ts/FunctionalTests/ts/HubConnectionTests.ts index 2f7a92c51b..49f85be013 100644 --- a/clients/ts/FunctionalTests/ts/HubConnectionTests.ts +++ b/clients/ts/FunctionalTests/ts/HubConnectionTests.ts @@ -125,7 +125,7 @@ describe("hubConnection", () => { }); it("rethrows an exception from the server when invoking", (done) => { - const errorMessage = "An unexpected error occurred invoking 'ThrowException' on the server. InvalidOperationException: An error occurred."; + const errorMessage = "An unexpected error occurred invoking 'ThrowException' on the server."; const hubConnection = new HubConnection(TESTHUBENDPOINT_URL, { logger: TestLogger.instance, protocol, @@ -198,7 +198,7 @@ describe("hubConnection", () => { }); it("rethrows an exception from the server when streaming", (done) => { - const errorMessage = "An unexpected error occurred invoking 'StreamThrowException' on the server. InvalidOperationException: An error occurred."; + const errorMessage = "An unexpected error occurred invoking 'StreamThrowException' on the server."; const hubConnection = new HubConnection(TESTHUBENDPOINT_URL, { logger: TestLogger.instance, protocol, @@ -353,7 +353,7 @@ describe("hubConnection", () => { }); hubConnection.onclose((error) => { - expect(error.message).toEqual("Server returned an error on close: Connection closed with an error. InvalidOperationException: Unable to resolve service for type 'System.Object' while attempting to activate 'FunctionalTests.UncreatableHub'."); + expect(error.message).toEqual("Server returned an error on close: Connection closed with an error."); done(); }); hubConnection.start(); diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs index e2a757d618..930e8406f7 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs @@ -225,7 +225,8 @@ namespace Microsoft.AspNetCore.SignalR Task.Factory.StartNew(_abortedCallback, this); } - internal async Task HandshakeAsync(TimeSpan timeout, IList supportedProtocols, IHubProtocolResolver protocolResolver, IUserIdProvider userIdProvider) + internal async Task HandshakeAsync(TimeSpan timeout, IList supportedProtocols, IHubProtocolResolver protocolResolver, + IUserIdProvider userIdProvider, bool enableDetailedErrors) { try { @@ -329,7 +330,8 @@ namespace Microsoft.AspNetCore.SignalR catch (Exception ex) { Log.HandshakeFailed(_logger, ex); - await WriteHandshakeResponseAsync(new HandshakeResponseMessage($"An unexpected error occurred during connection handshake. {ex.GetType().Name}: {ex.Message}")); + var errorMessage = ErrorMessageHelper.BuildErrorMessage("An unexpected error occurred during connection handshake.", ex, enableDetailedErrors); + await WriteHandshakeResponseAsync(new HandshakeResponseMessage(errorMessage)); return false; } } diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionHandler.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionHandler.cs index ea0d5aaaaf..7927e13e06 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionHandler.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubConnectionHandler.cs @@ -2,14 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; -using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.SignalR.Core; using Microsoft.AspNetCore.SignalR.Internal; using Microsoft.AspNetCore.SignalR.Internal.Protocol; -using Microsoft.AspNetCore.Sockets; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -25,6 +22,7 @@ namespace Microsoft.AspNetCore.SignalR private readonly HubOptions _globalHubOptions; private readonly IUserIdProvider _userIdProvider; private readonly HubDispatcher _dispatcher; + private readonly bool _enableDetailedErrors; public HubConnectionHandler(HubLifetimeManager lifetimeManager, IHubProtocolResolver protocolResolver, @@ -42,6 +40,8 @@ namespace Microsoft.AspNetCore.SignalR _logger = loggerFactory.CreateLogger>(); _userIdProvider = userIdProvider; _dispatcher = dispatcher; + + _enableDetailedErrors = _hubOptions.EnableDetailedErrors ?? _globalHubOptions.EnableDetailedErrors ?? false; } public override async Task OnConnectedAsync(ConnectionContext connection) @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.SignalR var connectionContext = new HubConnectionContext(connection, keepAlive, _loggerFactory); - if (!await connectionContext.HandshakeAsync(handshakeTimeout, supportedProtocols, _protocolResolver, _userIdProvider)) + if (!await connectionContext.HandshakeAsync(handshakeTimeout, supportedProtocols, _protocolResolver, _userIdProvider, _enableDetailedErrors)) { return; } @@ -139,9 +139,13 @@ namespace Microsoft.AspNetCore.SignalR private async Task SendCloseAsync(HubConnectionContext connection, Exception exception) { - CloseMessage closeMessage = exception == null - ? CloseMessage.Empty - : new CloseMessage($"Connection closed with an error. {exception.GetType().Name}: {exception.Message}"); + var closeMessage = CloseMessage.Empty; + + if (exception != null) + { + var errorMessage = ErrorMessageHelper.BuildErrorMessage("Connection closed with an error.", exception, _enableDetailedErrors); + closeMessage = new CloseMessage(errorMessage); + } try { diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubOptions.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubOptions.cs index 083cff2d83..83d1a70e1a 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubOptions.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubOptions.cs @@ -17,5 +17,7 @@ namespace Microsoft.AspNetCore.SignalR public TimeSpan? KeepAliveInterval { get; set; } = null; public IList SupportedProtocols { get; set; } = null; + + public bool? EnableDetailedErrors { get; set; } = null; } } diff --git a/src/Microsoft.AspNetCore.SignalR.Core/HubOptionsSetup.cs b/src/Microsoft.AspNetCore.SignalR.Core/HubOptionsSetup.cs index b1854d2c92..e3ee99cf32 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/HubOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/HubOptionsSetup.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using Microsoft.AspNetCore.SignalR; using Microsoft.AspNetCore.SignalR.Internal.Protocol; using Microsoft.Extensions.Options; diff --git a/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs b/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs index b4b29aebd6..1330ca1837 100644 --- a/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs +++ b/src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs @@ -15,6 +15,7 @@ using Microsoft.AspNetCore.SignalR.Internal.Protocol; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.SignalR.Internal { @@ -24,11 +25,14 @@ namespace Microsoft.AspNetCore.SignalR.Internal private readonly IServiceScopeFactory _serviceScopeFactory; private readonly IHubContext _hubContext; private readonly ILogger> _logger; + private readonly bool _enableDetailedErrors; - public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext hubContext, ILogger> logger) + public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext hubContext, IOptions> hubOptions, + IOptions globalHubOptions, ILogger> logger) { _serviceScopeFactory = serviceScopeFactory; _hubContext = hubContext; + _enableDetailedErrors = hubOptions.Value.EnableDetailedErrors ?? globalHubOptions.Value.EnableDetailedErrors ?? false; _logger = logger; DiscoverHubMethods(); } @@ -172,7 +176,9 @@ namespace Microsoft.AspNetCore.SignalR.Internal if (hubMethodInvocationMessage.ArgumentBindingException != null) { Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, hubMethodInvocationMessage.ArgumentBindingException); - await SendInvocationError(hubMethodInvocationMessage, connection, $"Failed to invoke '{hubMethodInvocationMessage.Target}'. {hubMethodInvocationMessage.ArgumentBindingException.Message}"); + var errorMessage = ErrorMessageHelper.BuildErrorMessage($"Failed to invoke '{hubMethodInvocationMessage.Target}' due to an error on the server.", + hubMethodInvocationMessage.ArgumentBindingException, _enableDetailedErrors); + await SendInvocationError(hubMethodInvocationMessage, connection, errorMessage); return; } @@ -209,12 +215,14 @@ namespace Microsoft.AspNetCore.SignalR.Internal catch (TargetInvocationException ex) { Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, ex); - await SendInvocationError(hubMethodInvocationMessage, connection, BuildUnexpectedErrorMessage(hubMethodInvocationMessage.Target, ex.InnerException)); + await SendInvocationError(hubMethodInvocationMessage, connection, + ErrorMessageHelper.BuildErrorMessage($"An unexpected error occurred invoking '{hubMethodInvocationMessage.Target}' on the server.", ex.InnerException, _enableDetailedErrors)); } catch (Exception ex) { Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, ex); - await SendInvocationError(hubMethodInvocationMessage, connection, BuildUnexpectedErrorMessage(hubMethodInvocationMessage.Target, ex)); + await SendInvocationError(hubMethodInvocationMessage, connection, + ErrorMessageHelper.BuildErrorMessage($"An unexpected error occurred invoking '{hubMethodInvocationMessage.Target}' on the server.", ex, _enableDetailedErrors)); } finally { @@ -223,11 +231,6 @@ namespace Microsoft.AspNetCore.SignalR.Internal } } - private string BuildUnexpectedErrorMessage(string methodName, Exception exception) - { - return $"An unexpected error occurred invoking '{methodName}' on the server. {exception.GetType().Name}: {exception.Message}"; - } - private async Task StreamResultsAsync(string invocationId, HubConnectionContext connection, IAsyncEnumerator enumerator) { string error = null; @@ -243,7 +246,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal catch (ChannelClosedException ex) { // If the channel closes from an exception in the streaming method, grab the innerException for the error from the streaming method - error = ex.InnerException == null ? ex.Message : ex.InnerException.Message; + error = ErrorMessageHelper.BuildErrorMessage("An error occurred on the server while streaming results.", ex.InnerException ?? ex, _enableDetailedErrors); } catch (Exception ex) { @@ -251,7 +254,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal if (!(ex is OperationCanceledException && connection.ActiveRequestCancellationSources.TryGetValue(invocationId, out var cts) && cts.IsCancellationRequested)) { - error = ex.Message; + error = ErrorMessageHelper.BuildErrorMessage("An error occurred on the server while streaming results.", ex, _enableDetailedErrors); } } finally diff --git a/src/Microsoft.AspNetCore.SignalR.Core/Internal/ErrorMessageHelper.cs b/src/Microsoft.AspNetCore.SignalR.Core/Internal/ErrorMessageHelper.cs new file mode 100644 index 0000000000..faec5ced4a --- /dev/null +++ b/src/Microsoft.AspNetCore.SignalR.Core/Internal/ErrorMessageHelper.cs @@ -0,0 +1,20 @@ +// 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. + +using System; + +namespace Microsoft.AspNetCore.SignalR.Internal +{ + internal static class ErrorMessageHelper + { + internal static string BuildErrorMessage(string message, Exception exception, bool includeExceptionDetails) + { + if (!includeExceptionDetails) + { + return message; + } + + return message + $" {exception.GetType().Name}: {exception.Message}"; + } + } +} diff --git a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs index eeaaae07f8..1d2f556dac 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs @@ -415,7 +415,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests var channel = await connection.StreamAsChannelAsync("StreamException").OrTimeout(); var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); - Assert.Equal("An unexpected error occurred invoking 'StreamException' on the server. InvalidOperationException: Error occurred while streaming.", ex.Message); + Assert.Equal("An unexpected error occurred invoking 'StreamException' on the server.", ex.Message); } catch (Exception ex) { @@ -469,7 +469,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests await connection.StartAsync().OrTimeout(); var ex = await Assert.ThrowsAsync(() => connection.InvokeAsync("Echo", "p1", 42)).OrTimeout(); - Assert.Equal("Failed to invoke 'Echo'. Invocation provides 2 argument(s) but target expects 1.", ex.Message); + Assert.Equal("Failed to invoke 'Echo' due to an error on the server.", ex.Message); } catch (Exception ex) { @@ -496,7 +496,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests await connection.StartAsync().OrTimeout(); var ex = await Assert.ThrowsAsync(() => connection.InvokeAsync("Echo", new int[] { 42 })).OrTimeout(); - Assert.StartsWith("Failed to invoke 'Echo'. Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message); + Assert.StartsWith("Failed to invoke 'Echo' due to an error on the server.", ex.Message); } catch (Exception ex) { @@ -553,7 +553,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests var channel = await connection.StreamAsChannelAsync("Stream", 42, 42); var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); - Assert.Equal("Failed to invoke 'Stream'. Invocation provides 2 argument(s) but target expects 1.", ex.Message); + Assert.Equal("Failed to invoke 'Stream' due to an error on the server.", ex.Message); } catch (Exception ex) { @@ -581,7 +581,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests var channel = await connection.StreamAsChannelAsync("Stream", "xyz"); var ex = await Assert.ThrowsAsync(() => channel.ReadAllAsync().OrTimeout()); - Assert.StartsWith("Failed to invoke 'Stream'. Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message); + Assert.Equal("Failed to invoke 'Stream' due to an error on the server.", ex.Message); } catch (Exception ex) { diff --git a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index f198761582..d2367de745 100644 --- a/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -637,11 +637,17 @@ namespace Microsoft.AspNetCore.SignalR.Tests } [Theory] - [InlineData(nameof(MethodHub.MethodThatThrows))] - [InlineData(nameof(MethodHub.MethodThatYieldsFailedTask))] - public async Task HubMethodCanThrowOrYieldFailedTask(string methodName) + [InlineData(nameof(MethodHub.MethodThatThrows), true)] + [InlineData(nameof(MethodHub.MethodThatYieldsFailedTask), false)] + public async Task HubMethodCanThrowOrYieldFailedTask(string methodName, bool detailedErrors) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + { + builder.AddSignalR(options => + { + options.EnableDetailedErrors = detailedErrors; + }); + }); var connectionHandler = serviceProvider.GetService>(); @@ -651,7 +657,14 @@ namespace Microsoft.AspNetCore.SignalR.Tests var message = await client.InvokeAsync(methodName).OrTimeout(); - Assert.Equal($"An unexpected error occurred invoking '{methodName}' on the server. InvalidOperationException: BOOM!", message.Error); + if (detailedErrors) + { + Assert.Equal($"An unexpected error occurred invoking '{methodName}' on the server. InvalidOperationException: BOOM!", message.Error); + } + else + { + Assert.Equal($"An unexpected error occurred invoking '{methodName}' on the server.", message.Error); + } // kill the connection client.Dispose(); @@ -1596,10 +1609,16 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } - [Fact] - public async Task ReceiveCorrectErrorFromStreamThrowing() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReceiveCorrectErrorFromStreamThrowing(bool detailedErrors) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + builder.AddSignalR(options => + { + options.EnableDetailedErrors = detailedErrors; + })); var connectionHandler = serviceProvider.GetService>(); using (var client = new TestClient()) @@ -1613,7 +1632,14 @@ namespace Microsoft.AspNetCore.SignalR.Tests Assert.Equal(1, messages.Count); var completion = messages[0] as CompletionMessage; Assert.NotNull(completion); - Assert.Equal("Exception from observable", completion.Error); + if (detailedErrors) + { + Assert.Equal("An error occurred on the server while streaming results. Exception: Exception from observable", completion.Error); + } + else + { + Assert.Equal("An error occurred on the server while streaming results.", completion.Error); + } client.Dispose(); @@ -2041,10 +2067,18 @@ namespace Microsoft.AspNetCore.SignalR.Tests } } - [Fact] - public async Task ErrorInHubOnConnectSendsCloseMessageWithError() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ErrorInHubOnConnectSendsCloseMessageWithError(bool detailedErrors) { - var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(); + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(builder => + { + builder.AddSignalR(options => + { + options.EnableDetailedErrors = detailedErrors; + }); + }); var connectionHandler = serviceProvider.GetService>(); using (var client = new TestClient(false, new JsonHubProtocol())) @@ -2054,7 +2088,14 @@ namespace Microsoft.AspNetCore.SignalR.Tests var message = await client.ReadAsync().OrTimeout(); var closeMessage = Assert.IsType(message); - Assert.Equal("Connection closed with an error. InvalidOperationException: Hub OnConnected failed.", closeMessage.Error); + if (detailedErrors) + { + Assert.Equal("Connection closed with an error. InvalidOperationException: Hub OnConnected failed.", closeMessage.Error); + } + else + { + Assert.Equal("Connection closed with an error.", closeMessage.Error); + } await connectionHandlerTask.OrTimeout(); }