🕵️ Review loglevel of certain Traces

[fgheysels]

The MessageHandler class catches exceptions that have been thrown by implementations of the MessageHandler and logs these exceptions as errors.

See this piece of code, extracted from the MessageHandler class (ok, the snippet is from a feature branch so it could differ a bit from main):

try
{
    Task<bool> processMessageAsync = 
        _messageHandlerImplementation(message, messageContext, correlationInfo, cancellationToken);
    
    if (processMessageAsync is null)
    {
        throw new InvalidOperationException(
            $"The '{typeof(IMessageHandler<,>).Name}' implementation '{_messageHandlerInstanceType.Name}' returned 'null' while calling the '{methodName}' method");
    }

    bool isProcessed = await processMessageAsync;
    _logger.LogTrace("Message handler '{MessageHandlerType}' successfully processed '{MessageType}' message", _messageHandlerInstanceType.Name, messageType.Name);

    return isProcessed;
}
catch (AmbiguousMatchException exception)
{
    _logger.LogError(exception, "Ambiguous match found of '{MethodName}' methods in the '{MessageHandlerType}'. Make sure that only 1 matching '{MethodName}' was found on the '{MessageHandlerType}' message handler", methodName, _messageHandlerInstanceType.Name, methodName, _messageHandlerInstanceType.Name);
    return false;
}
catch (Exception exception)
{
    _logger.LogError(exception, "Message handler '{MessageHandlerType}' failed to process '{MessageType}' due to a thrown exception", _messageHandlerInstanceType.Name, messageType.Name);
    return false;
}

I think it is OK to log the AmbiguousMatchException occurences as error traces, as this indicates something that is wrong in Arcus / setup of your MessageHandlers.

However, I'm wondering we really should log the other exceptions as 'errors'. If they're thrown by the implementation of the MessageHandler, I'd say that it's the responsability of the developer who implemented the MessageHandler to log.
At least, I think that, if we log this here as well, we should consider if logging it as an 'Error' trace is a good idea. In an application that uses Arcus, I'd like to see as less as possible log-traces that originate from Arcus. These traces surely help when developing / debugging / testing, but in production, I'd like to avoid that.
Therefore, we should maybe consider to log this trace at an Information level ( or even a Debug level ?).

[fgheysels]

In hindsight, it might be good to leave it as it is, and put the responsability with the developer / user of Arcus.
Application code can catch, handle and rethrow an exception. When it's rethrown, the 'outer caller' should log it.

See also this thread: https://softwareengineering.stackexchange.com/questions/365427/try-catch-log-rethrow-is-anti-pattern