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

Global exception handler catch wrong exception [SPR-16375] #20921

Closed
spring-projects-issues opened this issue Jan 12, 2018 · 3 comments
Closed

Global exception handler catch wrong exception [SPR-16375] #20921

spring-projects-issues opened this issue Jan 12, 2018 · 3 comments
Assignees
Labels
in: core

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jan 12, 2018

WhatAKitty opened SPR-16375 and commented

The controller file:

@RequestMapping('/test')
public void test() {
  throw new BusinessException("excpetion1");
}

The exception handler file:

@RequestMapping('/test')
@RestControllerAdvice(basePackages = "com.gnet")
public class BusinessExcepionAdvice {

    @ExceptionHandler(BusinessException.class)
    public ResponseEntity businessException(final BusinessException e) {
        throw new AnotherException("exception2")
    }

}

I think when I visit this controller method, it will give me the response as below:

{
    "timestamp": 1515751322930,
    "status": 500,
    "error": "Internal Server Error",
    "exception": "com.gnet.business.exception.AnotherException",
    "message": "excpetion2",
    "path": "/test"
}

But unfortunately, the result is this:

{
    "timestamp": 1515751322930,
    "status": 500,
    "error": "Internal Server Error",
    "exception": "com.gnet.business.exception.BusinessException",
    "message": "excpetion1",
    "path": "/test"
}

It will make people confused, especially if you have some code error in exception handler instead of throw exception manually as above.


Affects: 4.3.10

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 12, 2018

Rossen Stoyanchev commented

For debugging purposes, yes, but in production the original exception is more important. Note that AnotherException is also logged at warn level which is intended to assist with debugging.

Either way this is a confusing situation that you need to try and avoid (i.e. control exceptions in exception handling code more tightly). It is long standing, existing behavior that cannot change without causing regressions.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 15, 2018

WhatAKitty commented

But the exception handler is meanless. If we need origin exceptions, we could find it in exception stack also rather than put it into the last result.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 15, 2018

Rossen Stoyanchev commented

So you're actually raising the exception intentionally from the @ExceptionHandler method, and not just simulating an unexpected error during from an @ExceptionHandler. This is certainly not something that an @ExceptionHandler was designed for. I actually don't understand what you're trying to do. What do you want to happen to AnotherException, propagate to the Servlet container and become a 500?

@spring-projects-issues spring-projects-issues added type: enhancement in: core labels Jan 11, 2019
@spring-projects-issues spring-projects-issues removed the type: enhancement label Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core
Projects
None yet
Development

No branches or pull requests

2 participants