arXiv / arxiv-submission-ui

User interface of NG submit system.
MIT License
2 stars 6 forks source link

ARXIVNG-2881: add error message about large file uploads #167

Closed mhl10 closed 4 years ago

mhl10 commented 4 years ago

This PR:

Screenshot 2020-01-27 14 21 25
SBBCornell commented 4 years ago

Quick review based on the screenshot: it does look like some messages meant for screen readers only are displaying. I'll review more deeply...

DavidLFielding commented 4 years ago

Code looks fine. I was originally going to skip actually testing this but decided to take time for a quick test.

My problem was with base. After fixing the UI catches large files without ugly 500 error. Looks good!

Side note: Changing base seems a little tedious. I'm not sure if there is a way to simplify this. Should we be doing this system-wide?

DavidLFielding commented 4 years ago

I'm running against the latest develop branch which contains the file upload error improvements from this PR.

I'm only seeing the red error box at the top of the page when I upload a massive file. I'm not seeing red warning box down below or any details about the actual error. Am I supposed to see something about the uploaded file being too large?

I'm not getting a 500 error, which is good, but I have no idea why my upload failed. Well, I know, but the system is not reminding me.[Correction: I was just able to generate a 500 error while uploading a large file but I'm not sure what happened yet. Looks like some sort of read timeout where I see timeout value set to 30 seconds.]

arxiv-submission-ui | raise ReadTimeoutError(self, url, "Read timed out. (read timeout=%s)" % timeout_value) arxiv-submission-ui | urllib3.exceptions.ReadTimeoutError: HTTPConnectionPool(host='arxiv-filemanager', port=8000): Read timed out. (read timeout=30)

Hmm. I'm also able to upload several large files for a total of 60MB without errors. I believe we relaxed the lower legacy oversize limits. This seems off. I will need to double check.

mhl10 commented 4 years ago

I'm not getting a 500 error, which is good, but I have no idea why my upload failed. Well, I know, but the system is not reminding me

It looks like you're running in to the case where the ConnectionResetError is being caught, which is expected. I wasn't sure it was safe to assume that is this always caused by a large file--hence the generic error message. In the production environment we should be catching the RequestEntityTooLarge HTTP error, which is accompanied by a more specific error message.