Skip to content

Commit caf4c81

Browse files
DenisPekka Hyvönen
authored andcommitted
fix: don't use ErrorHandler for IOExceptions in StreamReceiverHandler (#10553)
Fixes #10351 Co-authored-by: Pekka Hyvönen <pekka@vaadin.com>
1 parent 17b6486 commit caf4c81

2 files changed

Lines changed: 65 additions & 25 deletions

File tree

flow-server/src/main/java/com/vaadin/flow/server/communication/StreamReceiverHandler.java

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,16 @@ protected void doHandleMultipartFileUpload(VaadinSession session,
187187
success = handleMultipartFileUploadFromInputStream(session,
188188
request, streamReceiver, owner);
189189
}
190+
} catch (IOException exception) {
191+
// do not report IO exceptions via ErrorHandler
192+
getLogger().warn(
193+
"IO Exception during file upload, fired as StreamingErrorEvent",
194+
exception);
190195
} catch (Exception exception) {
191196
session.lock();
192197
try {
198+
// Report other than IO exceptions, which are mistakes, via
199+
// ErrorHandler
193200
session.getErrorHandler().error(new ErrorEvent(exception));
194201
} finally {
195202
session.unlock();
@@ -500,37 +507,39 @@ private final Pair<Boolean, UploadStatus> streamToReceiver(
500507
session.unlock();
501508
}
502509
success = true;
503-
} catch (UploadInterruptedException e) {
504-
// Download interrupted by application code
505-
tryToCloseStream(out);
506-
StreamVariable.StreamingErrorEvent event = new StreamingErrorEventImpl(
507-
filename, type, contentLength, totalBytes, e);
508-
session.lock();
509-
try {
510-
streamVariable.streamingFailed(event);
511-
} finally {
512-
session.unlock();
513-
}
514-
// Note, we are not throwing interrupted exception forward as it is
515-
// not a terminal level error like all other exception.
510+
} catch (UploadInterruptedException | IOException e) {
511+
// Download is either interrupted by application code or some
512+
// IOException happens
513+
onStreamingFailed(session, filename, type, contentLength,
514+
streamVariable, out, totalBytes, e);
515+
// Interrupted exception and IOEXception are not thrown forward:
516+
// it's enough to fire them via streamVariable
516517
} catch (final Exception e) {
517-
tryToCloseStream(out);
518-
session.lock();
519-
try {
520-
StreamVariable.StreamingErrorEvent event = new StreamingErrorEventImpl(
521-
filename, type, contentLength, totalBytes, e);
522-
streamVariable.streamingFailed(event);
523-
// throw exception for terminal to be handled (to be passed to
524-
// terminalErrorHandler)
525-
throw new UploadException(e);
526-
} finally {
527-
session.unlock();
528-
}
518+
onStreamingFailed(session, filename, type, contentLength,
519+
streamVariable, out, totalBytes, e);
520+
// Throw not IOException and interrupted exception for terminal to
521+
// be handled (to be passed to terminalErrorHandler): such
522+
// exceptions mean mistakes in the implementation logic (not upload
523+
// I/O operations).
524+
throw new UploadException(e);
529525
}
530526
return new Pair<>(startedEvent.isDisposed(),
531527
success ? UploadStatus.OK : UploadStatus.ERROR);
532528
}
533529

530+
private void onStreamingFailed(VaadinSession session, String filename,
531+
String type, long contentLength, StreamVariable streamVariable,
532+
OutputStream out, long totalBytes, final Exception exception) {
533+
tryToCloseStream(out);
534+
session.lock();
535+
try {
536+
streamVariable.streamingFailed(new StreamingErrorEventImpl(filename,
537+
type, contentLength, totalBytes, exception));
538+
} finally {
539+
session.unlock();
540+
}
541+
}
542+
534543
private long updateProgress(VaadinSession session,
535544
StreamVariable streamVariable,
536545
StreamingProgressEventImpl progressEvent, long lastStreamingEvent,

flow-server/src/test/java/com/vaadin/flow/server/communication/StreamReceiverHandlerTest.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import com.vaadin.flow.server.StreamReceiver;
3535
import com.vaadin.flow.server.StreamResourceRegistry;
3636
import com.vaadin.flow.server.StreamVariable;
37+
import com.vaadin.flow.server.UploadException;
3738
import com.vaadin.flow.server.VaadinRequest;
3839
import com.vaadin.flow.server.VaadinResponse;
3940
import com.vaadin.flow.server.VaadinServlet;
@@ -72,6 +73,8 @@ public class StreamReceiverHandlerTest {
7273
private StreamReceiver streamReceiver;
7374
@Mock
7475
private StreamResourceRegistry registry;
76+
@Mock
77+
private ErrorHandler errorHandler;
7578

7679
private VaadinServletService mockService;
7780

@@ -403,4 +406,32 @@ public void partsAreUsedDirectlyIfPresentWithoutParsingInput()
403406
ApplicationConstants.CONTENT_TYPE_TEXT_HTML_UTF_8);
404407
Mockito.verify(response, Mockito.times(0)).setStatus(Mockito.anyInt());
405408
}
409+
410+
@Test
411+
public void handleFileUploadValidationAndData_inputStreamThrowsIOException_exceptionIsNotRethrown_exceptionIsNotHandlerByErrorHandler()
412+
throws UploadException {
413+
InputStream inputStream = new InputStream() {
414+
415+
@Override
416+
public int read() throws IOException {
417+
throw new IOException();
418+
}
419+
};
420+
handler.handleFileUploadValidationAndData(session, inputStream,
421+
streamReceiver, null, null, 0, stateNode);
422+
423+
verifyZeroInteractions(errorHandler);
424+
verify(streamVariable).streamingFailed(Mockito.any());
425+
}
426+
427+
@Test
428+
public void doHandleMultipartFileUpload_IOExceptionIsThrown_exceptionIsNotRethrown_exceptionIsNotHandlerByErrorHandler()
429+
throws IOException, ServletException {
430+
VaadinServletRequest request = Mockito.mock(VaadinServletRequest.class);
431+
Mockito.doThrow(IOException.class).when(request).getParts();
432+
handler.doHandleMultipartFileUpload(session, request, response,
433+
streamReceiver, stateNode);
434+
verifyZeroInteractions(errorHandler);
435+
}
436+
406437
}

0 commit comments

Comments
 (0)