Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

How to indicate errors with ChannelReader<T> ? #2609

Closed
Kikimora opened this issue Jul 9, 2018 · 7 comments
Closed

How to indicate errors with ChannelReader<T> ? #2609

Kikimora opened this issue Jul 9, 2018 · 7 comments

Comments

@Kikimora
Copy link

Kikimora commented Jul 9, 2018

SignalR does not report errors correctly with streaming methods. My code below throws error in 'GetOrAddObservable' with some message. I receive exception on client but it does not have my message, instead it has some generic error message like 'something went wrong on server'.

public ChannelReader<ArraySegment<TradeInfo>> Trades(string instrument)
{
   return _tradeData.GetOrAddObservable(instrument).AsChannelReader();
}

So I tried this instead with same result.

        public ChannelReader<ArraySegment<TradeInfo>> Trades(string instrument)
        {
            try
            {
                return _tradeData.GetOrAddObservable(instrument).AsChannelReader();
            }
            catch (Exception e)
            {
                var cr = Channel.CreateBounded<ArraySegment<TradeInfo>>(1);
                var completed = cr.Writer.TryComplete(new ErrorException(new Error("instrument not found")));
                return cr;
            }
        }

This is my client side code which receives this error.

        public static IObservable<T> AsObservable<T>(this ChannelReader<T> reader)
        {
            var subject = new Subject<T>();
            Task.Run(async () =>
            {
                try
                {
                    Log.Debug("Waiting for WebSocket data");
                    while (await reader.WaitToReadAsync())
                    {
                        Log.Debug("Got WebSocket data");
                        while (reader.TryRead(out var item))
                        {
                            Log.Debug("Read item from WebSocket {item}", item);
                            subject.OnNext(item);
                        }
                    }
                }
                catch (Exception e)
                {
                    Log.Error(e, "WebSocket read failed");
                    subject.OnError(e); //<-- I'd like to have more meaningful error message here
                    throw;
                }
                finally
                {
                    subject.OnCompleted();
                }
            });
            return subject;
        }

AsChannelReader copied from here - https://github.com/aspnet/SignalR/blob/master/samples/SignalRSamples/ObservableExtensions.cs

  • Versions of Server-Side NuGet Packages: 1.0.1
  • Versions of Client-Side NuGet/NPM Packages: 1.0.1
  • Are you using the C# client or the JavaScript client: C#
  • The Server you are using (Kestrel/HttpSysServer/IIS/IIS Express/Azure Web App/etc.): TestServer
  • The Operating System on the Server (Windows/Linux/macOS): macOS
  • The Operating System on the Client (Windows/Linux/macOS): macOS
@BrennanConroy BrennanConroy added this to the Discussions milestone Jul 9, 2018
@BrennanConroy
Copy link
Member

By default SignalR will not send the exception message to clients because they can contain sensitive information. You can change this behavior by setting EnableDetailedErrors to true in the server configuration, see https://docs.microsoft.com/en-us/aspnet/core/signalr/configuration?view=aspnetcore-2.1#configure-server-options

In 2.2 we have enabled apps to throw a HubException to allow the apps to send an actual error message to clients without globally setting EnableDetailedErrors.

@Kikimora
Copy link
Author

Kikimora commented Jul 9, 2018

@BrennanConroy Error message is lost even with HubException. Please see below code and results.

This is my server code

        public ChannelReader<ArraySegment<TradeInfo>> Trades(string instrument)
        {
            try
            {
                return _tradeData.GetOrAddObservable(instrument).AsChannelReader();
            }
            catch (Exception e)
            {
                throw new HubException(e.Message);
            }
        }

This is what I see on server
[20:27:59.356496 125 0HLF5O4VPJK63 ERR]
Microsoft.AspNetCore.SignalR.Internal.DefaultHubDispatcher Failed to invoke hub method 'Trades'.
Microsoft.AspNetCore.SignalR.HubException: instrument not found
at MyExchange.MarketData.MarketInfoHub.Trades(String instrument) in /Users/averbin/Projects/tradeio/api/exchange/MarketData.WS/src/MarketInfoHub.cs:line 60

And this is what I got on client
[20:25:43.796629 060 ERR]
Microsoft.AspNetCore.SignalR.HubException: An unexpected error occurred invoking 'Trades' on the server.
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

@BrennanConroy
Copy link
Member

Yeah, like I said, it's fixed in 2.2 (SignalR 1.1), you're using 1.0.1 so you won't have the change.

@Kikimora
Copy link
Author

Kikimora commented Jul 9, 2018

Any plans to release SignalR 1.1 as pre release nuget package?

@BrennanConroy
Copy link
Member

@DylanDmitri
Copy link
Contributor

DylanDmitri commented Jul 10, 2018

I was confused by these generic errors a couple of days ago, and had to search a bit to find the EnableDetailedErrors field. Maybe it would be useful for the generic responses to have an instructional snippet, like "For more information, set EnableDetailedErrors on the server."?

@aspnet-hello
Copy link

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.

@aspnet-hello aspnet-hello removed this from the Discussions milestone Sep 24, 2018
@aspnet aspnet locked and limited conversation to collaborators Sep 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants