Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve upload error handling #11

Open
mzur opened this issue Jun 20, 2022 · 17 comments · May be fixed by #30
Open

Improve upload error handling #11

mzur opened this issue Jun 20, 2022 · 17 comments · May be fixed by #30
Assignees

Comments

@mzur
Copy link
Member

mzur commented Jun 20, 2022

Improve the upload error handling as follows: If there is an error for an uploaded file, show a popup displaying the error (and the filename). The popup should offer an option to cancel the upload or to skip this file and continue with the remaining files.

@mzur mzur added the student label Jun 20, 2022
@mzur
Copy link
Member Author

mzur commented Oct 12, 2022

In addition: If there is an error with an uploaded file, retry the upload after 5 s (?). If this also fails, show the error. This can be helpful if the BIIGLE instance is updated during an upload und unavailable for a few seconds.

@mzur mzur removed the student label Oct 20, 2022
@mzur
Copy link
Member Author

mzur commented Apr 5, 2024

@lehecht Can you please have a look at this sometime? I see quite a few errors where file uploads abort because the object storage temporarily doesn't work. A simple retry and better error handling would be nice. The retry should also be done for chunked large files.

@lehecht
Copy link
Contributor

lehecht commented Apr 9, 2024

In addition: If there is an error with an uploaded file, retry the upload after 5 s (?).

There is already a mechanism to retry the image upload. Should I replace this with your idea or maybe just increase the number of retries?
Currently RETRY_UPLOAD = 2 is used.

@mzur
Copy link
Member Author

mzur commented Apr 9, 2024

This issue is about the client-side upload. It works like this:

Client -> Server -> Storage

The retry you linked is from Server to Storage. This issue is about a retry and better error handling from Client to Server. Maybe it has a retry mechanism, too (I don't remember and didn't look), but the "skip or cancel" popup should also be added.

@mzur
Copy link
Member Author

mzur commented Apr 9, 2024

On second thought, a popup immediately after the error might not be the ideal solution. I imagine sometimes people just let large volumes of data upload unsupervised (e.g. over night). Then they come back in the morning and see an error after the first file and the popup, so the rest of the files were not uploaded. A better way would be to automatically skip files with errors and then at the end (when all files without errors were uploaded) show a popup asking to retry or skip the failed files.

@lehecht
Copy link
Contributor

lehecht commented Apr 9, 2024

The retry you linked is from Server to Storage. This issue is about a retry and better error handling from Client to Server. Maybe it has a retry mechanism, too (I don't remember and didn't look), but the "skip or cancel" popup should also be added.

What I meant was, if I should replace the number of retries by the 'retry after 5 s'-idea (on the client side).

@mzur
Copy link
Member Author

mzur commented Apr 9, 2024

Is there a retry mechanism on the client side then? The Server to Storage retry mechanism should not be changed. The client side retry mechanism can have the 5 s delay.

@lehecht
Copy link
Contributor

lehecht commented Apr 9, 2024

Is there a retry mechanism on the client side then?

Yes, there it uses also 2 retries.

@lehecht
Copy link
Contributor

lehecht commented Apr 9, 2024

The client side retry mechanism can have the 5 s delay

So should I replace the former mechanism or add this delay?

@mzur
Copy link
Member Author

mzur commented Apr 9, 2024

I see, thanks. Please add the delay to the existing mechanism and increase the number of retries to 3. Then add the mechanism of first skipping failed uploads and finally asking to retry or skip all failed files described above. You can create two separate PRs for this, so the first change is published more quickly.

@lehecht
Copy link
Contributor

lehecht commented Apr 10, 2024

@mzur

[...] show a popup asking to retry or skip the failed files.

What do you think about this alternative to a popup? I saw something similar for already initialized files that were not uploaded.
But if you still prefer a popup, I'll use one.
Bildschirmfoto vom 2024-04-10 10-13-27

@mzur
Copy link
Member Author

mzur commented Apr 11, 2024

I like your idea! Some additional thoughts:

  • Maybe the failed files can also be highlighted with text-warning
  • The "Add directory, ..., Submit" buttons should disappear as they no longer make sense in this context
  • The "Re-upload, ..." buttons can have normal size
  • The "Re-upload" button can have btn-success as this is the recommended option
  • The whole message with buttons could be displayed inside a:
    <div class="panel panel-warning">
       <div class="panel-body text-warning>
       </div>
    </div>
    
    This is the box that BIIGLE also uses elsewhere.
  • Alternative button labels could be "retry failed files" and "skip failed files" to make the actions very clear.

@lehecht
Copy link
Contributor

lehecht commented Apr 19, 2024

@mzur

Maybe the failed files can also be highlighted with text-warning

Should I extend the fileBrowser component and its used components or apply the changes directly in these core files?

@mzur
Copy link
Member Author

mzur commented Apr 19, 2024

It's probably easier to update the core files, right? You can add a new optional property warningFiles or some such as an array of files that should be highlighted. If the prop is not given, the file browser behaves as before.

@lehecht
Copy link
Contributor

lehecht commented Apr 19, 2024

@mzur

How should I handle failed files with very long file names? It could happen, that the "failed" info is not (completly) visible anymore because of the box size.
Bildschirmfoto vom 2024-04-19 10-59-17

@mzur
Copy link
Member Author

mzur commented Apr 19, 2024

I suggest that you just change the color of the whole filename. Instead of showing "failed", you could replace the file icon with something else, e.g. a warning sign.

@mzur
Copy link
Member Author

mzur commented Apr 19, 2024

Also the directory tree above the failed file should be colored (but without warning icon).

@lehecht lehecht linked a pull request May 21, 2024 that will close this issue
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Medium Priority
Development

Successfully merging a pull request may close this issue.

2 participants