Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Upload succeeds even if the receiver throws exception #145

Closed
mvysny opened this issue Aug 1, 2019 · 5 comments · Fixed by vaadin/flow#6381
Closed

Upload succeeds even if the receiver throws exception #145

mvysny opened this issue Aug 1, 2019 · 5 comments · Fixed by vaadin/flow#6381
Assignees

Comments

@mvysny
Copy link
Member

mvysny commented Aug 1, 2019

Vaadin 13.0.11, Firefox 68.0.1

I have a very simple multi-file Upload component, configured in a way that Receiver.receiveUpload() always fails (always throws an exception). However, when I try to upload the files, the Upload component shows all uploads as successful (shows a check mark along all uploaded files in the browser). Also, the upload HTTP POST request ends with 200 OK, while it should end with 500 INTERNAL SERVER ERROR.

Attaching a screenshot.
Screenshot from 2019-08-01 16-55-35

@mvysny
Copy link
Member Author

mvysny commented Aug 1, 2019

However, the server-side log contains lots of exceptions, as it should:

[http-nio-8080-exec-1] ERROR com.vaadin.flow.server.DefaultErrorHandler - 
com.vaadin.flow.server.UploadException: Upload failed
	at com.vaadin.flow.server.communication.StreamReceiverHandler.streamToReceiver(StreamReceiverHandler.java:486)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.handleFileUploadValidationAndData(StreamReceiverHandler.java:324)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.handleStream(StreamReceiverHandler.java:253)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.handleMultipartFileUploadFromInputStream(StreamReceiverHandler.java:226)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.doHandleMultipartFileUpload(StreamReceiverHandler.java:177)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.handleRequest(StreamReceiverHandler.java:135)
	at com.vaadin.flow.server.communication.StreamRequestHandler.handleRequest(StreamRequestHandler.java:96)
	at com.vaadin.flow.server.VaadinService.handleRequest(VaadinService.java:1507)
	at com.vaadin.flow.server.VaadinServlet.service(VaadinServlet.java:242)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:200)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:490)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
	at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:678)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:408)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:836)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1839)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.RuntimeException: Foo
	at com.vaadin.starter.skeleton.MainView$1.receiveUpload(MainView.java:26)
	at com.vaadin.flow.component.upload.Upload$DefaultStreamVariable.getOutputStream(Upload.java:592)
	at com.vaadin.flow.server.communication.StreamReceiverHandler.streamToReceiver(StreamReceiverHandler.java:415)
	... 31 more

@mvysny
Copy link
Member Author

mvysny commented Aug 1, 2019

example-project.zip

@pleku pleku added this to Needs triage in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) via automation Aug 9, 2019
@caalador caalador moved this from Needs triage to P1 - High priority in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Aug 16, 2019
@pleku pleku added the bug label Aug 23, 2019
@denis-anisimov denis-anisimov moved this from P1 - High priority to WIP in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Sep 2, 2019
@denis-anisimov denis-anisimov self-assigned this Sep 2, 2019
@denis-anisimov
Copy link

The example is a bit artificial since the exception is thrown in the OutputStream getter.
Normally the exception is thrown on write/read.
So more proper example is :

Upload upload = new Upload(new Receiver() {
            @Override
            public OutputStream receiveUpload(String fileName,
                    String mimeType) {
                return new OutputStream() {

                    @Override
                    public void write(int b) throws IOException {
                        throw new RuntimeException("Foo");
                    }
                };
            }
        });

Here the exception is thrown during write into the OutputStream instance.
That what may happen during a file upload.
And the issue is the same: the server side doesn't notify the client side anyhow.

@denis-anisimov
Copy link

It looks like server side doesn't affect the client side anyhow in any situation.
At the moment the only way to fail upload is some logic in the client side which may decide that something goes wrong.
There is no code on the server side which somehow notify the client side that upload has failed.

So we need either API on the client side which allows to notify an upload error or the client side should somehow fail itself in a better way. E.g. when the stream is closed prematurely: the number of written bytes is less than the file size....
I'm not sure wether it's possible though.
We need help from components team...

@denis-anisimov
Copy link

As it's mentioned in the original description the response status is always 200. But it should return 500 in case of server side exception.
In that case the client side shows an error.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants