FailFastException and 2nd level retries behaviour change between v7 and v8

[blundell89]

Summary

Hi there,

We are working on upgrading from Rebus v7.2.1 to v8.2.2. We have second level retries enabled.

We have noticed that when throwing a FailFastException on v7, the behaviour is different on v8. On v7, the IFailed<> handler would be invoked whereas on v8, this is no longer the case.

Is this an intentional change in behaviour or is it a regression?

Reproduction

V7 app

using Rebus.Activation;
using Rebus.Config;
using Rebus.Exceptions;
using Rebus.Handlers;
using Rebus.Retry.Simple;

using var activator = new BuiltinHandlerActivator();

activator.Register(() => new Handler());

var bus = Configure.With(activator)
    .Transport(t => t.UseRabbitMq("rabbitmq://localhost", "test-queue-r7"))
    .Logging(l => l.ColoredConsole())
    .Options(o =>
    {
        o.SimpleRetryStrategy(secondLevelRetriesEnabled: true);
    })
    .Start();

await bus.Subscribe<Message>();

await bus.Publish(new Message(Guid.NewGuid()));

Console.WriteLine("Press enter to quit");
Console.ReadLine();

public record Message(Guid Id);

public class Handler : IHandleMessages<Message>, IHandleMessages<IFailed<Message>>
{
    public Task Handle(Message message)
    {
        Console.WriteLine("V7 handling message");
        throw new FailFastException("V7 failure!");
    }

    public Task Handle(IFailed<Message> message)
    {
        Console.WriteLine("V7 handling failed message");
        return Task.CompletedTask;
    }
}

V8 app

using Rebus.Activation;
using Rebus.Config;
using Rebus.Exceptions;
using Rebus.Handlers;
using Rebus.Retry.Simple;

using var activator = new BuiltinHandlerActivator();

activator.Register(() => new Handler());

var bus = Configure.With(activator)
    .Transport(t => t.UseRabbitMq("rabbitmq://localhost", "test-queue-r8"))
    .Logging(l => l.ColoredConsole())
    .Options(o =>
    {
        o.RetryStrategy(secondLevelRetriesEnabled: true);
    })
    .Start();

await bus.Subscribe<Message>();

await bus.Publish(new Message(Guid.NewGuid()));

Console.WriteLine("Press enter to quit");
Console.ReadLine();

public record Message(Guid Id);

public class Handler : IHandleMessages<Message>, IHandleMessages<IFailed<Message>>
{
    public Task Handle(Message message)
    {
        Console.WriteLine("V8 handling message");
        throw new FailFastException("V8 failure!");
    }

    public Task Handle(IFailed<Message> message)
    {
        Console.WriteLine("V8 handling failed message");
        return Task.CompletedTask;
    }
}

Output

[mookid8000]

It's a change in behavior, but it's not intentional... 😓 as in: I am unsure what the right thing to do is in this case.

My first impulse was to think that the v8 behavior was correct.... but after having thought some more about it, I could also see myself expect the v7 behavior.

Do you have any philosophical arguments for either behavior?

[AndreaCuneo]

@mookid8000 I do use FailFast for non-retriable exceptions configuring it globally via FailFastOn.

The v7 behaviour is useful to avoid every MessageHandler to have a common base or use Decorators to move directly to ErrorHandling on certain exceptions.

v7 behaviour is preferred to avoid a breaking change unless there is a benefit.

[blundell89]

Thanks for the quick response @mookid8000 😃

I agree with @AndreaCuneo. It's a nice feature to be able to avoid first-level retries with FailFastException and still have the ability to have the second-level behaviour before dead lettering. When opting in to second-level retries, I'd expect everything to follow that route (unless configured otherwise).

We use this feature today in a few places, although, like anything, if it was for the good of the product, we would make the changes to accommodate the breaking change.

[mookid8000]

I finally got around to look at this 😅 I haven't been able to come up with any real good philosophical argument for either solution, so I decided to look at changing the behavior to align with Rebus versions <=7... which is out as Rebus 8.2.3 on NuGet.org now.

There's one caveat to the new implementation though: Because Rebus 8 got a pretty fundamental refactoring in how retries are handled, to be able to work better in distributed error tracking scenarios, the 2nd level message delivery will be done immediately now, whereas previously it would roll back the incoming message and do the 2nd level delivery in a completely new message transaction.

Please try it out and see if it works for you 🙂

[blundell89]

Thank you for sorting quickly, and for keeping us updated!

[blundell89]

Hey again @mookid8000,

Having run a few tests it looks like the IFailed<Message>.Exceptions collection is now empty, where it previously contained the exception that was originally thrown. Would you expect this to still be populated?

Thanks again

[mookid8000]

Ah, good catch! And no, that was not the intention... let me just take a look at it

[mookid8000]

I've pushed Rebus 8.2.4 to NuGet.org now where I believe it works as intended!