search

rebus-org / Rebus.SqlServer

:bus: Microsoft SQL Server transport and persistence for Rebus
https://mookid.dk/category/rebus
Other
44 stars 45 forks source link

Defer using SQL Native TimeoutManager doesn't work in v7.2.0 #98

Open Denifia opened 1 year ago

Denifia commented 1 year ago

I have a use case where two systems are using rebus as two-way clients with sql server as the transport, each with their own queue name. System1 defers messages to System2 and in v7.1.7 that works fine but now that I've finally upgraded to v7.2.0, that functionality seems to be broken.

In System1, when a message is deferred via IBus.Defer or IBus.DeferLocal and the optional header rbs2-defer-recipient is set to "System2", a Rebus.Exceptions.MessageCouldNotBeDispatchedToAnyHandlersException is thrown when the message is dequeued from the "System1" queue and processed. To me, that sounds like it didn't attempt to deliver the message to System2 at all.

I've looked through the code and can't spot the exact issue but have been able to recreate my scenario with unit tests. Note that the ItWorks_SetDestinationHeaderFromTheOutside_WithTwoWaySender_AndSqlServerTimeoutManager test works if you set routing queue to "receiver" but that's not applicable for my situation.

I'd like to know if this is expected behaviour, if I've misconfigured something, or if it's a bug. If it's a bug I'm happy to help find and test a fix.

One additional finding I made is that in the existing tests, the automatic defer recipient pipeline step is never hit as the header is already set by something else.

mookid8000 commented 1 year ago

The problem seems to be that the rbs2-defer-recipient header gets overwritten by Rebus' routing: image

That seems like an unnecessarily strict thing to do, so I'll just go and see if I can relax that a bit. brb

No that was not the case. Looking at your test again.

mookid8000 commented 1 year ago

I guess the AutoDeferredRecipientStep must have been made redundant at some point in time. Cannot quite understand how that happened, but for that reason I'll go and update the test to verify something that makes more sense.

mookid8000 commented 1 year ago

Could you maybe look at the update test https://github.com/rebus-org/Rebus.SqlServer/blob/master/Rebus.SqlServer.Tests/Bugs/TestNativeDeferToSomeoneElse.cs and see if there's any of the updated test cases that match your scenario?

Denifia commented 1 year ago

Thanks for looking into this so quickly. I'll review your new tests against my use case when I'm back at work on Monday.

Denifia commented 1 year ago

New tests are good but no, they don't cover my use case.

If you replace the sender's transport setup with .Transport(x => x.UseSqlServer(new SqlServerTransportOptions(ConnectionString), "sender")), all 3 of the tests fail.

I am using 2 two-way clients in my use case. In your tests, you have 1 two-way client and 1 one-way client.