Skip to content

Exception details lost when using async API #1831

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.

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

Open
SzymonPobiega opened this issue May 8, 2025 · 2 comments · May be fixed by #1832
Open

Exception details lost when using async API #1831

SzymonPobiega opened this issue May 8, 2025 · 2 comments · May be fixed by #1832
Assignees
Labels
Milestone

Comments

@SzymonPobiega
Copy link

Describe the bug

It seems that the async API layer does not allow access to the root cause of the error when using BasicPublic.

When BasicPublishAsync fails (we call it here in our code) we get the following RabbitMQ.Client.Exceptions.PublishException exception:

RabbitMQ.Client.Exceptions.PublishException: Exception of type 'RabbitMQ.Client.Exceptions.PublishException' was thrown.
   at RabbitMQ.Client.Impl.Channel.PublisherConfirmationInfo.MaybeWaitForConfirmationAsync(CancellationToken cancellationToken)
   at RabbitMQ.Client.Impl.Channel.MaybeEndPublisherConfirmationTracking(PublisherConfirmationInfo publisherConfirmationInfo, CancellationToken cancellationToken)
   at RabbitMQ.Client.Impl.Channel.BasicPublishAsync[TProperties](String exchange, String routingKey, Boolean mandatory, TProperties basicProperties, ReadOnlyMemory`1 body, CancellationToken cancellationToken)
   at NServiceBus.Transport.RabbitMQ.ConfirmsAwareChannel.SendMessage(String address, OutgoingMessage message, BasicProperties properties, CancellationToken cancellationToken) in /_/src/NServiceBus.Transport.RabbitMQ/Connection/ConfirmsAwareChannel.cs:line 38
   at NServiceBus.Transport.RabbitMQ.MessageDispatcher.Dispatch(TransportOperations outgoingMessages, TransportTransaction transaction, CancellationToken cancellationToken) in /_/src/NServiceBus.Transport.RabbitMQ/Sending/MessageDispatcher.cs:line 48
   at NServiceBus.Transport.RabbitMQ.MessageDispatcher.Dispatch(TransportOperations outgoingMessages, TransportTransaction transaction, CancellationToken cancellationToken) in /_/src/NServiceBus.Transport.RabbitMQ/Sending/MessageDispatcher.cs:line 52
   at NServiceBus.TransportReceiveToPhysicalMessageConnector.Invoke(ITransportReceiveContext context, Func`2 next) in /_/src/NServiceBus.Core/Pipeline/Incoming/TransportReceiveToPhysicalMessageConnector.cs:line 67
   at NServiceBus.RetryAcknowledgementBehavior.Invoke(ITransportReceiveContext context, Func`2 next) in /_/src/NServiceBus.Core/ServicePlatform/Retries/RetryAcknowledgementBehavior.cs:line 25
   at NServiceBus.TracingExtensions.<>c__DisplayClass0_0`1.<<Invoke>g__TracePipelineStatus|0>d.MoveNext() in /_/src/NServiceBus.Core/OpenTelemetry/Tracing/TracingExtensions.cs:line 19
--- End of stack trace from previous location ---
   at NServiceBus.MainPipelineExecutor.Invoke(MessageContext messageContext, CancellationToken cancellationToken) in /_/src/NServiceBus.Core/Pipeline/MainPipelineExecutor.cs:line 50
   at NServiceBus.MainPipelineExecutor.Invoke(MessageContext messageContext, CancellationToken cancellationToken) in /_/src/NServiceBus.Core/Pipeline/MainPipelineExecutor.cs:line 78
   at NServiceBus.Transport.RabbitMQ.MessagePump.Process(AsyncEventingBasicConsumer consumer, BasicDeliverEventArgs message, CancellationToken messageProcessingCancellationToken) in /_/src/NServiceBus.Transport.RabbitMQ/Receiving/MessagePump.cs:line 437

Notice, the exception's message property is Exception of type 'RabbitMQ.Client.Exceptions.PublishException' was thrown. and there seem to be no dedicated properties on the exception object itself that would allow us to figure out what has happened.

Previously, we used BasicPublish (equivalent place in our codebase) and handled the errors using BasicReturn event:

void Channel_BasicReturn(object sender, BasicReturnEventArgs e)
{
    var message = $"Message could not be routed to {e.Exchange + e.RoutingKey}: {e.ReplyCode} {e.ReplyText}";

    if (e.BasicProperties.TryGetConfirmationId(out var deliveryTag))
    {
        SetException(deliveryTag, message);
    }
    else
    {
        Logger.Warn(message);
    }
}

This resulted in a nice exception message that could be used to either diagnose the issue or make about the resolution strategy:

System.Exception: Message could not be routed to amq.rabbitmq.reply-to.g1h2AA9yZXBseUAxMzAwMzQ0MjAAAAVRAAAAAGgA9c8=.XqSncsOeQAvX+2A45bhwhA==: 312 NO_ROUTE

Reproduction steps

For the specific exception that we are dealing with:

  1. Create an exchange that is not bound to any queue
  2. When using sync API, subscribe to BasicReturn
  3. Use BasicPublish to publish a message to that exchange and observe the event containing the details of the problem
  4. When using API, call BasicPublishAsync to publish a message to that exchange and observe PublishException with no problem details available

Expected behavior

The message property on PublishException contains the reason of the failure. What would be even better is dedicated properties on that exception type that contain more specific information.

Additional context

No response

@lukebakken lukebakken added this to the 7.2.0 milestone May 8, 2025
@lukebakken lukebakken self-assigned this May 8, 2025
lukebakken added a commit that referenced this issue May 8, 2025
Fixes #1831

This PR adds the `PublishReturnException` class that includes the
originating exchange and routing key for a `basic.return` message. It
should be backwards-compatible in the API.
@lukebakken
Copy link
Collaborator

Hello @SzymonPobiega!

I'm assuming that you're not using the BasicReturnAsync event any more, because in this scenario, that event handler will be called here and should provide the same information that it used to. Immediately after that event handler is called, the TaskCompletionSource for the publish will be set with the PublishException instance you see.

With regard to the PublishException, the IsReturn property on the exception should be true. Is it not?

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/Exceptions/PublishException.cs#L56-L59

The following property should give the delivery tag:

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/main/projects/RabbitMQ.Client/Exceptions/PublishException.cs#L61-L64

I'll have a PR for this request shortly, but I'll leave it to @Gsantomaggio to add a test for the new exception class 😇

@SzymonPobiega
Copy link
Author

Thanks for jumping on it! The exchange name in the PublishReturnException would be super helpful. I added one comment on a PR -- if it would be feasible to add also reply code and text (I am not sure if these values make sense in all cases when PublishReturnException is thrown).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants