Skip to content

Commit 98479a3

Browse files
authored
Add suppressed exceptions to BatchRequestException in AbstractRequestCache (#2431)
When batch requests fail, individual exceptions are now added as suppressed exceptions to preserve stack traces and error context for debugging. - Modify AbstractRequestCache.requests() to call addSuppressed() for each failed request - Add comprehensive test suite in AbstractRequestCacheTest to verify exception handling - Test covers batch failures, mixed success/failure, and successful batch scenarios
1 parent 278b549 commit 98479a3

File tree

2 files changed

+269
-1
lines changed

2 files changed

+269
-1
lines changed

impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,14 @@ public <REQ extends Request<?>, REP extends Result<REQ>> List<REP> requests(
150150
}
151151

152152
if (hasFailures) {
153-
throw new BatchRequestException("One or more requests failed", allResults);
153+
BatchRequestException exception = new BatchRequestException("One or more requests failed", allResults);
154+
// Add all individual exceptions as suppressed exceptions to preserve stack traces
155+
for (RequestResult<REQ, REP> result : allResults) {
156+
if (result.error() != null) {
157+
exception.addSuppressed(result.error());
158+
}
159+
}
160+
throw exception;
154161
}
155162

156163
return allResults.stream().map(RequestResult::result).toList();
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.impl.cache;
20+
21+
import java.util.Arrays;
22+
import java.util.List;
23+
import java.util.function.Function;
24+
25+
import org.apache.maven.api.ProtoSession;
26+
import org.apache.maven.api.annotations.Nonnull;
27+
import org.apache.maven.api.cache.BatchRequestException;
28+
import org.apache.maven.api.cache.RequestResult;
29+
import org.apache.maven.api.services.Request;
30+
import org.apache.maven.api.services.RequestTrace;
31+
import org.apache.maven.api.services.Result;
32+
import org.junit.jupiter.api.BeforeEach;
33+
import org.junit.jupiter.api.Test;
34+
35+
import static org.junit.jupiter.api.Assertions.assertEquals;
36+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
37+
import static org.junit.jupiter.api.Assertions.assertNotNull;
38+
import static org.junit.jupiter.api.Assertions.assertThrows;
39+
import static org.junit.jupiter.api.Assertions.assertTrue;
40+
import static org.mockito.Mockito.mock;
41+
42+
class AbstractRequestCacheTest {
43+
44+
private TestRequestCache cache;
45+
46+
@BeforeEach
47+
void setUp() {
48+
cache = new TestRequestCache();
49+
}
50+
51+
@Test
52+
void testBatchRequestExceptionIncludesSuppressedExceptions() {
53+
// Create mock requests and results
54+
TestRequest request1 = createTestRequest("request1");
55+
TestRequest request2 = createTestRequest("request2");
56+
TestRequest request3 = createTestRequest("request3");
57+
58+
// Create specific exceptions with different messages and stack traces
59+
RuntimeException exception1 = new RuntimeException("Error processing request1");
60+
IllegalArgumentException exception2 = new IllegalArgumentException("Invalid argument in request2");
61+
IllegalStateException exception3 = new IllegalStateException("Invalid state in request3");
62+
63+
// Set up the cache to return failures for all requests
64+
cache.addFailure(request1, exception1);
65+
cache.addFailure(request2, exception2);
66+
cache.addFailure(request3, exception3);
67+
68+
List<TestRequest> requests = Arrays.asList(request1, request2, request3);
69+
70+
// Create a supplier that should not be called since we're simulating cached failures
71+
Function<List<TestRequest>, List<TestResult>> supplier = reqs -> {
72+
throw new AssertionError("Supplier should not be called in this test");
73+
};
74+
75+
// Execute the batch request and expect BatchRequestException
76+
BatchRequestException batchException =
77+
assertThrows(BatchRequestException.class, () -> cache.requests(requests, supplier));
78+
79+
// Verify the main exception message
80+
assertEquals("One or more requests failed", batchException.getMessage());
81+
82+
// Verify that all individual exceptions are included as suppressed exceptions
83+
Throwable[] suppressedExceptions = batchException.getSuppressed();
84+
assertNotNull(suppressedExceptions);
85+
assertEquals(3, suppressedExceptions.length);
86+
87+
// Verify each suppressed exception
88+
assertTrue(Arrays.asList(suppressedExceptions).contains(exception1));
89+
assertTrue(Arrays.asList(suppressedExceptions).contains(exception2));
90+
assertTrue(Arrays.asList(suppressedExceptions).contains(exception3));
91+
92+
// Verify the results contain the correct error information
93+
List<RequestResult<?, ?>> results = batchException.getResults();
94+
assertEquals(3, results.size());
95+
96+
for (RequestResult<?, ?> result : results) {
97+
assertNotNull(result.error());
98+
assertInstanceOf(RuntimeException.class, result.error());
99+
}
100+
}
101+
102+
@Test
103+
void testBatchRequestWithMixedSuccessAndFailure() {
104+
TestRequest successRequest = createTestRequest("success");
105+
TestRequest failureRequest = createTestRequest("failure");
106+
107+
RuntimeException failureException = new RuntimeException("Processing failed");
108+
109+
// Set up mixed success/failure scenario
110+
cache.addFailure(failureRequest, failureException);
111+
112+
List<TestRequest> requests = Arrays.asList(successRequest, failureRequest);
113+
114+
Function<List<TestRequest>, List<TestResult>> supplier = reqs -> {
115+
// Only the success request should reach the supplier
116+
assertEquals(1, reqs.size());
117+
assertEquals(successRequest, reqs.get(0));
118+
return List.of(new TestResult(successRequest));
119+
};
120+
121+
BatchRequestException batchException =
122+
assertThrows(BatchRequestException.class, () -> cache.requests(requests, supplier));
123+
124+
// Verify only the failure exception is suppressed
125+
Throwable[] suppressedExceptions = batchException.getSuppressed();
126+
assertEquals(1, suppressedExceptions.length);
127+
assertEquals(failureException, suppressedExceptions[0]);
128+
129+
// Verify results: one success, one failure
130+
List<RequestResult<?, ?>> results = batchException.getResults();
131+
assertEquals(2, results.size());
132+
133+
RequestResult<?, ?> result1 = results.get(0);
134+
RequestResult<?, ?> result2 = results.get(1);
135+
136+
// One should be success, one should be failure
137+
boolean hasSuccess = (result1.error() == null) || (result2.error() == null);
138+
boolean hasFailure = (result1.error() != null) || (result2.error() != null);
139+
140+
assertTrue(hasSuccess);
141+
assertTrue(hasFailure);
142+
}
143+
144+
@Test
145+
void testSuccessfulBatchRequestDoesNotThrowException() {
146+
TestRequest request1 = createTestRequest("success1");
147+
TestRequest request2 = createTestRequest("success2");
148+
149+
List<TestRequest> requests = Arrays.asList(request1, request2);
150+
151+
Function<List<TestRequest>, List<TestResult>> supplier =
152+
reqs -> reqs.stream().map(TestResult::new).toList();
153+
154+
// Should not throw any exception
155+
List<TestResult> results = cache.requests(requests, supplier);
156+
157+
assertEquals(2, results.size());
158+
assertEquals(request1, results.get(0).getRequest());
159+
assertEquals(request2, results.get(1).getRequest());
160+
}
161+
162+
// Helper methods and test classes
163+
164+
private TestRequest createTestRequest(String id) {
165+
ProtoSession session = mock(ProtoSession.class);
166+
return new TestRequestImpl(id, session);
167+
}
168+
169+
// Test implementations
170+
171+
interface TestRequest extends Request<ProtoSession> {}
172+
173+
static class TestRequestImpl implements TestRequest {
174+
private final String id;
175+
private final ProtoSession session;
176+
177+
TestRequestImpl(String id, ProtoSession session) {
178+
this.id = id;
179+
this.session = session;
180+
}
181+
182+
@Override
183+
@Nonnull
184+
public ProtoSession getSession() {
185+
return session;
186+
}
187+
188+
@Override
189+
public RequestTrace getTrace() {
190+
return null;
191+
}
192+
193+
@Override
194+
public boolean equals(Object obj) {
195+
if (this == obj) {
196+
return true;
197+
}
198+
if (obj == null || getClass() != obj.getClass()) {
199+
return false;
200+
}
201+
TestRequestImpl that = (TestRequestImpl) obj;
202+
return java.util.Objects.equals(id, that.id);
203+
}
204+
205+
@Override
206+
public int hashCode() {
207+
return java.util.Objects.hash(id);
208+
}
209+
210+
@Override
211+
@Nonnull
212+
public String toString() {
213+
return "TestRequest[" + id + "]";
214+
}
215+
}
216+
217+
static class TestResult implements Result<TestRequest> {
218+
private final TestRequest request;
219+
220+
TestResult(TestRequest request) {
221+
this.request = request;
222+
}
223+
224+
@Override
225+
@Nonnull
226+
public TestRequest getRequest() {
227+
return request;
228+
}
229+
}
230+
231+
static class TestRequestCache extends AbstractRequestCache {
232+
private final java.util.Map<TestRequest, RuntimeException> failures = new java.util.HashMap<>();
233+
234+
void addFailure(TestRequest request, RuntimeException exception) {
235+
failures.put(request, exception);
236+
}
237+
238+
@Override
239+
protected <REQ extends Request<?>, REP extends Result<REQ>> CachingSupplier<REQ, REP> doCache(
240+
REQ req, Function<REQ, REP> supplier) {
241+
// Check if we have a pre-configured failure for this request
242+
RuntimeException failure = failures.get(req);
243+
if (failure != null) {
244+
// Return a pre-cached failure by creating a supplier that always throws
245+
return new PreCachedFailureCachingSupplier<>(failure);
246+
}
247+
248+
// For non-failure cases, return a normal caching supplier
249+
return new CachingSupplier<>(supplier);
250+
}
251+
252+
// Custom CachingSupplier that simulates a pre-cached failure
253+
private static class PreCachedFailureCachingSupplier<REQ, REP> extends CachingSupplier<REQ, REP> {
254+
PreCachedFailureCachingSupplier(RuntimeException failure) {
255+
super(null); // No supplier needed
256+
// Pre-populate the value with the failure
257+
this.value = new AltRes(failure);
258+
}
259+
}
260+
}
261+
}

0 commit comments

Comments
 (0)