Skip to content

Commit 8ff8351

Browse files
CrowayCroway
committed
fix: do not overwrite async timeout response with 500 in afterProcess
When Spring's async timeout handler sends 503, the executor thread's afterProcess() catches AsyncRequestNotUsableException from writeResponse() and falls through to sendError(500), overwriting the 503. Skip the sendError(500) fallback when the failure is AsyncRequestNotUsableException.
1 parent 029ae2a commit 8ff8351

2 files changed

Lines changed: 171 additions & 4 deletions

File tree

‎components-starter/camel-platform-http-starter/src/main/java/org/apache/camel/component/platform/http/springboot/SpringBootPlatformHttpConsumer.java‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public CompletableFuture<Void> service(HttpServletRequest request, HttpServletRe
9898
// do not leak exception back to caller
9999
LOG.warn("Error handling request due to: {}", e.getMessage(), e);
100100
try {
101-
if (!response.isCommitted()) {
101+
if (!isAsyncRequestNotUsable(e) && !response.isCommitted()) {
102102
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
103103
}
104104
} catch (Exception e1) {
@@ -149,21 +149,27 @@ protected void handleService(HttpServletRequest request, HttpServletResponse res
149149

150150
protected void afterProcess(HttpServletResponse response, Exchange exchange) throws Exception {
151151
boolean writeFailure = false;
152+
boolean asyncCompleted = false;
152153
try {
153154
// now lets output to the res
154155
if (LOG.isTraceEnabled()) {
155156
LOG.trace("Writing res for exchangeId: {}", exchange.getExchangeId());
156157
}
157158
binding.writeResponse(exchange, response);
158159
} catch (Exception e) {
159-
writeFailure = true;
160-
handleFailure(exchange, e);
160+
if (isAsyncRequestNotUsable(e)) {
161+
asyncCompleted = true;
162+
LOG.debug("Async request already completed, response not usable: {}", e.getMessage());
163+
} else {
164+
writeFailure = true;
165+
handleFailure(exchange, e);
166+
}
161167
} finally {
162168
doneUoW(exchange);
163169
releaseExchange(exchange, false);
164170
}
165171
try {
166-
if (writeFailure && !response.isCommitted()) {
172+
if (writeFailure && !asyncCompleted && !response.isCommitted()) {
167173
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
168174
}
169175
} catch (Exception e) {
@@ -172,6 +178,16 @@ protected void afterProcess(HttpServletResponse response, Exchange exchange) thr
172178

173179
}
174180

181+
static boolean isAsyncRequestNotUsable(Throwable e) {
182+
for (Throwable t = e; t != null; t = t.getCause()) {
183+
if ("org.springframework.web.context.request.async.AsyncRequestNotUsableException"
184+
.equals(t.getClass().getName())) {
185+
return true;
186+
}
187+
}
188+
return false;
189+
}
190+
175191
private void handleFailure(Exchange exchange, Throwable failure) {
176192
getExceptionHandler().handleException(
177193
"Failed writing HTTP response url: " + getEndpoint().getPath() + " due to: " + failure.getMessage(),

‎components-starter/camel-platform-http-starter/src/test/java/org/apache/camel/component/platform/http/springboot/SpringBootPlatformHttpAsyncNotUsableTest.java‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.camel.component.platform.http.springboot;
18+
19+
import jakarta.servlet.http.HttpServletResponse;
20+
import org.apache.camel.Exchange;
21+
import org.apache.camel.Processor;
22+
import org.apache.camel.builder.RouteBuilder;
23+
import org.apache.camel.component.platform.http.PlatformHttpEndpoint;
24+
import org.apache.camel.component.platform.http.spi.PlatformHttpConsumer;
25+
import org.apache.camel.component.platform.http.spi.PlatformHttpEngine;
26+
import org.apache.camel.http.common.DefaultHttpBinding;
27+
import org.apache.camel.spring.boot.CamelAutoConfiguration;
28+
import org.apache.camel.test.spring.junit6.CamelSpringBootTest;
29+
import org.assertj.core.api.Assertions;
30+
import org.junit.jupiter.api.Test;
31+
import org.springframework.beans.factory.annotation.Autowired;
32+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
33+
import org.springframework.boot.resttestclient.autoconfigure.AutoConfigureRestTestClient;
34+
import org.springframework.boot.test.context.SpringBootTest;
35+
import org.springframework.context.annotation.Bean;
36+
import org.springframework.context.annotation.Configuration;
37+
import org.springframework.core.env.Environment;
38+
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
39+
import org.springframework.security.web.SecurityFilterChain;
40+
import org.springframework.test.web.servlet.client.EntityExchangeResult;
41+
import org.springframework.test.web.servlet.client.RestTestClient;
42+
import org.springframework.web.context.request.async.AsyncRequestNotUsableException;
43+
44+
import java.io.IOException;
45+
46+
/**
47+
* Verifies that afterProcess() does NOT call sendError(500) when
48+
* writeResponse() throws AsyncRequestNotUsableException.
49+
*
50+
* When Spring's async timeout fires, the timeout handler already sends 503.
51+
* Camel's executor thread still runs and afterProcess() catches the write
52+
* failure. The current (buggy) code calls sendError(500) which overwrites
53+
* the 503. The fix should detect AsyncRequestNotUsableException and skip
54+
* the sendError(500) fallback.
55+
*/
56+
@EnableAutoConfiguration
57+
@CamelSpringBootTest
58+
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, classes = { CamelAutoConfiguration.class,
59+
SpringBootPlatformHttpAsyncNotUsableTest.TestConfiguration.class,
60+
PlatformHttpComponentAutoConfiguration.class, SpringBootPlatformHttpAutoConfiguration.class })
61+
@AutoConfigureRestTestClient
62+
public class SpringBootPlatformHttpAsyncNotUsableTest {
63+
64+
@Autowired
65+
RestTestClient restTestClient;
66+
67+
@Test
68+
public void testAsyncNotUsableShouldNotOverwriteWith500() throws Exception {
69+
EntityExchangeResult<String> result = restTestClient.get().uri("/async-not-usable")
70+
.exchange()
71+
.expectBody(String.class)
72+
.returnResult();
73+
74+
// When writeResponse() throws AsyncRequestNotUsableException,
75+
// afterProcess() should NOT call sendError(500).
76+
// In a real timeout scenario, Spring's timeout handler already sent 503.
77+
// Overwriting with 500 is the bug we're fixing.
78+
Assertions.assertThat(result.getStatus().value()).isNotEqualTo(500);
79+
}
80+
81+
@Test
82+
public void testRegularIOExceptionStillReturns500() throws Exception {
83+
EntityExchangeResult<String> result = restTestClient.get().uri("/io-error")
84+
.exchange()
85+
.expectBody(String.class)
86+
.returnResult();
87+
88+
// Regular IOExceptions during writeResponse() should still produce 500.
89+
// This is the existing correct behavior — only AsyncRequestNotUsableException
90+
// should be treated differently.
91+
Assertions.assertThat(result.getStatus().value()).isEqualTo(500);
92+
}
93+
94+
@Configuration
95+
public static class TestConfiguration {
96+
@Bean
97+
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
98+
http.authorizeHttpRequests(auth -> auth.anyRequest().permitAll())
99+
.csrf(csrf -> csrf.disable());
100+
return http.build();
101+
}
102+
103+
@Bean(name = "platform-http-engine")
104+
public PlatformHttpEngine asyncNotUsableEngine(Environment env) {
105+
int port = Integer.parseInt(env.getProperty("server.port", "8080"));
106+
return new AsyncNotUsableEngine(port);
107+
}
108+
109+
@Bean
110+
public RouteBuilder servletPlatformHttpRouteBuilder() {
111+
return new RouteBuilder() {
112+
@Override
113+
public void configure() {
114+
from("platform-http:/async-not-usable")
115+
.setBody().constant("ok");
116+
from("platform-http:/io-error")
117+
.setBody().constant("ok");
118+
}
119+
};
120+
}
121+
}
122+
123+
private static class AsyncNotUsableBinding extends DefaultHttpBinding {
124+
125+
@Override
126+
public void writeResponse(Exchange exchange, HttpServletResponse response) throws IOException {
127+
String uri = exchange.getMessage().getHeader(Exchange.HTTP_URI, String.class);
128+
if ("/async-not-usable".equals(uri)) {
129+
throw new AsyncRequestNotUsableException("Response not usable after async request completion");
130+
} else if ("/io-error".equals(uri)) {
131+
throw new IOException("Simulated IO error");
132+
} else {
133+
super.writeResponse(exchange, response);
134+
}
135+
}
136+
}
137+
138+
private static class AsyncNotUsableEngine extends SpringBootPlatformHttpEngine {
139+
140+
public AsyncNotUsableEngine(int port) {
141+
super(port);
142+
}
143+
144+
@Override
145+
public PlatformHttpConsumer createConsumer(PlatformHttpEndpoint endpoint, Processor processor) {
146+
SpringBootPlatformHttpConsumer answer = new SpringBootPlatformHttpConsumer(endpoint, processor);
147+
answer.setBinding(new AsyncNotUsableBinding());
148+
return answer;
149+
}
150+
}
151+
}

0 commit comments

Comments
 (0)