fix issue 26317

[dongbaibai]

fix 【@ExceptionHandler methods not invokable if matched on exception's cause level > 1 】
the cause is:
ExceptionHandlerMethodResolver#resolveMethodByThrowable Find a Method to handle the given Throwable
only find Exception.cause twice

it will not affect the order of looking up for exception handler methods (1.local exception handler method,2.global exception handler method)
it only affects ExceptionHandlerMethodResolver.resolveMethod that what we what

hope to adopt

[pivotal-issuemaster]

@dongbaibai Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[rod2j]

Hi @dongbaibai ,

Did you actually check that your change fixed the issue I related ?

Because watching at your commit, you only changed the implementation of ExceptionHandlerMethodResolver#resolveMethodByThrowable:

• you replaced the use of recursivity with a nested while loop
• but there's no change in the outcome

So that will obviously not change anything in the way the resolved method is actually called and cannot fix the issue.

[quaff]

Hi @dongbaibai ,

Did you actually check that your change fixed the issue I related ?

Because watching at your commit, you only changed the implementation of ExceptionHandlerMethodResolver#resolveMethodByThrowable:

• you replaced the use of recursivity with a nested while loop
• but there's no change in the outcome

So that will obviously not change anything in the way the resolved method is actually called and cannot fix the issue.

Agree.

[quaff]

Possible endless loop here, depth should be limited, and cause should not be exception itself.

[dongbaibai]

It can't be infinite loop

[dongbaibai]

hi @rod2j
here is original process:

1.find method by exception type first
2.if method is null and the exception has cause
3.then find method by cause type

so ExceptionHandlerMethodResolver#resolveMethodByThrowable can not find a method by exception type that was nested more than one exception

just like this : new RuntimeException(new Exception(new IllegalArgumentException("ExceptionHandler will be resolved but cannot be invoked")));

so i think what need to do is change the logic that assigns the exception type only

[rstoyanchev]

I don't think this addresses #26317 which fails not because the method is not matched, but because the method arguments cannot be properly matched. I'm closing for now but we can re-open again if necessary. @dongbaibai have you confirmed the fix in any way? For example with the sample attached to #26317. There are no tests either to confirm the fix.

[dongbaibai]

I don't think this addresses #26317 which fails not because the method is not matched, but because the method arguments cannot be properly matched. I'm closing for now but we can re-open again if necessary. @dongbaibai have you confirmed the fix in any way? For example with the sample attached to #26317. There are no tests either to confirm the fix.

i will attach some sample later

[rstoyanchev]

There is no need to attach a sample. There is a sample under #26317 and that should be used to confirm any potential fix.

[dongbaibai]

/**
 * For convenience, I've focused all of my code on this one class
 */
@SpringBootApplication
public class Application {

    public static void main(String[] args) {
        SpringApplication.run(Application.class);
    }

    @RestController
    public static class TestController {
        @GetMapping("/ExceptionHandler")
        public String hi() {
            throw new RuntimeException(new Exception(new IllegalArgumentException("error message")));
        }
    }

    /**
     * wrapper @ExceptionHandler method with ExceptionHandlerBean
     * to simulate to find an @ExceptionHandler method for the given exception
     * on ExceptionHandlerExceptionResolver.exceptionHandlerAdviceCache
     * not on the controller class itself
     */
    @ResponseBody
    @ControllerAdvice
    public static class ExceptionHandlerBean {

        @ExceptionHandler(value = {IllegalArgumentException.class})
        public String error() {
            return "error";
        }

    }
}

I will prove from two ways

1. init exceptionHandler, ExceptionHandlerExceptionResolver#initExceptionHandlerAdviceCache. The picture of exceptionHandler is below. We saw IllegalArgumentException mapping Application$ExceptionHandlerBean.error. So the init exceptionHandler is alright.
2. Find a method to handle the given Exception, ExceptionHandlerMethodResolver#resolveMethod. The picture of exception type is below.

[quaff]

@dongbaibai You should write unit test which failed before you commit and passed now, instead of pasting lots of code.

[sbrannen]

@dongbaibai, I've edited your latest comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

Furthermore, as mentioned previously in #26343 (comment), when submitting a PR that claims to fix a bug, the PR must contain one or more JUnit-based tests that demonstrate the bug (by failing before the fix) and the fix (by passing after the fix).

Providing a sample Spring Boot application that demonstrates the problem is not sufficient since it cannot be incorporated into the Spring Framework test suite.

This PR will remain closed until tests are provided that confirm the bug and the fix.

[pivotal-cla]

@dongbaibai Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.