spring-cloud-gateway icon indicating copy to clipboard operation
spring-cloud-gateway copied to clipboard

Gateway MVC: Retry filter sends empty body when retrying POST/PUT/etc

Open MPoorter opened this issue 10 months ago • 4 comments

Spring Boot: 3.2.4 Spring Cloud Gateway MVC: 4.1.0 Spring Cloud: 2023.0.0

Summary: As far as I can tell, there is still an issue in Spring Cloud Gateway MVC with empty bodies in a retried http request. I am aware of this issue, but I believe that that was specific to and resolved for Spring Cloud Gateway Webflux. The issue still seems to exist in Spring Cloud Gateway MVC.

From my investigation, I think the issue lies in the fact that the body of the request is read when sending it across the first time. However, since the body is an InputSteam, that means it can no longer be read again the second time round. The same request object is used to retry, but this time it says the body is empty.

Looking at the RetryFilterFunctions, the same request object is passed to next and also passed to RetryException:

return (request, next) -> retryTemplate.execute(context -> {
	ServerResponse serverResponse = next.handle(request);

	if (isRetryableStatusCode(serverResponse.statusCode(), config)
			&& isRetryableMethod(request.method(), config)) {
		// use this to transfer information to HttpStatusRetryPolicy
		throw new RetryException(request, serverResponse);
	}
	return serverResponse;

Looking at the RestClientProxyExchange, we can see that the body is copied. However, this logic of the copying the body still reads the entire original InputStream and means that it cannot be read again afterwards. (I verified in debugging that after this method is called, reading the InputStream returns an empty array and the body of the request is an empty string).

Further down, I do see the following code:

ServerResponse serverResponse = GatewayServerResponse.status(clientResponse.getStatusCode())
		.build((req, httpServletResponse) -> {
			try (clientResponse) {
				// copy body from request to clientHttpRequest
				StreamUtils.copy(clientResponse.getBody(), httpServletResponse.getOutputStream());
			}
			return null;
		});

However, this wasn't being hit, and I'm unclear whether the comment is describing what the code is actually doing.


Below are samples of my code, but happy to also provide a minimal example if needed.

The Route with retry filter:

@Configuration
public class QuotingRouter {

    @Value("${services.quoting}")
    private String quotingService;

    @Bean
    public RouterFunction<ServerResponse> quotes() {
        return route("quotes")
            .route(path("/quotes/**"), http(quotingService))
            .filter(retry(config -> config.setRetries(2).addMethods(HttpMethod.POST)))
            .build();
    }
}

The test I'm running for it, using Wiremock and MockMVC:

private static final String JSON_CONTENT = "{\"testKey\": \"testValue\"}";

@DynamicPropertySource
static void configureProperties(final DynamicPropertyRegistry registry) {
    registry.add("services.quoting", WIRE_MOCK::baseUrl);
}

@RegisterExtension
public static final WireMockExtension WIRE_MOCK = WireMockExtension.newInstance()
    .options(wireMockConfig().dynamicPort())
    .configureStaticDsl(true)
    .build();

@Autowired
protected MockMvc mockMvc;

@SneakyThrows
void testPostForQuotingRoutesWillRetry() {
    stubFor(WireMock.post("/quotes/4500bb78-0dc4-4e0f-8401-69049118732d")
        .withRequestBody(equalToJson(JSON_CONTENT))
        .inScenario("Retry Scenario 2")
        .whenScenarioStateIs(STARTED)
        .willReturn(WireMock.serviceUnavailable())
        .willSetStateTo("Cause Success"));

    stubFor(WireMock.post("/quotes/4500bb78-0dc4-4e0f-8401-69049118732d")
        .withRequestBody(equalToJson(JSON_CONTENT))
        .inScenario("Retry Scenario 2")
        .whenScenarioStateIs("Cause Success")
        .willReturn(WireMock.ok()));

    mockMvc.perform(post("/quotes/4500bb78-0dc4-4e0f-8401-69049118732d")
            .content(JSON_CONTENT)
            .contentType(APPLICATION_JSON)
        )
        .andExpect(status().isOk());

    verify(2, WireMock.postRequestedFor(WireMock.urlPathEqualTo("/quotes/4500bb78-0dc4-4e0f-8401-69049118732d")).withRequestBody(equalToJson(JSON_CONTENT)));
}

This test fails with the following error message:

                                               Request was not matched
                                               =======================

-----------------------------------------------------------------------------------------------------------------------
| Closest stub                                             | Request                                                  |
-----------------------------------------------------------------------------------------------------------------------
                                                           |
POST                                                       | POST
/quotes/4500bb78-0dc4-4e0f-8401-69049118732d               | /quotes/4500bb78-0dc4-4e0f-8401-69049118732d
                                                           |
[equalToJson]                                              |                                                     <<<<< Body does not match
{                                                          |
  "testKey" : "testValue"                                  |
}                                                          |

MPoorter avatar Apr 05 '24 15:04 MPoorter

There's a BodyFilterFunctions.adaptCachedBody filter that I use with the read body predicate. Looks like we need to add MvcUtils.cacheAndReadBody() (or similar) to the retry filter. After that, the adaptCachedBody filter should work.

https://github.com/spencergibb/spring-cloud-gateway-mvc-sample/blob/ada208a372c8d1356776fbd7db7e5c4a5462d589/src/main/java/com/example/gatewaymvcsample/Route06ReadBodyPredicate.java#L24

spencergibb avatar Apr 05 '24 15:04 spencergibb

This issue would benefit from a more complete MRE (including all imports)

NadChel avatar Apr 12 '24 16:04 NadChel

What does MRE mean?

spencergibb avatar Apr 12 '24 18:04 spencergibb

What does MRE mean?

Minimal reproducible example

NadChel avatar Apr 12 '24 18:04 NadChel