Skip to content

Commit 8bfe7ab

Browse files
authored
fix: notify listener for all InputStreamDownloadCallback failures (#21994)
Makes sure that transfer progress listeners are notified also for DownloadResponse.error return value and unchecked exception thrown by InputStreamDownloadCallback. Fixes #21931
1 parent 6ced5a6 commit 8bfe7ab

File tree

2 files changed

+78
-5
lines changed

2 files changed

+78
-5
lines changed

flow-server/src/main/java/com/vaadin/flow/server/streams/InputStreamDownloadHandler.java

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.IOException;
2020
import java.io.InputStream;
2121
import java.io.OutputStream;
22+
import java.io.UncheckedIOException;
2223

2324
import com.vaadin.flow.server.HttpStatusCode;
2425
import com.vaadin.flow.server.VaadinResponse;
@@ -51,14 +52,27 @@ public void handleDownloadRequest(DownloadEvent downloadEvent)
5152
DownloadResponse download;
5253
try {
5354
download = callback.complete(downloadEvent);
54-
} catch (IOException ioe) {
55+
} catch (IOException | RuntimeException e) {
5556
// Set status before output is closed (see #8740)
5657
response.setStatus(HttpStatusCode.INTERNAL_SERVER_ERROR.getCode());
57-
notifyError(downloadEvent, ioe);
58-
throw ioe;
58+
IOException cause;
59+
if (e instanceof IOException ioe) {
60+
cause = ioe;
61+
} else if (e instanceof UncheckedIOException uioe) {
62+
cause = uioe.getCause();
63+
} else {
64+
cause = new IOException(e.getMessage(), e);
65+
}
66+
notifyError(downloadEvent, cause);
67+
throw e;
5968
}
6069
if (download.hasError()) {
6170
response.setStatus(download.getError());
71+
String message = download.getErrorMessage();
72+
if (message == null) {
73+
message = "Download failed with code " + download.getError();
74+
}
75+
notifyError(downloadEvent, new IOException(message));
6276
return;
6377
}
6478

flow-server/src/test/java/com/vaadin/flow/server/streams/InputStreamDownloadHandlerTest.java

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public void onError(TransferContext context, IOException reason) {
134134
}
135135

136136
@Test
137-
public void transferProgressListener_addListener_errorOccured_errorlistenerInvoked()
137+
public void transferProgressListener_addListener_errorOccurred_errorListenerInvoked()
138138
throws IOException {
139139
DownloadEvent event = Mockito.mock(DownloadEvent.class);
140140
Mockito.when(event.getSession()).thenReturn(session);
@@ -173,7 +173,7 @@ public void transferProgressListener_addListener_errorOccured_errorlistenerInvok
173173
}
174174

175175
@Test
176-
public void transferProgressListener_addListener_callbackErrorOccured_errorlistenerInvoked() {
176+
public void transferProgressListener_addListener_callbackIOExceptionOccurred_errorListenerInvoked() {
177177
DownloadEvent event = Mockito.mock(DownloadEvent.class);
178178
Mockito.when(event.getSession()).thenReturn(session);
179179
Mockito.when(event.getResponse()).thenReturn(response);
@@ -203,6 +203,65 @@ public void transferProgressListener_addListener_callbackErrorOccured_errorliste
203203
whenCompleteResult.get());
204204
}
205205

206+
@Test
207+
public void transferProgressListener_addListener_callbackUncheckedExceptionOccurred_errorListenerInvoked() {
208+
DownloadEvent event = Mockito.mock(DownloadEvent.class);
209+
Mockito.when(event.getSession()).thenReturn(session);
210+
Mockito.when(event.getResponse()).thenReturn(response);
211+
Mockito.when(event.getOwningElement()).thenReturn(owner);
212+
OutputStream outputStreamMock = Mockito.mock(OutputStream.class);
213+
Mockito.when(event.getOutputStream()).thenReturn(outputStreamMock);
214+
215+
AtomicReference<Boolean> whenCompleteResult = new AtomicReference<>();
216+
217+
InvocationTrackingTransferProgressListener transferListener = new InvocationTrackingTransferProgressListener();
218+
DownloadHandler handler = DownloadHandler.fromInputStream(req -> {
219+
throw new RuntimeException("I/O exception");
220+
}, transferListener).whenComplete((context, success) -> {
221+
whenCompleteResult.set(success);
222+
});
223+
224+
try {
225+
handler.handleDownloadRequest(event);
226+
Assert.fail("Expected an exception to be thrown");
227+
} catch (Exception e) {
228+
}
229+
Assert.assertEquals(List.of("onError"), transferListener.invocations);
230+
Assert.assertNotNull("Expected whenComplete to be invoked, but was not",
231+
whenCompleteResult.get());
232+
Assert.assertFalse(
233+
"Expected whenComplete to be invoked with false result, but got true",
234+
whenCompleteResult.get());
235+
}
236+
237+
@Test
238+
public void transferProgressListener_addListener_callbackResponseError_errorListenerInvoked()
239+
throws IOException {
240+
DownloadEvent event = Mockito.mock(DownloadEvent.class);
241+
Mockito.when(event.getSession()).thenReturn(session);
242+
Mockito.when(event.getResponse()).thenReturn(response);
243+
Mockito.when(event.getOwningElement()).thenReturn(owner);
244+
OutputStream outputStreamMock = Mockito.mock(OutputStream.class);
245+
Mockito.when(event.getOutputStream()).thenReturn(outputStreamMock);
246+
247+
AtomicReference<Boolean> whenCompleteResult = new AtomicReference<>();
248+
249+
InvocationTrackingTransferProgressListener transferListener = new InvocationTrackingTransferProgressListener();
250+
DownloadHandler handler = DownloadHandler.fromInputStream(
251+
req -> DownloadResponse.error(500, "I/O exception"),
252+
transferListener).whenComplete((context, success) -> {
253+
whenCompleteResult.set(success);
254+
});
255+
256+
handler.handleDownloadRequest(event);
257+
Assert.assertEquals(List.of("onError"), transferListener.invocations);
258+
Assert.assertNotNull("Expected whenComplete to be invoked, but was not",
259+
whenCompleteResult.get());
260+
Assert.assertFalse(
261+
"Expected whenComplete to be invoked with false result, but got true",
262+
whenCompleteResult.get());
263+
}
264+
206265
@Test
207266
public void inline_setFileNameInvokedByDefault() throws IOException {
208267
DownloadEvent event = Mockito.mock(DownloadEvent.class);

0 commit comments

Comments
 (0)