-
-
Notifications
You must be signed in to change notification settings - Fork 325
fix: Cloud files throwing error on upload #685
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -170,8 +170,26 @@ class AjaxUploader extends Component<UploadProps> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uploadFiles = (files: File[]) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const originFiles = [...files] as RcFile[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cacheFiles = async (files: File[]) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (files?.length) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cachedFiles = await Promise.all( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| files.map(async file => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const buffer = await file.arrayBuffer(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new File([buffer], file.name, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: file.type, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lastModified: file.lastModified, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+178
to
+181
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new File([buffer], file.name, { | |
| type: file.type, | |
| lastModified: file.lastModified, | |
| }); | |
| const clonedFile = new File([buffer], file.name, { | |
| type: file.type, | |
| lastModified: file.lastModified, | |
| }); | |
| // Preserve additional own properties (e.g. webkitRelativePath) from the original File | |
| const ownProps = Object.getOwnPropertyNames(file); | |
| ownProps.forEach(prop => { | |
| // Skip properties already handled by the File constructor | |
| if (prop === 'name' || prop === 'type' || prop === 'lastModified' || prop === 'size') { | |
| return; | |
| } | |
| try { | |
| const desc = Object.getOwnPropertyDescriptor(file, prop); | |
| if (desc) { | |
| // Avoid redefining non-configurable properties on the cloned file | |
| Object.defineProperty(clonedFile, prop, desc); | |
| } else { | |
| (clonedFile as any)[prop] = (file as any)[prop]; | |
| } | |
| } catch { | |
| // Silently ignore properties that cannot be reassigned (e.g. read-only, non-configurable) | |
| } | |
| }); | |
| return clonedFile; |
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cacheFiles method lacks documentation explaining its purpose and why files need to be cached via arrayBuffer. Consider adding a JSDoc comment explaining that this is needed to prevent errors when uploading files from cloud storage, as those files may become inaccessible after the initial file selection event.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cacheFiles method lacks error handling for the arrayBuffer() operation. If arrayBuffer() fails for any file (e.g., due to file access issues), the entire Promise.all will reject and no files will be uploaded. Consider adding try-catch around the arrayBuffer call to handle failures gracefully, either by skipping problematic files or logging errors.
Copilot
AI
Jan 5, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uploadFiles method has been changed to async but none of its call sites await it (lines 64, 106, 114). This means errors thrown in cacheFiles or during the async upload process won't be caught properly and could result in unhandled promise rejections. The calls should either be awaited with proper error handling, or uploadFiles should handle errors internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other file attributes would be lost.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve attached a screenshot after printing the original and cloned File objects, and I don’t see any difference in their attributes. Please let me know if I’m missing something.