search

rabbitmq / rabbitmq-dotnet-client

RabbitMQ .NET client for .NET Standard 2.0+ and .NET 4.6.2+
https://www.rabbitmq.com/dotnet.html
Other
2.05k stars 575 forks source link

HandleMainLoopException puts exception StackTrace in the EventSource Message #1493

Closed eerhardt closed 4 months ago

eerhardt commented 4 months ago

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

  1. Add an EventListener to the rabbitmq-client EventSource.
  2. Create a RabbitMQ connection to a server.
  3. Stop the server to generate an exception
  4. 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.

image

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.

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/1d5b36ad9d48f228a6086f4d90922289c8596225/projects/RabbitMQ.Client/client/impl/Connection.Receive.cs#L176-L187

And the ShutdownEventArgs ToString() puts the full exception.ToString() in the result:

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/dd102787d237b217f65acb43e4e020de2d6f8114/projects/RabbitMQ.Client/client/api/ShutdownEventArgs.cs#L129-L137

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 https://github.com/dotnet/aspire/issues/2118#issuecomment-1935280012 for more details.

lukebakken commented 4 months ago

Feel free to open a PR targeting 6.x. I can merge / port it into main.