Skip to content

Commit 4772e54

Browse files
mcollovatimcollovati
authored
fix: notify listeners when InputStream download handler callback fails (#21952)
When a callback provided to DownloadHandler.fromInputStream throws an IOException the transfer progress listeners are not notified about the error. This change catched IOException potentially thrown by the callback and notifies the registered listeners. Part of #21931
1 parent b0bc948 commit 4772e54

File tree

6 files changed

+130
-34
lines changed

6 files changed

+130
-34
lines changed

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,16 @@ public InputStreamDownloadHandler(InputStreamDownloadCallback callback) {
4747
@Override
4848
public void handleDownloadRequest(DownloadEvent downloadEvent)
4949
throws IOException {
50-
DownloadResponse download = callback.complete(downloadEvent);
5150
VaadinResponse response = downloadEvent.getResponse();
51+
DownloadResponse download;
52+
try {
53+
download = callback.complete(downloadEvent);
54+
} catch (IOException ioe) {
55+
// Set status before output is closed (see #8740)
56+
response.setStatus(HttpStatusCode.INTERNAL_SERVER_ERROR.getCode());
57+
notifyError(downloadEvent, ioe);
58+
throw ioe;
59+
}
5260
if (download.hasError()) {
5361
response.setStatus(download.getError());
5462
return;

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

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import java.util.ArrayList;
2626
import java.util.List;
2727
import java.util.Optional;
28-
import java.util.concurrent.atomic.AtomicBoolean;
28+
import java.util.concurrent.atomic.AtomicReference;
2929

3030
import org.junit.Assert;
3131
import org.junit.Before;
@@ -135,7 +135,7 @@ public void onError(TransferContext context, IOException reason) {
135135

136136
@Test
137137
public void transferProgressListener_addListener_errorOccured_errorlistenerInvoked()
138-
throws URISyntaxException, IOException {
138+
throws IOException {
139139
DownloadEvent event = Mockito.mock(DownloadEvent.class);
140140
Mockito.when(event.getSession()).thenReturn(session);
141141
Mockito.when(event.getResponse()).thenReturn(response);
@@ -145,44 +145,62 @@ public void transferProgressListener_addListener_errorOccured_errorlistenerInvok
145145
.write(Mockito.any(byte[].class), Mockito.anyInt(),
146146
Mockito.anyInt());
147147
Mockito.when(event.getOutputStream()).thenReturn(outputStreamMock);
148-
List<String> invocations = new ArrayList<>();
148+
AtomicReference<Boolean> whenCompleteResult = new AtomicReference<>();
149+
InvocationTrackingTransferProgressListener transferListener = new InvocationTrackingTransferProgressListener();
149150
DownloadHandler handler = DownloadHandler.fromInputStream(req -> {
150151
// Simulate a download of 165000 bytes
151152
byte[] data = getBytes();
152153
ByteArrayInputStream inputStream = new ByteArrayInputStream(data);
153154
return new DownloadResponse(inputStream, "download",
154155
"application/octet-stream", data.length);
155-
}, new TransferProgressListener() {
156-
@Override
157-
public void onStart(TransferContext context) {
158-
invocations.add("onStart");
159-
}
156+
}, transferListener).whenComplete((context, success) -> {
157+
whenCompleteResult.set(success);
158+
});
160159

161-
@Override
162-
public void onProgress(TransferContext context,
163-
long transferredBytes, long totalBytes) {
164-
invocations.add("onProgress");
165-
}
160+
try {
161+
handler.handleDownloadRequest(event);
162+
Assert.fail("Expected an IOException to be thrown");
163+
} catch (Exception e) {
164+
}
165+
Assert.assertEquals(List.of("onStart", "onError"),
166+
transferListener.invocations);
167+
Assert.assertNotNull("Expected whenComplete to be invoked, but was not",
168+
whenCompleteResult.get());
169+
Assert.assertFalse(
170+
"Expected whenComplete to be invoked with false result, but got true",
171+
whenCompleteResult.get());
166172

167-
@Override
168-
public void onComplete(TransferContext context,
169-
long transferredBytes) {
170-
invocations.add("onComplete");
171-
}
173+
}
172174

173-
@Override
174-
public void onError(TransferContext context, IOException reason) {
175-
invocations.add("onError");
176-
Assert.assertEquals("I/O exception", reason.getMessage());
177-
}
175+
@Test
176+
public void transferProgressListener_addListener_callbackErrorOccured_errorlistenerInvoked() {
177+
DownloadEvent event = Mockito.mock(DownloadEvent.class);
178+
Mockito.when(event.getSession()).thenReturn(session);
179+
Mockito.when(event.getResponse()).thenReturn(response);
180+
Mockito.when(event.getOwningElement()).thenReturn(owner);
181+
OutputStream outputStreamMock = Mockito.mock(OutputStream.class);
182+
Mockito.when(event.getOutputStream()).thenReturn(outputStreamMock);
183+
184+
AtomicReference<Boolean> whenCompleteResult = new AtomicReference<>();
185+
186+
InvocationTrackingTransferProgressListener transferListener = new InvocationTrackingTransferProgressListener();
187+
DownloadHandler handler = DownloadHandler.fromInputStream(req -> {
188+
throw new IOException("I/O exception");
189+
}, transferListener).whenComplete((context, success) -> {
190+
whenCompleteResult.set(success);
178191
});
179192

180193
try {
181194
handler.handleDownloadRequest(event);
182195
Assert.fail("Expected an IOException to be thrown");
183196
} catch (Exception e) {
184197
}
185-
Assert.assertEquals(List.of("onStart", "onError"), invocations);
198+
Assert.assertEquals(List.of("onError"), transferListener.invocations);
199+
Assert.assertNotNull("Expected whenComplete to be invoked, but was not",
200+
whenCompleteResult.get());
201+
Assert.assertFalse(
202+
"Expected whenComplete to be invoked with false result, but got true",
203+
whenCompleteResult.get());
186204
}
187205

188206
@Test
@@ -327,4 +345,32 @@ private static byte[] getBytes() {
327345
}
328346
return data;
329347
}
348+
349+
private static class InvocationTrackingTransferProgressListener
350+
implements TransferProgressListener {
351+
private final List<String> invocations = new ArrayList<>();
352+
353+
@Override
354+
public void onStart(TransferContext context) {
355+
invocations.add("onStart");
356+
}
357+
358+
@Override
359+
public void onProgress(TransferContext context, long transferredBytes,
360+
long totalBytes) {
361+
invocations.add("onProgress");
362+
}
363+
364+
@Override
365+
public void onComplete(TransferContext context, long transferredBytes) {
366+
invocations.add("onComplete");
367+
}
368+
369+
@Override
370+
public void onError(TransferContext context, IOException reason) {
371+
invocations.add("onError");
372+
Assert.assertEquals("I/O exception", reason.getMessage());
373+
}
374+
}
375+
330376
}

‎flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/DownloadHandlerView.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,16 @@
1717

1818
import java.io.ByteArrayInputStream;
1919
import java.io.File;
20+
import java.io.IOException;
2021
import java.nio.charset.StandardCharsets;
21-
import java.util.ArrayList;
22-
import java.util.List;
2322

24-
import com.vaadin.flow.component.DetachEvent;
2523
import com.vaadin.flow.component.html.Anchor;
2624
import com.vaadin.flow.component.html.Div;
2725
import com.vaadin.flow.component.html.NativeButton;
2826
import com.vaadin.flow.router.Route;
27+
import com.vaadin.flow.server.HttpStatusCode;
2928
import com.vaadin.flow.server.streams.DownloadEvent;
3029
import com.vaadin.flow.server.streams.DownloadHandler;
31-
import com.vaadin.flow.server.HttpStatusCode;
32-
import com.vaadin.flow.server.StreamRegistration;
33-
import com.vaadin.flow.server.VaadinSession;
3430
import com.vaadin.flow.server.streams.DownloadResponse;
3531
import com.vaadin.flow.uitest.servlet.ViewTestLayout;
3632

@@ -88,16 +84,26 @@ public String getUrlPostfix() {
8884
inputStreamDownload.setId("download-handler-input-stream");
8985

9086
Anchor inputStreamErrorDownload = new Anchor("",
91-
"InputStream DownloadHandler shorthand");
87+
"InputStream DownloadHandler shorthand (ERROR)");
9288
inputStreamErrorDownload
9389
.setHref(DownloadHandler
9490
.fromInputStream(downloadEvent -> DownloadResponse
9591
.error(HttpStatusCode.INTERNAL_SERVER_ERROR))
9692
.inline());
9793
inputStreamErrorDownload.setId("download-handler-input-stream-error");
9894

95+
Anchor inputStreamCallbackError = new Anchor("",
96+
"InputStream DownloadHandler callback shorthand (CALLBACK EXCEPTION)");
97+
inputStreamCallbackError
98+
.setHref(DownloadHandler.fromInputStream(downloadEvent -> {
99+
throw new IOException("Callback exception");
100+
}).inline());
101+
inputStreamCallbackError
102+
.setId("download-handler-input-stream-callback-error");
103+
99104
add(handlerDownload, fileDownload, classDownload, servletDownload,
100-
inputStreamDownload, inputStreamErrorDownload);
105+
inputStreamDownload, inputStreamErrorDownload,
106+
inputStreamCallbackError);
101107

102108
NativeButton reattach = new NativeButton("Remove and add back",
103109
event -> {

‎flow-tests/test-root-context/src/main/java/com/vaadin/flow/uitest/ui/TransferProgressListenerView.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public class TransferProgressListenerView extends Div {
3333
static final String WHEN_START_ID = "for-servlet-resource-when-start";
3434
static final String ON_PROGRESS_ID = "for-servlet-resource-on-progress";
3535
static final String ON_ERROR_ID = "for-servlet-resource-on-error";
36+
static final String ON_CALLBACK_ERROR_ID = "from-inputstream-on-callback-error";
3637
static final String ON_COMPLETE_ID = "for-servlet-resource-when-complete";
3738

3839
public TransferProgressListenerView() {
@@ -99,5 +100,24 @@ public int read() throws IOException {
99100
add(imageError);
100101
add(new Div("Error:"));
101102
add(forServletResourceOnError);
103+
104+
Div fromInputStreamOnCallbackError = new Div(
105+
"File download onError status (callback error)...");
106+
fromInputStreamOnCallbackError.setId(ON_CALLBACK_ERROR_ID);
107+
errorDownloadHandler = DownloadHandler.fromInputStream(req -> {
108+
throw new IOException("Simulated error");
109+
}).whenComplete(success -> {
110+
if (!success) {
111+
fromInputStreamOnCallbackError.setText(
112+
"File download onError status: callback error");
113+
}
114+
});
115+
116+
imageError = new Image(errorDownloadHandler, "no-image");
117+
118+
add(imageError);
119+
add(new Div("Error (callback):"));
120+
add(fromInputStreamOnCallbackError);
121+
102122
}
103123
}

‎flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/DownloadHandlerIT.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.time.Duration;
2222
import java.time.temporal.ChronoUnit;
2323
import java.util.List;
24-
import java.util.concurrent.TimeUnit;
2524

2625
import org.apache.commons.io.FilenameUtils;
2726
import org.apache.commons.io.IOUtils;
@@ -137,6 +136,21 @@ public void getDynamicDownloadHandlerFailingInputStream_errorIsReceived() {
137136
findElement(By.className("error-code")).getText());
138137
}
139138

139+
@Test
140+
public void getDynamicDownloadHandlerFailingInputStreamCallback_errorIsReceived() {
141+
open();
142+
143+
WebElement link = findElement(
144+
By.id("download-handler-input-stream-callback-error"));
145+
link.click();
146+
147+
getDriver().manage().timeouts().scriptTimeout(Duration.of(15, SECONDS));
148+
149+
// Jetty error page
150+
Assert.assertTrue($("h2").withTextContaining("HTTP ERROR 500")
151+
.withTextContaining("java.io.IOException: Callback").exists());
152+
}
153+
140154
@Test
141155
public void detach_attachALink_getDynamicVaadinResource()
142156
throws IOException {

‎flow-tests/test-root-context/src/test/java/com/vaadin/flow/uitest/ui/TransferProgressListenerIT.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ public void downloadServletResource_listenersAdded_listenersInvoked()
4646
"File download whenComplete status: completed");
4747
waitForStatus(TransferProgressListenerView.ON_ERROR_ID,
4848
"File download onError status: error");
49+
waitForStatus(TransferProgressListenerView.ON_CALLBACK_ERROR_ID,
50+
"File download onError status: callback error");
4951
}
5052

5153
private void waitForStatus(String id, String status) {

0 commit comments

Comments
 (0)