Merge pull request #1785 from aspnet/release/2.1

Add detailed error option (#1763)

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<TestHub>(
                 serviceScopeFactory,
                 new HubContext<TestHub>(new DefaultHubLifetimeManager<TestHub>(NullLogger<DefaultHubLifetimeManager<TestHub>>.Instance)),
+                Options.Create(new HubOptions<TestHub>()),
+                Options.Create(new HubOptions()),
                 new Logger<DefaultHubDispatcher<TestHub>>(NullLoggerFactory.Instance));
 
             var pair = DuplexPipe.CreateConnectionPair(PipeOptions.Default, PipeOptions.Default);

benchmarks/Microsoft.AspNetCore.SignalR.Microbenchmarks/HubConnectionContextBenchmark.cs

@@ -56,7 +56,8 @@ namespace Microsoft.AspNetCore.SignalR.Microbenchmarks
         {
             _pipe.AddReadResult(new ValueTask<ReadResult>(_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<ReadResult>(_handshakeRequestResult));
 
-            await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _failureHubProtocolResolver, _userIdProvider);
+            await _hubConnectionContext.HandshakeAsync(TimeSpan.FromSeconds(5), _supportedProtocols, _failureHubProtocolResolver,
+                _userIdProvider, enableDetailedErrors: true);
         }
     }
 

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();

src/Microsoft.AspNetCore.SignalR.Core/HubConnectionContext.cs

@@ -225,7 +225,8 @@ namespace Microsoft.AspNetCore.SignalR
             Task.Factory.StartNew(_abortedCallback, this);
         }
 
-        internal async Task<bool> HandshakeAsync(TimeSpan timeout, IList<string> supportedProtocols, IHubProtocolResolver protocolResolver, IUserIdProvider userIdProvider)
+        internal async Task<bool> HandshakeAsync(TimeSpan timeout, IList<string> 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;
             }
         }

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<THub> _dispatcher;
+        private readonly bool _enableDetailedErrors;
 
         public HubConnectionHandler(HubLifetimeManager<THub> lifetimeManager,
                                     IHubProtocolResolver protocolResolver,
@@ -42,6 +40,8 @@ namespace Microsoft.AspNetCore.SignalR
             _logger = loggerFactory.CreateLogger<HubConnectionHandler<THub>>();
             _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
+            var closeMessage = CloseMessage.Empty;
-                ? CloseMessage.Empty
+
-                : new CloseMessage($"Connection closed with an error. {exception.GetType().Name}: {exception.Message}");
+            if (exception != null)
+            {
+                var errorMessage = ErrorMessageHelper.BuildErrorMessage("Connection closed with an error.", exception, _enableDetailedErrors);
+                closeMessage = new CloseMessage(errorMessage);
+            }
 
             try
             {

src/Microsoft.AspNetCore.SignalR.Core/HubOptions.cs

@@ -17,5 +17,7 @@ namespace Microsoft.AspNetCore.SignalR
         public TimeSpan? KeepAliveInterval { get; set; } = null;
 
         public IList<string> SupportedProtocols { get; set; } = null;
+
+        public bool? EnableDetailedErrors { get; set; } = null;
     }
 }

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;
 

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<THub> _hubContext;
         private readonly ILogger<HubDispatcher<THub>> _logger;
+        private readonly bool _enableDetailedErrors;
 
-        public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext<THub> hubContext, ILogger<DefaultHubDispatcher<THub>> logger)
+        public DefaultHubDispatcher(IServiceScopeFactory serviceScopeFactory, IHubContext<THub> hubContext, IOptions<HubOptions<THub>> hubOptions,
+            IOptions<HubOptions> globalHubOptions, ILogger<DefaultHubDispatcher<THub>> 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<object> 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

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}";
+        }
+    }
+}

test/Microsoft.AspNetCore.SignalR.Client.FunctionalTests/HubConnectionTests.cs

@@ -415,7 +415,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
                     var channel = await connection.StreamAsChannelAsync<int>("StreamException").OrTimeout();
 
                     var ex = await Assert.ThrowsAsync<HubException>(() => 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<HubException>(() => 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<HubException>(() => 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<int>("Stream", 42, 42);
                     var ex = await Assert.ThrowsAsync<HubException>(() => 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<int>("Stream", "xyz");
                     var ex = await Assert.ThrowsAsync<HubException>(() => 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)
                 {

test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs

@@ -637,11 +637,17 @@ namespace Microsoft.AspNetCore.SignalR.Tests
         }
 
         [Theory]
-        [InlineData(nameof(MethodHub.MethodThatThrows))]
+        [InlineData(nameof(MethodHub.MethodThatThrows), true)]
-        [InlineData(nameof(MethodHub.MethodThatYieldsFailedTask))]
+        [InlineData(nameof(MethodHub.MethodThatYieldsFailedTask), false)]
-        public async Task HubMethodCanThrowOrYieldFailedTask(string methodName)
+        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<HubConnectionHandler<MethodHub>>();
 
@@ -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]
+        [Theory]
-        public async Task ReceiveCorrectErrorFromStreamThrowing()
+        [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<HubConnectionHandler<StreamingHub>>();
 
             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]
+        [Theory]
-        public async Task ErrorInHubOnConnectSendsCloseMessageWithError()
+        [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<HubConnectionHandler<OnConnectedThrowsHub>>();
 
             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<CloseMessage>(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();
             }