From ef8d1db403fac0319261d168e51753c4fab95571 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 3 Oct 2023 19:01:13 +0100 Subject: [PATCH] Improve exception handling in startCallableProcessing Absorb any exception from the submission to the executor. It will be handled in the resulting ASYNC dispatch. Closes gh-30232 --- .../request/async/WebAsyncManager.java | 4 +-- .../request/async/WebAsyncManagerTests.java | 29 ++++++++++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index de35961e20..7f6fea22de 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -22,7 +22,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.Future; -import java.util.concurrent.RejectedExecutionException; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; @@ -344,10 +343,9 @@ public final class WebAsyncManager { }); interceptorChain.setTaskFuture(future); } - catch (RejectedExecutionException ex) { + catch (Throwable ex) { Object result = interceptorChain.applyPostProcess(this.asyncWebRequest, callable, ex); setConcurrentResultAndDispatch(result); - throw ex; } } diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java index 3b2c79d8a1..2b62a60f8a 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTests.java @@ -17,6 +17,7 @@ package org.springframework.web.context.request.async; import java.util.concurrent.Callable; +import java.util.concurrent.Future; import java.util.function.Consumer; import jakarta.servlet.http.HttpServletRequest; @@ -46,18 +47,18 @@ import static org.mockito.Mockito.verify; */ class WebAsyncManagerTests { - private AsyncWebRequest asyncWebRequest = mock(); + private final AsyncWebRequest asyncWebRequest = mock(); - private MockHttpServletRequest servletRequest = new MockHttpServletRequest(); + private final MockHttpServletRequest servletRequest = new MockHttpServletRequest(); - private WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(servletRequest); + private final WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(servletRequest); @BeforeEach void setup() { this.asyncManager.setTaskExecutor(new SyncTaskExecutor()); this.asyncManager.setAsyncWebRequest(this.asyncWebRequest); - verify(this.asyncWebRequest).addCompletionHandler((Runnable) notNull()); + verify(this.asyncWebRequest).addCompletionHandler(notNull()); reset(this.asyncWebRequest); } @@ -135,6 +136,26 @@ class WebAsyncManagerTests { verify(interceptor).postProcess(this.asyncWebRequest, task, concurrentResult); } + @Test // gh-30232 + void startCallableProcessingSubmitException() throws Exception { + RuntimeException ex = new RuntimeException(); + + setupDefaultAsyncScenario(); + + this.asyncManager.setTaskExecutor(new SimpleAsyncTaskExecutor() { + @Override + public Future submit(Runnable task) { + throw ex; + } + }); + this.asyncManager.startCallableProcessing(() -> "not used"); + + assertThat(this.asyncManager.hasConcurrentResult()).isTrue(); + assertThat(this.asyncManager.getConcurrentResult()).isEqualTo(ex); + + verifyDefaultAsyncScenario(); + } + @Test void startCallableProcessingBeforeConcurrentHandlingException() throws Exception { Callable task = new StubCallable(21);