Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhandled exception in opened handler (SocketIO v2.3.1) #239

Open
23W opened this issue Nov 19, 2021 · 8 comments
Open

Unhandled exception in opened handler (SocketIO v2.3.1) #239

23W opened this issue Nov 19, 2021 · 8 comments

Comments

@23W
Copy link
Contributor

23W commented Nov 19, 2021

Hi @doghappy
Customer sent screenshot with unhandled exception.
image
and brief exception text part:

************** Exception Text **************
SocketIOClient.Exceptions.InvalidSocketStateException: Connection is not open.
at SocketIOClient.WebSocketClient.ClientWebSocket.<SendMessageAsync>d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at SocketIOClient.WebSocketClient.ClientWebSocket.<SendMessageAsync>d__17.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at SocketIOClient.SocketIO.<OpenedHandler>d__105.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()

Seems exception occurs in OpenedHandler method at attempt to execute await Socket.SendMessageAsync(msg);

        private async void OpenedHandler(string sid, int pingInterval, int pingTimeout)
        {
            Id = sid;
            _pingInterval = pingInterval;
            string msg = Options.EioHandler.CreateConnectionMessage(Namespace, Options.Query);
            await Socket.SendMessageAsync(msg);
        }

Exception was not caught in OpenedHandler() or in OpenedProcessor.Process, which called this method as delegate.
Can you please give recommendation how this situation is possible and how to fix code in 2.3.1 (this lib version is used in production and should be fixed).

@doghappy
Copy link
Owner

doghappy commented Nov 22, 2021

Seems exception occurs in OpenedHandler method at attempt to execute await Socket.SendMessageAsync(msg);

If the exception is indeed thrown here, it means that the WebSocket was disconnected shortly after the connection was established, causing some information of socket.io to not be sent.

If this problem occurs by accident, can it be fixed like this?

        private async void OpenedHandler(string sid, int pingInterval, int pingTimeout)
        {
            Id = sid;
            _pingInterval = pingInterval;
            string msg = Options.EioHandler.CreateConnectionMessage(Namespace, Options.Query);
            try
            {
                throw new Exception("TEST Exception");
                // await Socket.SendMessageAsync(msg).ConfigureAwait(false);
            }
            catch (Exception e)
            {
                ErrorHandler(e.Message);
                await ConnectAsync().ConfigureAwait(false);
            }
        }

btw, I will consider adding a better solution to the latest version.

@23W
Copy link
Contributor Author

23W commented Nov 22, 2021

@doghappy
Thank you for the advise, your suggestion seems works well. We age going to update our local fork of your library to fix the issue.

@doghappy
Copy link
Owner

Thank you for the advise, your suggestion seems works well. We age going to update our local fork of your library to fix the issue.

I suggest you based on this branch https://github.com/doghappy/socket.io-client-csharp/tree/v2.x

@23W
Copy link
Contributor Author

23W commented Nov 22, 2021

Thank you for the advise, your suggestion seems works well. We age going to update our local fork of your library to fix the issue.

I suggest you based on this branch https://github.com/doghappy/socket.io-client-csharp/tree/v2.x

Do you mean to make a PR in v2.x branch with this fix?

@doghappy
Copy link
Owner

no.

I suggest you based on this branch https://github.com/doghappy/socket.io-client-csharp/tree/v2.x

I suggest you base on this branch https://github.com/doghappy/socket.io-client-csharp/tree/v2.x instead of downloading from the release page.

It is better if you are willing to submit a PR :)


btw, I would rather find the real reason, maybe it is the network reason, or something else? Do you have any suggestions?

In v3.x, I added a retry mechanism, I don't know which way is better.

@DmitryBk
Copy link
Contributor

DmitryBk commented Nov 22, 2021

Of course, it is a rare case but I would suggest starting standard reconnection flow for this case

private async void OpenedHandler(string sid, int pingInterval, int pingTimeout)
{
    try
    {
        Id = sid;
        _pingInterval = pingInterval;
        string msg = Options.EioHandler.CreateConnectionMessage(Namespace, Options.Query);
        await Socket.SendMessageAsync(msg).ConfigureAwait(false);
    }
    catch (Exception e)
    {
        ErrorHandler(e.Message);

        Attempts++;
        await InvokeDisconnectAsync(e.Message).ConfigureAwait(false);
    }
}

@doghappy
Copy link
Owner

doghappy commented Nov 22, 2021

There is an if condition in InvokeDisconnectAsync

private async Task InvokeDisconnectAsync(string reason)
{
if (Connected)
{
Connected = false;
Disconnected = true;
if (Options.EIO == 3)
{
_pingTokenSorce.Cancel();
}
OnDisconnected?.Invoke(this, reason);
if (reason != "io server disconnect" && reason != "io client disconnect")
{
//In the this cases (explicit disconnection), the client will not try to reconnect and you need to manually call socket.connect().
if (Options.Reconnection)
{
await ConnectAsync().ConfigureAwait(false);
}
}
}
}

When an exception is thrown in OpenedHandler, the value of Connected is false

At this point, WS has been established, but socket.io has not been established yet. The data sent by OpenedHandler is to establish socket.io connection on the basis of WS.

@DmitryBk
Copy link
Contributor

DmitryBk commented Nov 22, 2021

It works for me with throw new Exception("TEST Exception");
It seems the OpenedHandler is called after the connection is established and after ConnectedHandler which set Connected = true

Sorry I was wrong, Connected is really false in that moment.
My new suggestion just increments Attempts that OnReconnected event will be faired.

catch (Exception e)
{
    ErrorHandler(e.Message);

    Attempts++;
    await ConnectAsync().ConfigureAwait(false);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants