Skip to content

Common exceptions for resource not found, redirection, noContent etc... [SPR-11552] #16177

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

Closed
spring-projects-issues opened this issue Mar 14, 2014 · 5 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

cemo koc opened SPR-11552 and commented

In our codebase we are using some convenient exceptions for resource not found, redirection (302/301) and noContent...

It would be nice to have such kind of exceptions in spring. :)

Edit: I forgot to say that we are providing necessary mapping as well. For example a ResourceNotFoundException returns a 404 to client.


Affects: 3.2.8, 4.0.2

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

a ResourceNotFoundException returns a 404

We have NoHandlerFoundException translated to 404 in DefaultHandlerExceptionResolver.

redirection (302/301)

How do you use this, given that a controller can return a "redirect:" prefixed String.

noContent (204?)

Also a question mark here. I don't think of this as an error but rather a normal condition covered by @ResponseStatus + void return type or a ResponseEntity<Void>.

What others do you have in mind?

@spring-projects-issues
Copy link
Collaborator Author

cemo koc commented

I am using org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler with a ControllerAdvice.

My controllers are calling a service method.Then a service method is calling another service etc... For example:

ControllerA --> ServiceA --> ServiceB

In ServiceB I would like to call a RedirectException and want to be handled correctly. This helps to reduce unnecessary return statements and simplify code. Shortly these exceptions will be useful in not only controller layer but also in business layers. It is not easy to returning a ResponseEntity<Void> in business layer. But throwing an exception is easy to deal.

Another example will be in method parameter handling. In our code base we are resolving some parameters by URL. If there is mismatch or moved URL, we are throwing an exception to redirect. This is greatly reducing complexity and lets developer to focus on core.

A piece of code from my application:

@ControllerAdvice
public class ErrorControllerAdvice extends ResponseEntityExceptionHandler {

   @ExceptionHandler(RedirectException.class)
   @ResponseStatus(value = HttpStatus.MOVED_PERMANENTLY)
   public ModelAndView handleMovedPermanently(RedirectException ex, HttpServletResponse response) {
      RedirectView redirectView = new RedirectView();
      redirectView.setUrl(ex.getUrl());
      redirectView.setStatusCode(HttpStatus.MOVED_PERMANENTLY);
      return new ModelAndView(redirectView);
   }

   @ExceptionHandler(ResourceNotFoundException.class)
   @ResponseStatus(value = HttpStatus.NOT_FOUND)
   public String handleNotFound(ResourceNotFoundException ex) {
       
      //implementation
   }
}

I would like to see these exceptions in Spring Core and handled correctly in both DefaultHandlerExceptionResolver and ResponseEntityExceptionHandler.

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Business components arguably shouldn't raise exceptions that extend from web layer (or Spring MVC) types.

I came across an example in this spring.io PR where a BlogService raises a BlogPostMovedException which is then handled in BlogController through an @ExceptionHandler method. That's basically what I would expect. The web layer catching business exceptions and translating them accordingly.

A good way to think about it is how it would be if services were exposed through a different layer (integration scenario, jms, etc). You'd probably wish those exceptions had not been dependent on web concepts.

@spring-projects-issues
Copy link
Collaborator Author

cemo koc commented

I am considering DefaultHandlerExceptionResolver and ResponseEntityExceptionHandler as a part of web layer. If those exceptions could be declared in Spring MVC, I can not see how different modules will utilize them without depending on them.

Referred pull request is very nice and semantically it is very clear. But let me to give another an updated version of this.

  1. BlogPostMovedException extends RedirectException (This is what I want :) ).
  2. BlogService raises BlogPostMovedException
  3. DefaultHandlerExceptionResolver, ResponseEntityExceptionHandler and other related exception handle exception.

This is semantically same as before but reduce necessary codes to be written. You do not need to an exception handling mechanism in BlogController. Consider another Controller similar to BlogController. Do you think that It should also write same handling strategy?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented May 5, 2014

Rossen Stoyanchev commented

Yes I realize that's what you were asking for. However, the BlogService is part of sagan-common which does not depend on Spring MVC. The actual web logic is in sagan-site (see here). Also notice that the BlogController prepends "/blog/" and this is definitely not something that a business component should know.

You do not need to an exception handling mechanism in BlogController. Consider another Controller similar to BlogController. Do you think that It should also write same handling strategy?

Yes.

If the case with BlogPostMovedException was repeated in sagan in more areas and I found the number of exception-handling methods overwhelming, I might consider an interface (e.g. PublicSlugAware) or something that a business service can implement that then an @ExceptionHandler method could handle generically. Furthermore an @ControllerAdvice ensures this can be done once.

@spring-projects-issues spring-projects-issues added status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement in: web Issues in web modules (web, webmvc, webflux, websocket) labels Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants