Improve unexpected server error message to client (#1532)

* Improve unexpected server error message to client

* Separated expected vs unexpected errors in error message. Fixed broken tests

* Fix ts functional tests

client-ts/FunctionalTests/ts/HubConnectionTests.ts

@@ -122,7 +122,7 @@ describe("hubConnection", () => {
             });
 
             it("rethrows an exception from the server when invoking", (done) => {
-                const errorMessage = "An error occurred.";
+                const errorMessage = "An unexpected error occurred invoking 'ThrowException' on the server. InvalidOperationException: An error occurred.";
                 const hubConnection = new HubConnection(TESTHUBENDPOINT_URL, {
                     logger: LogLevel.Trace,
                     protocol,
@@ -130,7 +130,7 @@ describe("hubConnection", () => {
                 });
 
                 hubConnection.start().then(() => {
-                    hubConnection.invoke("ThrowException", errorMessage).then(() => {
+                    hubConnection.invoke("ThrowException", "An error occurred.").then(() => {
                         // exception expected but none thrown
                         fail();
                     }).catch((e) => {
@@ -195,7 +195,7 @@ describe("hubConnection", () => {
             });
 
             it("rethrows an exception from the server when streaming", (done) => {
-                const errorMessage = "An error occurred.";
+                const errorMessage = "An unexpected error occurred invoking 'StreamThrowException' on the server. InvalidOperationException: An error occurred.";
                 const hubConnection = new HubConnection(TESTHUBENDPOINT_URL, {
                     logger: LogLevel.Trace,
                     protocol,
@@ -203,13 +203,13 @@ describe("hubConnection", () => {
                 });
 
                 hubConnection.start().then(() => {
-                    hubConnection.stream("StreamThrowException", errorMessage).subscribe({
+                    hubConnection.stream("StreamThrowException", "An error occurred.").subscribe({
                         complete: function complete() {
                             hubConnection.stop();
                             fail();
                         },
                         error: function error(err) {
-                            expect(err.message).toEqual("An error occurred.");
+                            expect(err.message).toEqual(errorMessage);
                             hubConnection.stop();
                             done();
                         },

src/Microsoft.AspNetCore.SignalR.Core/Internal/DefaultHubDispatcher.cs

@@ -170,6 +170,13 @@ namespace Microsoft.AspNetCore.SignalR.Internal
                     return;
                 }
 
+                if (hubMethodInvocationMessage.ArgumentBindingException != null)
+                {
+                    Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, hubMethodInvocationMessage.ArgumentBindingException);
+                    await SendInvocationError(hubMethodInvocationMessage, connection, $"Failed to invoke '{hubMethodInvocationMessage.Target}'. {hubMethodInvocationMessage.ArgumentBindingException.Message}");
+                    return;
+                }
+
                 var hubActivator = scope.ServiceProvider.GetRequiredService<IHubActivator<THub>>();
                 var hub = hubActivator.Create();
 
@@ -181,7 +188,15 @@ namespace Microsoft.AspNetCore.SignalR.Internal
 
                     if (isStreamedInvocation)
                     {
-                        var enumerator = GetStreamingEnumerator(connection, hubMethodInvocationMessage.InvocationId, methodExecutor, result, methodExecutor.MethodReturnType);
+                        if (!TryGetStreamingEnumerator(connection, hubMethodInvocationMessage.InvocationId, methodExecutor, result, methodExecutor.MethodReturnType, out var enumerator))
+                        {
+                            Log.InvalidReturnValueFromStreamingMethod(_logger, methodExecutor.MethodInfo.Name);
+
+                            await SendInvocationError(hubMethodInvocationMessage, connection,
+                                $"The value returned by the streaming method '{methodExecutor.MethodInfo.Name}' is null, does not implement the IObservable<> interface or is not a ReadableChannel<>.");
+                            return;
+                        }
+
                         Log.StreamingResult(_logger, hubMethodInvocationMessage.InvocationId, methodExecutor);
                         await StreamResultsAsync(hubMethodInvocationMessage.InvocationId, connection, enumerator);
                     }
@@ -195,12 +210,12 @@ namespace Microsoft.AspNetCore.SignalR.Internal
                 catch (TargetInvocationException ex)
                 {
                     Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, ex);
-                    await SendInvocationError(hubMethodInvocationMessage, connection, ex.InnerException.Message);
+                    await SendInvocationError(hubMethodInvocationMessage, connection, BuildUnexpectedErrorMessage(hubMethodInvocationMessage.Target, ex.InnerException));
                 }
                 catch (Exception ex)
                 {
                     Log.FailedInvokingHubMethod(_logger, hubMethodInvocationMessage.Target, ex);
-                    await SendInvocationError(hubMethodInvocationMessage, connection, ex.Message);
+                    await SendInvocationError(hubMethodInvocationMessage, connection, BuildUnexpectedErrorMessage(hubMethodInvocationMessage.Target, ex));
                 }
                 finally
                 {
@@ -209,6 +224,11 @@ 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;
@@ -369,7 +389,7 @@ namespace Microsoft.AspNetCore.SignalR.Internal
             return false;
         }
 
-        private IAsyncEnumerator<object> GetStreamingEnumerator(HubConnectionContext connection, string invocationId, ObjectMethodExecutor methodExecutor, object result, Type resultType)
+        private bool TryGetStreamingEnumerator(HubConnectionContext connection, string invocationId, ObjectMethodExecutor methodExecutor, object result, Type resultType, out IAsyncEnumerator<object> enumerator)
         {
             if (result != null)
             {
@@ -378,17 +398,19 @@ namespace Microsoft.AspNetCore.SignalR.Internal
                     resultType.GetInterfaces().FirstOrDefault(IsIObservable);
                 if (observableInterface != null)
                 {
-                    return AsyncEnumeratorAdapters.FromObservable(result, observableInterface, CreateCancellation());
+                    enumerator = AsyncEnumeratorAdapters.FromObservable(result, observableInterface, CreateCancellation());
+                    return true;
                 }
 
                 if (IsChannel(resultType, out var payloadType))
                 {
-                    return AsyncEnumeratorAdapters.FromChannel(result, payloadType, CreateCancellation());
+                    enumerator = AsyncEnumeratorAdapters.FromChannel(result, payloadType, CreateCancellation());
+                    return true;
                 }
             }
 
-            Log.InvalidReturnValueFromStreamingMethod(_logger, methodExecutor.MethodInfo.Name);
+            enumerator = null;
-            throw new InvalidOperationException($"The value returned by the streaming method '{methodExecutor.MethodInfo.Name}' is null, does not implement the IObservable<> interface or is not a ReadableChannel<>.");
+            return false;
 
             CancellationToken CreateCancellation()
             {

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

@@ -397,7 +397,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
                     var channel = await connection.StreamAsync<int>("StreamException").OrTimeout();
 
                     var ex = await Assert.ThrowsAsync<HubException>(() => channel.ReadAllAsync().OrTimeout());
-                    Assert.Equal("Error occurred while streaming.", ex.Message);
+                    Assert.Equal("An unexpected error occurred invoking 'StreamException' on the server. InvalidOperationException: Error occurred while streaming.", ex.Message);
                 }
                 catch (Exception ex)
                 {
@@ -451,7 +451,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("Invocation provides 2 argument(s) but target expects 1.", ex.Message);
+                    Assert.Equal("Failed to invoke 'Echo'. Invocation provides 2 argument(s) but target expects 1.", ex.Message);
                 }
                 catch (Exception ex)
                 {
@@ -478,7 +478,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("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'. Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message);
                 }
                 catch (Exception ex)
                 {
@@ -535,7 +535,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
 
                     var channel = await connection.StreamAsync<int>("Stream", 42, 42);
                     var ex = await Assert.ThrowsAsync<HubException>(() => channel.ReadAllAsync().OrTimeout());
-                    Assert.Equal("Invocation provides 2 argument(s) but target expects 1.", ex.Message);
+                    Assert.Equal("Failed to invoke 'Stream'. Invocation provides 2 argument(s) but target expects 1.", ex.Message);
                 }
                 catch (Exception ex)
                 {
@@ -563,7 +563,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests
 
                     var channel = await connection.StreamAsync<int>("Stream", "xyz");
                     var ex = await Assert.ThrowsAsync<HubException>(() => channel.ReadAllAsync().OrTimeout());
-                    Assert.StartsWith("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 'Stream'. Error binding arguments. Make sure that the types of the provided values match the types of the hub method being invoked.", ex.Message);
                 }
                 catch (Exception ex)
                 {

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

@@ -449,7 +449,7 @@ namespace Microsoft.AspNetCore.SignalR.Tests
 
                 var result = (await client.InvokeAsync(methodName).OrTimeout());
 
-                Assert.Equal("BOOM!", result.Error);
+                Assert.Equal($"An unexpected error occurred invoking '{methodName}' on the server. InvalidOperationException: BOOM!", result.Error);
 
                 // kill the connection
                 client.Dispose();