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

ModelAndView's setStatus does not work for @ExceptionHandler methods [SPR-14796] #19362

Closed
spring-projects-issues opened this issue Oct 10, 2016 · 4 comments

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 10, 2016

Manuel Jordan opened SPR-14796 and commented

I am working with Spring Framework 4.3.3

About Spring MVC about handle exceptions I have the following about a @ControllerAdvice (note: not totally valid about logic):

@ExceptionHandler(value={EntityNotPreviouslyPersistedException.class})
public ModelAndView handleEntityNotPreviouslyPersistedException(
	EntityNotPreviouslyPersistedException ex,
	WebRequest request,
	HttpServletRequest request_) {

	logger.info("EntityNotPreviouslyPersistedException...");
        ...
		
	logger.info("HTTP method: {}", request_.getMethod());

	HttpStatus httpStatus = null;

	if(request_.getMethod().equals(HttpMethod.DELETE.toString()) || request_.getMethod().equals(HttpMethod.PUT.toString())){
		httpStatus = HttpStatus.BAD_REQUEST;
	}
	else if(request_.getMethod().equals(HttpMethod.GET.toString())){
		httpStatus = HttpStatus.NOT_FOUND;
	}
	else{
		logger.warn("It should be not printed...");
	}

	logger.info("HTTP method: {} - HttpStatus: {}", request_.getMethod(), httpStatus.toString());

	ModelAndView mav = new ModelAndView();
	mav.setStatus(httpStatus);
	mav.addObject("message", error);
	mav.setViewName("redirect:/error"); //but was:<302>
	//mav.setViewName("/error");// was:<200>
	return mav;
}

I did realize that mav.setStatus(httpStatus) is executed but finally is ignored.

  • If I use mav.setViewName("redirect:/error"); the status 302 is applied
  • If I use mav.setViewName("/error"); the status 200 is applied
  • If I don't use or not declare mav.setViewName the status 200 is applied

Even when has more sense the 302 status, it to show an error page with some message.
I have two questions from here:

(1) Why the setViewName overrides the HttpStatus value?
(2) In what valid scenario the setStatus is mandatory to be used? I did a research on Google about to get some examples seeing in action the setStatus method and nothing. Of course the better approach is use a @RequestMapping or @GetMapping method that returns a String object (view name) and use Model object.

I know it is trivial, but I am with this doubt.

Note: If I put in the current Spring Reference Documentation (htmlsingle mode) the setStatus term, there are no results.

Thanks by your understanding.


Affects: 4.3 GA, 4.3.3

Issue Links:

  • #18136 ability to set response status on ModelAndView
@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 19, 2016

Rossen Stoyanchev commented

It looks like we are only supporting the ModelAndView status on @RequestMapping methods but not on @ExceptionHandler, so that needs to be fixed.

Yes we could document this better in the docs and reference as well. In simple terms it should result in the response status being set prior to View rendering. As part of View rendering however the View itself can still change the status, and thus override it. This will be the case of RedirectView which selects an appropriate redirect code, which can be customized. I am not aware of any other View implementations that explicitly set the status.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 21, 2016

Manuel Jordan commented

Interesting. Good to know in someway, it is really a bug...

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 24, 2016

Rossen Stoyanchev commented

This is now resolved with efe399 and c06283.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 24, 2016

Manuel Jordan commented

Thanks a lot, btw the second link is broken or 404.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants