-
Notifications
You must be signed in to change notification settings - Fork 613
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
EventListenerto therabbitmq-clientEventSource. - 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.
