Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HandleMainLoopException puts exception StackTrace in the EventSource Message #1493

Closed
eerhardt opened this issue Feb 9, 2024 · 1 comment · Fixed by #1495
Closed

HandleMainLoopException puts exception StackTrace in the EventSource Message #1493

eerhardt opened this issue Feb 9, 2024 · 1 comment · Fixed by #1495
Assignees
@lukebakken
Labels
bug
Milestone
6.9.0

Comments

@eerhardt
Copy link
Contributor

eerhardt commented Feb 9, 2024

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.

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.
@eerhardt eerhardt added the bug label Feb 9, 2024
This was referenced Feb 9, 2024
Closed
Merged
@lukebakken lukebakken self-assigned this Feb 9, 2024
@lukebakken lukebakken added this to the 6.9.0 milestone Feb 9, 2024
@lukebakken
Copy link
Contributor

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

eerhardt added a commit to eerhardt/rabbitmq-dotnet-client that referenced this issue Feb 9, 2024
@eerhardt
Remove StackTrace from EventSource Message
7ede330 
When logging connection exception information to the EventSoruce we are putting the full exception ToString in the Message which clutters it to the users.

Instead, just put the Exception.Message in the EventSource event message. The full exception information comes in the details of the EventSource event.

Fix rabbitmq#1493
@eerhardt eerhardt mentioned this issue Feb 9, 2024
Merged
11 tasks
lukebakken pushed a commit that referenced this issue Feb 12, 2024
@eerhardt @lukebakken
Remove StackTrace from EventSource Message
55010ab 
When logging connection exception information to the EventSoruce we are putting the full exception ToString in the Message which clutters it to the users.

Instead, just put the Exception.Message in the EventSource event message. The full exception information comes in the details of the EventSource event.

Fix #1493
lukebakken added a commit that referenced this issue Feb 12, 2024
@lukebakken
Port #1494 to main
67ce09c 
Fixes #1493 by porting 55010ab to `main`
@lukebakken lukebakken mentioned this issue Feb 12, 2024
Merged
lukebakken added a commit that referenced this issue Feb 12, 2024
@lukebakken
Port #1494 to main
e42fe53 
Fixes #1493 by porting 55010ab to `main`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukebakken lukebakken

Labels
bug
Projects
None yet
Milestone
6.9.0
Development

Successfully merging a pull request may close this issue.

Port #1494 to main rabbitmq/rabbitmq-dotnet-client
2 participants
@lukebakken @eerhardt