Skip to content

Commit

Permalink
Enhance WebSocket exception handling and logging
Browse files Browse the repository at this point in the history
This change enhances the robustness of the WebSocket handling by
ensuring that exceptions are properly logged and do not cause
the service to crash.

- Exceptions/Faults Task.Run which handles a websocket
    connection are now caught and logged. The connection is
    closed with an internal server error status.

- Service exceptions are caught and logged. The connection is
    closed with an internal server error status.

- Exception logging added to empty catch blocks.

- Exceptions thrown while handling a websocket connection now
    result in the connection being closed with an internal
    server error status.

- The following exceptions are now logged (or improved):
 - HTTP Listener failures.
 - Unauthorized (by ACL) connections.
 - Non-websocket requests.
 - Connection timeouts.
 - Connection acceptance errors.
 - Invalid websocket paths.
 - Exceptions thrown while starting the server.
 - Exceptions thrown while stopping the server.
  • Loading branch information
thesprockee committed Jan 27, 2024
1 parent 3dfe70e commit ff0f881
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 13 deletions.
6 changes: 3 additions & 3 deletions EchoRelay.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ await Parser.Default.ParseArguments<CliOptions>(args).WithParsedAsync(async opti
// Start the server.
await Server.Start();
}
catch (System.Net.HttpListenerException ex)
catch (Exception ex)
{
Log.Warning("Exception encountered: {}", ex.Message);
Log.Fatal("HTTP Server encountered fatal exception: {Exception}", ex.ToString());

}
});
}
Expand Down
52 changes: 42 additions & 10 deletions EchoRelay.Core/Server/Server.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public async Task Start(CancellationTokenSource? cancellationTokenSource = null)
listenerContext.Response.StatusCode = (int)HttpStatusCode.BadRequest;
listenerContext.Response.Close();

// TODO: Log the interaction.
Log.Warning("Received a non-websocket request from {RemoteEndPoint}", listenerContext.Request.RemoteEndPoint);

// Do not accept this client.
continue;
Expand All @@ -240,7 +240,7 @@ public async Task Start(CancellationTokenSource? cancellationTokenSource = null)
OnAuthorizationResult?.Invoke(this, listenerContext.Request.RemoteEndPoint, authorized);
if (!authorized)
{
// TODO: Log the interaction.
Log.Warning("Received a connection from {RemoteEndPoint} which was not authorized by the ACL.", listenerContext.Request.RemoteEndPoint);

// Do not accept this client.
continue;
Expand All @@ -258,7 +258,7 @@ public async Task Start(CancellationTokenSource? cancellationTokenSource = null)
listenerContext.Response.StatusCode = (int)HttpStatusCode.InternalServerError;
listenerContext.Response.Close();

// TODO: Log the exception.
Log.Error(e, "Failed to accept a websocket connection from {RemoteEndPoint}", listenerContext.Request.RemoteEndPoint);
continue;
}

Expand All @@ -269,29 +269,61 @@ public async Task Start(CancellationTokenSource? cancellationTokenSource = null)
listenerContext.Response.StatusCode = (int)HttpStatusCode.NotFound;
listenerContext.Response.Close();

// TODO: Log the interaction.
Log.Warning("Received a connection from {RemoteEndPoint} which requested an invalid path: {Path}", listenerContext.Request.RemoteEndPoint, listenerContext.Request.Url?.LocalPath);
continue;
}

// Create a task to handle the new connection asynchronously (we do not await, so we can continue accepting connections).
var handleConnectionTask = Task.Run(() => service.HandleConnection(listenerContext, webSocketContext.WebSocket));
var handleConnectionTask = Task.Run(() => service.HandleConnection(listenerContext, webSocketContext.WebSocket))
.ContinueWith(t =>
{
if (t.IsFaulted)
{
Exception ex = t.Exception ?? new Exception("Unknown exception");
if (ex is AggregateException)
{
ex = ex.InnerException ?? ex;
}
if (ex is WebSocketException)
{
Log.Warning("An error occurred while handling a connection from {RemoteEndPoint}: {Exception}", listenerContext.Request.RemoteEndPoint, ex.Message.ToString());
}
else
{
Log.Warning(ex, "An error occurred while handling a connection from {RemoteEndPoint}: {Exception}", listenerContext.Request.RemoteEndPoint, ex.Message.ToString());
}
}
});
}
}
catch (TimeoutException)
{
Log.Debug("Server task timed out.");
}
catch (TaskCanceledException)
{
Log.Debug("Server task was cancelled.");
}
catch (Exception e)
{
Log.Error(e, "Server task encountered an exception: {Exception}", e.Message.ToString());
}

// Ensure our listener is closed
listener.Close();
try
{
listener.Close();

// Set our state to not running.
Running = false;
// Set our state to not running.
Running = false;

// Fire our stopped event
OnServerStopped?.Invoke(this);
// Fire our stopped event
OnServerStopped?.Invoke(this);
}
catch (Exception e)
{
Log.Error(e, "An error occurred while stopping the server: {Exception}", e.Message.ToString());
}
}

/// <summary>
Expand Down
7 changes: 7 additions & 0 deletions EchoRelay.Core/Server/Services/Service.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ internal async Task HandleConnection(HttpListenerContext context, WebSocket webS
}
}
}
catch (Exception e) {
// Close the connection with an internal server error status.
try {
await webSocket.CloseAsync(WebSocketCloseStatus.InternalServerError, "", CancellationToken.None);
} catch {}
Log.Warning("An exception occurred while handling a websocket connection: {0}", e);
}
finally
{
// Whether we exit gracefully or encounter an exception, remove the peer from our list of peers.
Expand Down

0 comments on commit ff0f881

Please sign in to comment.