Skip to content

Drop log level of aborted websocket exception thrown when closing websocket #298

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

Merged
merged 1 commit into from
May 9, 2019

Conversation

mbett7
Copy link
Contributor

@mbett7 mbett7 commented May 3, 2019

The exception can be logged quite frequently when the client wamp channel closes the websocket. Since there is a race condition between the client & router closing their respective ends of the websocket after receiving a GOODBYE message.

@darkl
Copy link
Member

darkl commented May 3, 2019

It would be better to introduce a separate logger for these exceptions, and allow interested users (as yourself) to disable it.

@mbett7
Copy link
Contributor Author

mbett7 commented May 5, 2019

Add a separate logger to WampSharp? I don't quite see how that would help.

Instead of swallowing the exception we could also reduce it's log level (warning is a bit too high for something that can happen to a healthy environment). i.e. something like this?

try
{
    await mWebSocket.CloseAsync(WebSocketCloseStatus.NormalClosure,
                                String.Empty,
                                CancellationToken.None)
                    .ConfigureAwait(false);
}
catch (WebSocketException ex) when (ex.WebSocketErrorCode == WebSocketError.InvalidState)
{
    mLogger.DebugException("Failed closing the websocket as it was already aborted", ex);
}
catch (Exception ex)
{
    mLogger.WarnException("Failed sending a close message to client", ex);
}

@darkl
Copy link
Member

darkl commented May 5, 2019 via email

@mbett7
Copy link
Contributor Author

mbett7 commented May 8, 2019

Right, I see how that would work.

I think changing the log level would be a better solution in this case however. Since it's a warning a user would see in their logs whenever they close a wamp channel and there is no "fix" for it. All the user could do is disable it.

@darkl

This comment has been minimized.

@darkl
Copy link
Member

darkl commented May 8, 2019

Go for it, you have a green light from me.

Elad

@mbett7 mbett7 force-pushed the matt/ignore-close-exception branch from 317832f to 3ec9d2b Compare May 8, 2019 20:56
@mbett7 mbett7 changed the title Do not log websocket aborted exceptions when closing websocket Drop log level of aborted websocket exception thrown when closing websocket May 8, 2019
@darkl darkl merged commit ea9e421 into Code-Sharp:wampv2 May 9, 2019
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

Successfully merging this pull request may close these issues.

2 participants