Skip to content

Commit

Permalink
downgrade inner httpconnection error to warning (#482)
Browse files Browse the repository at this point in the history
* downgrade inner httpconnection error to warning, improve exception message

* Add comments
  • Loading branch information
vicancy authored Apr 18, 2019
1 parent 149b696 commit c05e752
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Http.Connections;
using Microsoft.AspNetCore.Http.Connections.Client;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;

namespace Microsoft.Azure.SignalR
{
Expand All @@ -27,7 +28,7 @@ internal class ConnectionFactory : IConnectionFactory
public ConnectionFactory(string hubName, IServiceEndpointProvider provider, IServerNameProvider nameProvider, ILoggerFactory loggerFactory)
{
_provider = provider ?? throw new ArgumentNullException(nameof(provider));
_loggerFactory = loggerFactory;
_loggerFactory = loggerFactory == null ? (ILoggerFactory)NullLoggerFactory.Instance : new GracefulLoggerFactory(loggerFactory);
_userId = nameProvider?.GetName();
_hubName = hubName;
}
Expand Down Expand Up @@ -83,5 +84,68 @@ public Task DisposeAsync(ConnectionContext connection)

return ((HttpConnection)connection).DisposeAsync();
}

private sealed class GracefulLoggerFactory : ILoggerFactory
{
private readonly ILoggerFactory _inner;
public GracefulLoggerFactory(ILoggerFactory inner)
{
_inner = inner;
}

public void Dispose()
{
_inner.Dispose();
}

public ILogger CreateLogger(string categoryName)
{
var innerLogger = _inner.CreateLogger(categoryName);
return new GracefulLogger(innerLogger);
}

public void AddProvider(ILoggerProvider provider)
{
_inner.AddProvider(provider);
}

private sealed class GracefulLogger : ILogger
{
private readonly ILogger _inner;
public GracefulLogger(ILogger inner)
{
_inner = inner;
}

/// <summary>
/// Downgrade error level logs, and also exclude exception details
/// Exceptions thrown from inside the HttpConnection are supposed to be handled by the caller and logged with more user-friendly message
/// </summary>
/// <typeparam name="TState"></typeparam>
/// <param name="logLevel"></param>
/// <param name="eventId"></param>
/// <param name="state"></param>
/// <param name="exception"></param>
/// <param name="formatter"></param>
public void Log<TState>(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
{
if (logLevel >= LogLevel.Error)
{
logLevel = LogLevel.Warning;
}
_inner.Log(logLevel, eventId, state, null, formatter);
}

public bool IsEnabled(LogLevel logLevel)
{
return _inner.IsEnabled(logLevel);
}

public IDisposable BeginScope<TState>(TState state)
{
return _inner.BeginScope(state);
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -525,17 +525,17 @@ private void AbortConnection()
private static class Log
{
// Category: ServiceConnection
private static readonly Action<ILogger, Exception> _failedToWrite =
LoggerMessage.Define(LogLevel.Error, new EventId(1, "FailedToWrite"), "Failed to send message to the service.");
private static readonly Action<ILogger, string, Exception> _failedToWrite =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(1, "FailedToWrite"), "Failed to send message to the service: {message}");

private static readonly Action<ILogger, string, Exception> _failedToConnect =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(2, "FailedToConnect"), "Failed to connect to the service: {message}.");
LoggerMessage.Define<string>(LogLevel.Error, new EventId(2, "FailedToConnect"), "Failed to connect to the service, will retry after the back off period. Error detail: {message}.");

private static readonly Action<ILogger, Exception> _errorProcessingMessages =
LoggerMessage.Define(LogLevel.Error, new EventId(3, "ErrorProcessingMessages"), "Error when processing messages.");

private static readonly Action<ILogger, string, Exception> _connectionDropped =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(4, "ConnectionDropped"), "Connection {ServiceConnectionId} to the service was dropped.");
LoggerMessage.Define<string>(LogLevel.Error, new EventId(4, "ConnectionDropped"), "Connection to the service was dropped, probably caused by network instability or service restart. Will try to reconnect after the back off period. Id: {ServiceConnectionId}");

private static readonly Action<ILogger, Exception> _failedToCleanupConnections =
LoggerMessage.Define(LogLevel.Error, new EventId(5, "FailedToCleanupConnection"), "Failed to clean up client connections.");
Expand Down Expand Up @@ -617,19 +617,16 @@ private static class Log

public static void FailedToWrite(ILogger logger, Exception exception)
{
_failedToWrite(logger, exception);
_failedToWrite(logger, exception.Message, null);
}

public static void FailedToConnect(ILogger logger, Exception exception)
{
var message = exception.Message;
var baseException = exception.GetBaseException();
if (baseException != null)
{
message += " " + baseException.Message;
}
message += ". " + baseException.Message;

_failedToConnect(logger, message, exception);
_failedToConnect(logger, message, null);
}

public static void ErrorProcessingMessages(ILogger logger, Exception exception)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ private static class Log
private static readonly Action<ILogger, Exception> _failedToCleanupConnections =
LoggerMessage.Define(LogLevel.Error, new EventId(5, "FailedToCleanupConnection"), "Failed to clean up client connections.");

private static readonly Action<ILogger, Exception> _errorSendingMessage =
LoggerMessage.Define(LogLevel.Error, new EventId(6, "ErrorSendingMessage"), "Error while sending message to the service.");
private static readonly Action<ILogger, string, Exception> _errorSendingMessage =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(6, "ErrorSendingMessage"), "Error while sending message to the service, the connection carrying the traffic is dropped. Error detail: {message}");

private static readonly Action<ILogger, string, Exception> _sendLoopStopped =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(7, "SendLoopStopped"), "Error while processing messages from {TransportConnectionId}.");
Expand Down Expand Up @@ -51,7 +51,7 @@ public static void FailedToCleanupConnections(ILogger logger, Exception exceptio

public static void ErrorSendingMessage(ILogger logger, Exception exception)
{
_errorSendingMessage(logger, exception);
_errorSendingMessage(logger, exception.Message, exception);
}

public static void SendLoopStopped(ILogger logger, string connectionId, Exception exception)
Expand Down

0 comments on commit c05e752

Please sign in to comment.