Clarify intended advice execution behavior in Spring version 5.2.7+

[ranjit-g]

Affects: 5.2.7.RELEASE

StackOverflow question : https://stackoverflow.com/q/65032499/4214241

Sample Github project shared for the question : https://github.com/RitamChakraborty/bank_transaction_springaop/tree/r_g_answer

Issue descrption :

The following @AfterThrowing advice does not work as expected in Spring versions above : 5.2.6.RELEASE

@After("execution(public void dev.ritam.model.Bank.setTempPin(..))")
public void validatePin() {
    if (bank.getPinCode() != bank.getTempPin()) {
        throw new RuntimeException("Wrong Pin");
    } else {
        System.out.println("Correct Pin");
    }
}

@AfterThrowing(value = "execution(public void dev.ritam.model.Bank.setTempPin(..))", throwing = "e")
public void logException(Exception e) {
    System.out.println(e.getMessage());
}

For spring versions above : 5.2.6.RELEASE ( tested with releases 5.2.7.RELEASE,5.2.8.RELEASE,5.2.9.RELEASE,5.3.1)

@AfterThrowing advice does not execute on exception from @After advice. The same results if @After advice is replaced with @Before.

For spring versions equal to or below : 5.2.6.RELEASE ( tested with releases 5.2.6.RELEASE,5.2.5.RELEASE,5.2.4.RELEASE, 5.2.3.RELEASE) , the advice happens from @AfterThrowing on exception from @After

[jhoeller]

This seems to be a side effect of #25186 which was trying to make advice ordering more reliable. We'll try to revise this once more.

[jhoeller]

It turns out that our revised advice ordering is actually correct and closely aligned with AspectJ semantics.

Each of the distinct advice types of a particular aspect is conceptually meant to apply to the join point directly. As a consequence, an @AfterThrowing advice method is not supposed to receive an exception from an accompanying @After/@AfterReturning method. This just happened to accidentally work before, in a JVM-dependent way, being at odds with AspectJ semantics.

Also, @After seems to be the wrong advice type for your purposes above. @After advice in AspectJ is defined as "after finally advice", analogous to a finally block in a try-catch statement. It will be invoked for any outcome, normal return or exception thrown from the join point (user-declared target method). In contrast to that, @AfterReturning will only apply to successful normal returns which is what your advice implementation seems to expect (otherwise your freshly thrown RuntimeException there would potentially override an original exception from the target method, silently swallowing the latter).

As a solution, keep using your @AfterThrowing method as advice for exceptions from the target method, while explicitly implementing the same logic (or delegating to the same logic) in an @AfterReturning method which contains your pin code check. This will work reliably in any AspectJ environment and in particular in any Spring version.

I've repurposed this ticket towards explicit documentation notes around the intended semantics.

[ranjit-g]

Thank you for the detailed explanation.

My understanding regarding the execution order of advice types was inline with your explanation until I saw a change in behavior between versions.

My original suggestion to the SO issue was to use @Before to validate and to handle the exception with @AfterThrowing . This arrangement , to my understanding , should work. But that is not happening (I did mention this in the first post) for versions above 5.2.6.RELEASE.

Sample code

    @Before("execution(public void dev.ritam.model.Bank.setTempPin(..))")
    public void validatePin() {
        if (bank.getPinCode() != bank.getTempPin()) {
            throw new RuntimeException("Wrong Pin");
        } else {
            System.out.println("Correct Pin");
        }
    }

    @AfterThrowing(value = "execution(public void dev.ritam.model.Bank.setTempPin(..))", throwing = "e")
    public void logException(Exception e) {
        System.out.println(e.getMessage());
    }

Could you please review and confirm if that is expected?