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.

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

ReplyToAddress header check causes an unexpected exception when bridging audit and error queues #122

Closed
mauroservienti opened this issue Sep 14, 2022 · 6 comments · Fixed by #172
Labels
Bug Something isn't working

Comments

@mauroservienti
Copy link
Member

mauroservienti commented Sep 14, 2022

Let’s imagine the following, intentionally, simplistic scenario:

Left side (ServiceControl):

  • audit queue
  • error queue

Right side:

  • endpoint A
  • endpoint B

Bridge:

  • on the left:
    • has: audit
    • has: error
  • on the right:
    • nothing, there are no user endpoints on the left side, no business communication happens across the bridge, only audited and failed messages

A sends a message to B. B has auditing enabled, processes the message, and dispatches the audited message to the audit queue. The bridge picks up the message from the audit queue and blows up with the following exception:

No target address mapping could be found for source address: A

Which is caused by the shoveling mechanism that checks for the ReplyToAddress header. In the above scenario, the processed message has that header set to “A”: https://github.com/Particular/NServiceBus.Transport.Bridge/blob/7a81f83649cac2f1b89253ea4c344a1c959df86d/src/NServiceBus.Transport.Bridge/MessageShovel.cs#L53-L66

TranslateToTargetAddress checks for mapped endpoints, and cannot find A

https://github.com/Particular/NServiceBus.Transport.Bridge/blob/7a81f83649cac2f1b89253ea4c344a1c959df86d/src/NServiceBus.Transport.Bridge/EndpointRegistry.cs#L56-L64

The above makes sense for “regular” messages, but, as far as I have understood, not for audited or failed messages. No one will reply to an audit message or a failed one. I’d even argue that for audited or failed messages that header doesn’t need to be translated, or at least the mapping check should be skipped.

A workaround would be to define all right-side endpoints even those that don’t need to send messages through the bridge, which is a cumbersome solution.

@kbaley
Copy link
Member

kbaley commented Oct 11, 2022

@mauroservienti Is your suggested workaround covered by this documentation?

@mauroservienti
Copy link
Member Author

No, it's not. The documentation is generic about bridging ServiceControl queues, it makes no mentions of what could happen when not all queues are mapped and how to workaround that. Which could be an improvement on its own.

@andreasohlund
Copy link
Member

#160 will fix the audit queue but errors is more tricky I believe since if we don't transform the address SC would not be able to retry the failure since the endpoint might be on a different transport than SC.

For that to work, I think we would have to make SC aware of the message being bridged and either be able to send it to all the transports or somehow relay the retry via the bridge.

@mauroservienti
Copy link
Member Author

The FailedQ header needs to be transformed for sure. I'm not sure the ReplyToAddress should be touched even for failed messages. ServiceControl will never use the ReplyToAddress, AFAIK.

@andreasohlund
Copy link
Member

Good point, the reply to address should be left untouched but the failed queue needs to be transformed 👍

This should be an easy fix similar to #160

@SzymonPobiega SzymonPobiega added the Bug Something isn't working label Dec 9, 2022
@SzymonPobiega SzymonPobiega added this to the 1.0.0 milestone Dec 9, 2022
@SzymonPobiega
Copy link
Member

Fixed in #172. Will be released in 1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants