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

Add Javadoc to MvcResult getResponse().getErrorMessage() #31386

Closed
berse2212 opened this issue Oct 9, 2023 · 3 comments
Closed

Add Javadoc to MvcResult getResponse().getErrorMessage() #31386

berse2212 opened this issue Oct 9, 2023 · 3 comments
Assignees
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@berse2212
Copy link

We are currently in the middle of migrating to spring 6 (via Spring Boot 3)

In Spring 5 we checked the error Message in SpringBoot Tests like this:

this.mvc.perform(put("/badUrl/abcde"))
				.andExpect(status().isBadRequest())
				.andExpect(result -> assertThat(result.getResponse().getErrorMessage()).isEqualTo(ERROR_MSG));

Now with the change to ErrorDetail weg cannot get the error Message via the getErrorMessage() Method:

MockHttpServletResponse:
           Status = 400
    Error message = null
          Headers = [Vary:"Origin", "Access-Control-Request-Method", "Access-Control-Request-Headers", Content-Type:"application/problem+json", X-Content-Type-Options:"nosniff", X-XSS-Protection:"0", Cache-Control:"no-cache, no-store, max-age=0, must-revalidate", Pragma:"no-cache", Expires:"0", X-Frame-Options:"DENY"]
     Content type = application/problem+json
             Body = {"type":"about:blank","title":"Bad Request","status":400,"detail":"Fehler beim Zuweisen. Es wurden keine Änderungen der durchgeführt.","instance":"/badUrl/abcde"}
    Forwarded URL = null
   Redirected URL = null
          Cookies = []

 

org.opentest4j.AssertionFailedError: 
expected: "Fehler beim Zuweisen. Es wurden keine Änderungen der durchgeführt."
but was: null
Expected :"Fehler beim Zuweisen. Es wurden keine Änderungen der durchgeführt."
Actual   :null

Now the workaround would be validate the JSON and get the detail field manually, but I would expect the MockMvc Result to handle this. Otherwise the get ErrorMessage is highly misleading. Or am I misunderstanding something here?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 9, 2023
@sbrannen sbrannen added in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) labels Oct 9, 2023
@bclozel
Copy link
Member

bclozel commented Oct 13, 2023

This .getErrorMessage() method is about the Servlet response itself, and more precisely the HttpServletResponse#sendError method. This is not about the recent ProblemDetail support.

Otherwise the get ErrorMessage is highly misleading.

Maybe you would like to send a PR to add a Javadoc section for this method to disambiguate the current state of things?

Thanks!

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Oct 13, 2023
@bclozel bclozel self-assigned this Oct 13, 2023
@berse2212
Copy link
Author

berse2212 commented Oct 18, 2023

I am not sure I understand your response correctly. We are not calling the HttpServletResponse#sendError method but rather just throw a ResponseStatusException like this:

	throw new ResponseStatusException(HttpStatus.BAD_REQUEST, ERROR_MSG);

And rethrow it in the GlobalExceptionHandler:

	@ExceptionHandler({ResponseStatusException.class})
	public void handleResponseStatusExceptions(ResponseStatusException e) {
		throw e;
	}

Now that the automatic ResponseStatusException handling has changed to ProblemDetail like described here: https://stackoverflow.com/questions/75029947/error-response-body-changed-after-boot-3-upgrade we had to change our tests for these cases and manually parsing the JSON to get the error message.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 18, 2023
@bclozel
Copy link
Member

bclozel commented Oct 18, 2023

I am not sure I understand your response correctly. We are not calling the HttpServletResponse#sendError method but rather just throw a ResponseStatusException like this:

Yes, I understand. Previously, letting an exception unhandled would reach the Servlet container as HttpServletResponse#sendError and would render an error page with the Servlet container configuration. See

/**
* Apply the resolved status code and reason to the response.
* <p>The default implementation sends a response error using
* {@link HttpServletResponse#sendError(int)} or
* {@link HttpServletResponse#sendError(int, String)} if there is a reason
* and then returns an empty ModelAndView.
* @param statusCode the HTTP status code
* @param reason the associated reason (may be {@code null} or empty)
* @param response current HTTP response
* @since 5.0
*/
protected ModelAndView applyStatusAndReason(int statusCode, @Nullable String reason, HttpServletResponse response)

Here, the error is instead rendered as a ProblemDetail JSON response, so the getErrorMessage() call doesn't apply anymore. From a Servlet perspective, this is just a regular JSON response.

we had to change our tests for these cases and manually parsing the JSON to get the error message.

I think this is the right approach. Feel free to create another issue if you have ideas on how to improve this.

I think we'll reuse this issue to add the missing javadoc on this method.

@bclozel bclozel added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Oct 18, 2023
@bclozel bclozel added this to the 6.1.x milestone Oct 18, 2023
@bclozel bclozel changed the title MvcResult getResponse().getErrorMessage() doesn't return the new ProblemDetailMessage Add Javadoc to MvcResult getResponse().getErrorMessage() Oct 18, 2023
@bclozel bclozel modified the milestones: 6.1.x, 6.1.2 Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

4 participants