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

File → menu → Download buffers entire file into a blob #1887

Closed
ZeroCool22 opened this issue Jan 9, 2022 · 7 comments · Fixed by #1894
Closed

File → menu → Download buffers entire file into a blob #1887

ZeroCool22 opened this issue Jan 9, 2022 · 7 comments · Fixed by #1894
Assignees
Labels
area/screen/files Issues related to Files screen kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up released status/blocked Unable to be worked further until needs are met

Comments

@ZeroCool22
Copy link

  • OS: Windows 10 x64
  • Version of IPFS Desktop [0.18.1]

Describe the bug

https://streamable.com/9z175t

To Reproduce
Steps to reproduce the behavior:

  1. Go to the file
  2. Click on more
  3. Scroll down to Download
  4. Blank Screen after some seconds.

Log files:

combined.log
error.log

PD: I tried reinstalling, but same problem.

@hacdias
Copy link
Member

hacdias commented Jan 10, 2022

Hi @ZeroCool22! Thanks for the bug report. I could not reproduce it, which makes me think it may be related to RAM memory. I see you have 32 GB of RAM and that it reaches ~60% usage before crashing. How large is the file? Could you also try checking the developers console when it happens (CTRL+SHIFT+I)?

@ipfs ipfs deleted a comment Jan 10, 2022
@ZeroCool22
Copy link
Author

Hi @ZeroCool22! Thanks for the bug report. I could not reproduce it, which makes me think it may be related to RAM memory. I see you have 32 GB of RAM and that it reaches ~60% usage before crashing. How large is the file? Could you also try checking the developers console when it happens (CTRL+SHIFT+I)?

Here you go:

https://streamable.com/u0kchy

@ZeroCool22
Copy link
Author

No help?

I really want to use IPFS... :(

@lidel
Copy link
Member

lidel commented Jan 11, 2022

@ZeroCool22 thank you for recording the demo, it is very useful.
We will fix this, but for now you need to use a workaround: avoid using the "Download" ("Descarga") action from menu, and click on "Local gateway" instead:

2022-01-11_23-25

Let us know if this works with your 7GB file.


@hacdias my understanding of the problem is that we need to change how "Download" context action of a single file is implemented on the Files screen. It reads correct URL from doFilesDownloadLink but then downloads it using downloadFile function from src/files/download-file.js, which buffers the entire thing in memory to construct a blob:.

I believe creating a blob via window.URL.createObjectURL in downloadFile was necessary because we did not have ?download=true four years ago (was introduced in 2020 in ipfs/kubo#7677), and we also displayed progress bar or something.

Today it is not necessary, and as we can see harms UX when a big file is downloaded. We should remove blob freation and leverage regular URL with added download=true parameter instead. Code for tracking download progress also should be removed – browser will handle that.

Will you have time to submit a fix?

@lidel lidel changed the title Jan 11, 2022
@lidel lidel transferred this issue from ipfs/ipfs-desktop Jan 11, 2022
@lidel lidel added area/screen/files Issues related to Files screen kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up labels Jan 11, 2022
@lidel lidel added this to Weekly Candidates in Maintenance Priorities - JS Jan 11, 2022
@ipfs ipfs deleted a comment from welcome bot Jan 11, 2022
@hacdias
Copy link
Member

hacdias commented Jan 14, 2022

@lidel I think one of the issues is the directory: there's no way of downloading a directory via the gateway and the API is POST only (unless that changed recently)

Ref ipfs/kubo#7746

@lidel
Copy link
Member

lidel commented Jan 16, 2022

@hacdias afaik /api/v0/get is also exposed on Gateway port, and there it should accept GET (POST restriction applies only to the RPC API port).
I think is fine to use that until we have a better way for fetching directories on Gateways.

@lidel lidel moved this from Weekly Candidates to In Progress in Maintenance Priorities - JS Jan 21, 2022
@lidel lidel moved this from In Progress to Parked/Blocked in Maintenance Priorities - JS Feb 18, 2022
@lidel lidel added the status/blocked Unable to be worked further until needs are met label Feb 18, 2022
@BigLep BigLep removed this from Parked/Blocked in Maintenance Priorities - JS Sep 9, 2022
@ipfs-gui-bot
Copy link
Collaborator

🎉 This issue has been resolved in version 2.21.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/screen/files Issues related to Files screen kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up released status/blocked Unable to be worked further until needs are met
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants