-
Notifications
You must be signed in to change notification settings - Fork 606
Description
Describe the bug
When a connection exception happens, we log a message to the RabbitMqClientEventSource
. However, the full exception ToString is being logged in the Message
of the EventSource event.
Reproduction steps
- Add an
EventListener
to therabbitmq-client
EventSource. - Create a RabbitMQ connection to a server.
- Stop the server to generate an exception
- Observe the EventSource Error event data
This can easily be done with a .NET Aspire app that uses the RabbitMQ component and looking in the Structured Logs entry for the exception.
Expected behavior
The Message should only contain the message information. Not the StackTrace.
Additional context
I believe the issue is isolated to the HandleMainLoopException
method. The other places that log exceptions all appear to pass the caught exception into the logging correctly.
In HandleMainLoopException
, it puts the ShutdownEventArgs
ToString() into the message.
rabbitmq-dotnet-client/projects/RabbitMQ.Client/client/impl/Connection.Receive.cs
Lines 176 to 187 in 1d5b36a
private void HandleMainLoopException(ShutdownEventArgs reason) | |
{ | |
if (!SetCloseReason(reason)) | |
{ | |
LogCloseError($"Unexpected Main Loop Exception while closing: {reason}", reason.Exception); | |
return; | |
} | |
_channel0.MaybeSetConnectionStartException(reason.Exception); | |
OnShutdown(reason); | |
LogCloseError($"Unexpected connection closure: {reason}", reason.Exception); |
And the ShutdownEventArgs
ToString() puts the full exception.ToString()
in the result:
rabbitmq-dotnet-client/projects/RabbitMQ.Client/client/api/ShutdownEventArgs.cs
Lines 129 to 137 in dd10278
public override string ToString() | |
{ | |
return $"AMQP close-reason, initiated by {Initiator}" | |
+ $", code={ReplyCode}" | |
+ (ReplyText != null ? $", text='{ReplyText}'" : string.Empty) | |
+ $", classId={ClassId}" | |
+ $", methodId={MethodId}" | |
+ (Cause != null ? $", cause={Cause}" : string.Empty) | |
+ (_exception != null ? $", exception={_exception}" : string.Empty); |
We could fix this by adding a new method like "GetEventLogMessage()" method that contains all this information, but doesn't include the full ToString of the exception in the message.
See dotnet/aspire#2118 (comment) for more details.