search

tarekoraby / upload-download-files

A web app that enables users to upload and download files to and from the server.
The Unlicense
3 stars 1 forks source link

lazy download with feedback on error #1

Open nittka opened 3 years ago

nittka commented 3 years ago

Just in case you want to enhance your example. We are still looking for the canonical solution meeting the following demands

Because if an exception is thrown during the actual download by the resource factory, no UI is present for notifying the user. The error handler can log on the server side. But without feedback the user might try downloading again and again.

Our current code is based on https://vaadin.com/forum/thread/17061112/download-link-or-button

On button click, we validate and prepare the complete byte stream in memory in order to be able to show a notification in case of errors, then we register the resource and (do not set the page location because afterwards all navigation in the application is broken) open it in a new window. We can do so because we have few users and only small downloads. But I have the feeling that there must be a better way - the file download wrapper seems not to address the issue either.

tarekoraby commented 3 years ago

The solution in the forum would have been my best idea for your use case. But you seem to suggest that this solution has limitations. I didn't get what you mean. Can you elaborate?

nittka commented 3 years ago

Thanks for the quick feedback and confirming that the post contains an up-to-date example. The problem, I was hinting at may well be caused by adaptions we made in our application.

What are our requirements for the download:

So far we did not manage to handle exceptions inside the InputStreamFactory as we'd like. The factory code is invoked during a vaadin request without a UI being available, so the error handler can log an exception on the server side but it cannot show a (general) error notification. We currently solve this by copying the download content on button click, handling potential exceptions "outside" the stream factory. So we create the notification on button click and do not register a resource (due to the exception). Of course, this is not really lazy resource access.

The navigation problem may be due our adapted navigation implementation. We wanted our code for creating the menu to be easily readable and generic, so all menu and submenu entries are always created. Framework code then checks user role and permissions to remove entries again (including removing the main menu entry if all sumbenu entries have been removed) if permissions are missing. The view classes are annotated with role and permission information. So you can simply add a view to some sumbenu without having to worry about checking these permissions manually (problem of top-down menu creation). It is well possible that the code we use here interferes with the page.setLocation for starting the download. So what did I mean by navigation is broken...

Using the page.setLocation-code after registering the download resource what happens in our application is that once a download is invoked, no other download can be started (there are several download buttons using the same code pattern) and clicking on any menu/submenu has no effect at all. It seems that using page.setLocation breaks all client/server-connection (in our application) and you have to do a page reload to get them synchronized again.

I did not yet have time to reduce everything to a minimal example to check what causes which behaviour.

tarekoraby commented 3 years ago

The factory code is invoked during a vaadin request without a UI being available

I guess this is the part that confuses me the most about your use case. I don't understand what you mean by a UI being unavailable. To my mind, there must be a UI instance that represents what the user sees on their currently open window tab (which should be obtainable via UI.getCurrent()).

nittka commented 3 years ago

When I debugged the error handler (after throwing an exception within the input stream factory) UI.getCurrent() returned null. In fact, our error handler always checks for a UI that way and shows a generic error message, if it finds one. So far, this never posed a problem, but when migrating from Vaadin 8 to Vaadin 14 (which I might have mentioned earlier), errors when downloading a file went by silently without the user getting any feedback.

I will have to double check (currently working on other issues, I just saw the blog post and hoped for hints indicating what we might be doing wrong).

nittka commented 3 years ago

I reproduced the behaviour in https://github.com/tarekoraby/Vaadin14-demos modifying the AppAdminView

@Route(value = "AppAdminView", layout = AppMainView.class)
public class AppAdminView extends VerticalLayout {

    private static final Logger logger = LoggerFactory.getLogger(AppAdminView.class);

    public AppAdminView() {

        //when pressing this button a "contact the support" notification is shown
        add(new Button("Red button", e -> {
            throw new IllegalStateException("Red buttons must not be pressed");
        }));

        Button button = new Button("Download", event -> {
            logger.info("Download button was pressed");
            //use a zip file so the download dialog is opened instead of a new page with the dynamic resource location
            final StreamResource resource = new StreamResource("foo.zip",
                () -> {
                    //Notification.show("Stream factory was called"); //causes exception
                    logger.info("Stream factory was called");
                    logger.info("current ui is: " + UI.getCurrent());
                    //when throwing this exception, a stacktrace is shown in the console once
                    //clearing the console and pressing the button again has no effect!
                    //NO "contact the support" notification is shown to the user
                    //because UI.getCurrent() is null (see above logging or debug CustomExceptionHandler)
                    throw new IllegalStateException("something went wrong with the download");
                    //                  return new ByteArrayInputStream("foo".getBytes());
                });
            final StreamRegistration registration = VaadinSession.getCurrent().getResourceRegistry()
                .registerResource(resource);
            UI.getCurrent().getPage().setLocation(registration.getResourceUri());
        });
        add(button);

    }
}

I assume that our problems with the navigation have the same underlying cause as the second point. Whereas in the demo application the navigation is achived using RouterLinks, we use events. The download changes the page location. The browser looks the same, but no events are fired anymore.