Update file upload so that it has better error support. #8427
Conversation
| @@ -80,4 +80,14 @@ public static String readLastLines(File file, int lines) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| public static String humanizeFileSize(long bytes) { | |||
There was a problem hiding this comment.
Check if could be used: https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FileUtils.html#byteCountToDisplaySize(long).
But I see that it's implemented in Javascript a similar function, so probably good to keep them aligned.
The calculation doesn't seem accurate, the original exception message is Maximum upload size of 100000000 bytes exceeded
Also the message looks not good: Payload Too LargeMaximum upload size...
There was a problem hiding this comment.
I added javadocs explaining that this was mostly picked due to the extra decimal places.
If we don't want to support new code and prefer to round to the nearest MB then I'm fine with removing this and using the apache commons version.
Regarding the error message, this seems to be caused by the data.errorThrown being prepended to the error message. Since we translate the other messages we probably don't want to add non translated and translated text at the same time so I moved the assignment of data.errorThrown at the end only in cases where the message was still empty. This should soft your issue.
| public ApiError maxFileExceededHandler(final Exception exception, final HttpServletRequest request) { | ||
| Exception ex; | ||
| long contentLength = request.getContentLengthLong(); | ||
| // As MaxUploadSizeExceededException is a sping exception, we need to convert it to a localized exception so that it can be translated. |
There was a problem hiding this comment.
| // As MaxUploadSizeExceededException is a sping exception, we need to convert it to a localized exception so that it can be translated. | |
| // As MaxUploadSizeExceededException is a Spring exception, we need to convert it to a localized exception so that it can be translated. |
| @@ -91,30 +91,76 @@ | |||
| } | |||
| }); | |||
|
|
|||
| var humanizeDataSize = function (bytes) { | |||
There was a problem hiding this comment.
You need to build the project with mvn clean install, so Prettier formatting is applied to Javascript files.
There was a problem hiding this comment.
I'm not sure I understand this comment? Are you implying that most developers will not do a mvn clean install?
There was a problem hiding this comment.
@ianwallen if you build the project locally you should see web-ui/src/main/resources/catalog/components/filestore/FileStoreDirective.js with changes due to Prettier maven plugin.
It seems some changes have been done in the file after doing a mvn install.
You just need to run that and commit the changes.
There was a problem hiding this comment.
OK - I understand -I have pushed the pretty print changes.
ianwallen
force-pushed
the
file_upload_message
branch
from
b9d1074
to
84c4ea4
Compare
- Add bilingual error message. - Fix queue removal so that it only remove items that are complete instead of fully clearing the queue. - Fix error message Uncaught TypeError: Cannot read properties of undefined (reading 'message') - Catch status 0 which is generally a network error and display an error message - Catch status 413 which may come from a proxy server with no messages - it now displays correct error message. - Add humanizeFileSize so that users can see errors in nice format instead of long byte format.
ianwallen
force-pushed
the
file_upload_message
branch
from
84c4ea4
to
2d97711
Compare
Before
After
If the proxy such as nginx returns 413 File too large then the following is displayed
And if the upload times out or connection is closed while uploading then the following will be displayed.
Checklist
mainbranch, backports managed with labelREADME.mdfilespom.xmldependency management. Update build documentation with intended library use and library tutorials or documentation