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

HttpMediaTypeNotAcceptableException exception missing when using ContentNegotiationConfigurer #26742

Closed
membersound opened this issue Mar 29, 2021 · 8 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@membersound
Copy link

membersound commented Mar 29, 2021

spring-boot-stater-parent.2.4.3 Test OK
spring-boot-stater-parent.2.4.4 Test fails

I have a test that worked in 2.4.3, but fails since 2.4.4..
Thus I assume there has been a change, but I don't know where.

It certainly has to do with ContentNegotiationConfigurer when setting a custom parameterName("_format") and using defaultContentType(MediaType.APPLICATION_JSON).

I was not able to locate the change that introduced this. Neither am I sure if the former or later outcome is the correct one. But I think the HttpMediaTypeNotAcceptableException should not be swallowed (like now in 2.4.4.)

@Configuration
public class ContentNegotiationConfiguration implements WebMvcConfigurer {
	@Override
	public void configureContentNegotiation(ContentNegotiationConfigurer configurer) {
		configurer
			.favorParameter(true)
			.parameterName("_format")
			.defaultContentType(MediaType.APPLICATION_JSON);
	}
}

@SpringBootTest
@AutoConfigureMockMvc
public class SpringBasicErrorControllerTest {
	@Autowired
	private MockMvc mvc;

	//test for '/error' servlet handler
	@Test
	public void testMediaTypeNotSupported() throws Exception {
		MvcResult rsp = mvc.perform(MockMvcRequestBuilders.get("/error?_format=test")
				.requestAttr(RequestDispatcher.ERROR_STATUS_CODE, 501))
				.andReturn();

                //fails for 2.4.4 (ex is null)
		assertTrue(rsp.getResolvedException() instanceof HttpMediaTypeNotAcceptableException);
	}
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 29, 2021
@bclozel
Copy link
Member

bclozel commented Mar 29, 2021

I'm not sure this setup is valid; MockMvc is not using a real Servlet container, so error dispatches are not done in this case.

Are you testing the Error Controller provided by Spring Boot or a custom controller?
Do you still get the same error if you switch to using TestRestTemplate and a random port?

We can't think of a recent change that might provoke this, we'll need more information on this.

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Mar 29, 2021
@membersound
Copy link
Author

membersound commented Mar 30, 2021

I'm testing this on the default BasicErrorController from spring-boot.
What is interesting: when using an accept http header, the HttpMediaTypeNotAcceptableException is returned as expected. Both in 2.4.3. and 2.4.4.
Thus I assume that in general the exception should always be returned, as it was in 2.4.3.

	@Test
	public void testMediaTypeNotSupported() throws Exception {
		MvcResult rsp = mvc.perform(MockMvcRequestBuilders.get("/error")
                                                .accept("test/test")
				                .requestAttr(RequestDispatcher.ERROR_STATUS_CODE, 501))
				.andReturn();

		//succeeds for both 2.4.3 and 2.4.4
		assertTrue(rsp.getResolvedException() instanceof HttpMediaTypeNotAcceptableException);
	}

@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 Mar 30, 2021
@membersound
Copy link
Author

Still no opinions on this?

@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 10, 2021
@snicoll
Copy link
Member

snicoll commented Sep 26, 2023

Unfortunately, no. It's not even clear if the issue is located in Spring Boot or Spring Framework. In the lack of a sample that reproduces the problem, and given those versions are EOL now, I am going to close this issue. I am sorry this got overlooked, and we can obviously reopen if you can share a small sample that reproduces the problem with a supported version.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2023
@snicoll snicoll removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 26, 2023
@membersound
Copy link
Author

membersound commented Oct 10, 2023

@snicoll See working example attached, with latest spring-boot-3.1.4.

Still the problem is that MvcResult.getResolvedException() is null, which was not the case up to spring-boot-stater-parent.2.4.3! Please reopen.

HttpMediaType.zip

@snicoll snicoll reopened this Oct 10, 2023
@snicoll snicoll added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 10, 2023
@snicoll
Copy link
Member

snicoll commented Oct 21, 2023

I am trying to understand what you're trying to show and the test fails even if I remove the WebMvcConfigurer completely so I don't know if that's supposed to be related.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Oct 21, 2023
@membersound
Copy link
Author

membersound commented Oct 22, 2023

I just want to show that my test succeeds in spring-boot-starter-parent-2.4.3.

As written, the problem for junit tests is that MvcResult.getResolvedException() always resolves is null since 2.4.4 onwards, which makes it impossible to verify the exception (as it was possible until 2.4.3 where you could just call getResolvedException() and verify the exception returned from the web controller).

@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 22, 2023
@bclozel
Copy link
Member

bclozel commented Jan 2, 2024

I have tracked this down to the following change: #24539

The exception handling is now more lenient and this was done on purpose. If I understand correctly, a test was previously catching that (and actually testing the Framework behavior). I would suggest removing this assertion as a result since the behavior changed on purpose. I'm closing this issue as a result. Thanks!

@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jan 2, 2024
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 2, 2024
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: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

5 participants