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.

Sign up for GitHub

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

Jump to bottom

[Bug] Can't download large files #266

Closed
Raptime opened this issue May 23, 2023 · 20 comments · Fixed by #286
Closed

[Bug] Can't download large files #266

Raptime opened this issue May 23, 2023 · 20 comments · Fixed by #286
Assignees
@gantoine
Labels
bug Something isn't working

Comments

@Raptime
Copy link

Raptime commented May 23, 2023

RomM version
1.8.4

Describe the bug
downloading big ROM (2GB+) doesn't work. small ROM (20MB) download fine

To Reproduce
Steps to reproduce the behavior:

  1. Select Switch as a platform
  2. Download a game
  3. It says "Downloading xxx..."
  4. Nothing happens

Expected behavior
ROM should start downloading, just like when you download a smaller ROM

Additional context
docker logs -f romm doesn't show any error
@Raptime Raptime added the bug Something isn't working label May 23, 2023
@gantoine
Copy link
Member

I'm able to download a 10GB+ file just fine, so my guess is the request is timing out when trying to prepare the file for download. Could you open the developer console, navigate to the Network tab, try to download the file and post the result/headers of the request?

Screenshot 2023-05-23 at 11 03 52 AM

@Raptime
Copy link
Author

Raptime commented May 24, 2023

Looks like a timeout indeed. I'm using Romm in a docker container and I mount an NFS drive for the ROMs, could be why...
image
image

@gantoine
Copy link
Member

So all the posts I can find about NS_BASE_STREAM_CLOSED speak of a caching issue in Firefox. I'm gonna try the same thing on my hosted instance with another 10GB file. In the meantime, can you let me know if there are any errors in the console?

@Raptime
Copy link
Author

Raptime commented May 25, 2023 •
edited

The following error in the console appears after the timeout (sorry, I should have included it in my earlier reply):

Uncaught (in promise) DOMException: The operation was aborted. download.65cfb14c.js:12:93650
    Ft http://docker.home.example.org:8123/assets/download.65cfb14c.js:12
    AsyncFunctionThrow self-hosted:814
    (Async: async)
    forEach self-hosted:203
    Ft http://docker.home.example.org:8123/assets/download.65cfb14c.js:12
    0 http://docker.home.example.org:8123/assets/Details.30da469b.js:1
    Sm http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    Qt http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    ct http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    ct http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    n http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    (Async: EventListener.handleEvent)
    Nn http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    pm http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    Cm http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    I http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    B http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    g http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    D http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    run http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    update http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    ee http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    K http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    U http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    g http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    w http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    I http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    B http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    g http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    D http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    run http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    update http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    ee http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    K http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    U http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    g http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    w http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    I http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    B http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    g http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    D http://docker.home.example.org:8123/assets/index.83f1f892.js:1
    run http://docker.home.example.org:8123/assets/index.83f1f892.js:1

I will try using a different browser and report

Edit: same thing using Chrome, it times out. Must be something wrong with my set up.

@VermiumSifell
Copy link

Regarding "https://github.com/zurdi15/romm/blob/master/frontend/src/services/download.js" is it necessary to do this part?

        var file = await fetch(file_full_path)
        var fileBlob = await file.blob()
        saveAs(fileBlob, rom.file_name)

Can't be just send the file right away without making it a blob and use "saveAs" function? Can't we just in some way return the file? I'm not an expert at all on managing files in JavaScript. But I also think this part should be a part of backend and listen for progress from backend and then when file is ready (if multiple files, it may take some time, otherwise just send it right away). I have no idea if my idea is something that is better (or work) but just wanted to try help improving download part

@gantoine
Copy link
Member

@VermiumSifell You're not far off; we can easily handle downloading single-file roms, or ones where the parts are already zipped. The issue lies with multi-file ROMs, since they'd still require some form of on-the-fly compression.

@gantoine gantoine assigned gantoine and unassigned zurdi15 Jun 11, 2023
@gantoine
Copy link
Member

Alright I've got something working for both single file and multi-file download https://github.com/GAntoine/romm/pull/1

It's based off my pagination PR, so that'll need to go in first before this can get merged.

cc @zurdi15

@VermiumSifell
Copy link

@VermiumSifell You're not far off; we can easily handle downloading single-file roms, or ones where the parts are already zipped. The issue lies with multi-file ROMs, since they'd still require some form of on-the-fly compression.

Cool, btw cant we just send the files individually?

@gantoine
Copy link
Member

cant we just send the files individually

We can but then we go back to the original issue, which is downloading using fetch, and will require the file to be built in memory before it's downloaded.

@VermiumSifell
Copy link

Can we change from using fetch to something else? I think some games are really huge and not all people can give their romm 16GB+ RAM

@gantoine
Copy link
Member

If you can think of another way to fetch multiple files without a direct-download link I'm all ears!

@zurdi15
Copy link
Member

zurdi15 commented Jun 12, 2023

Alright I've got something working for both single file and multi-file download GAntoine#1

It's based off my pagination PR, so that'll need to go in first before this can get merged.

cc @zurdi15

I Just saw it. There are some things I want to fix from the current PR but after that I agree on changing the download feature to the backend, but I am a little bit noob with this here, so I am all ears too!

@gantoine
Copy link
Member

There are some things I want to fix from the current PR

By all means feel free to modify it, or comment with suggestions of things we can improve (on either of them).

@VermiumSifell
Copy link

If you can think of another way to fetch multiple files without a direct-download link I'm all ears!

Altough, why not use a direct-link? Is it bad in some way? Since we do a "GET" request we can make sure it is not executed, and therefor get all files individually which should improve performance of downloads, aswell as showing progress for user a lot better. Maybe we can implement a API endpoint with ratelimits to the files? And stream the file to the user instead?

@VermiumSifell
Copy link

Btw, I'm currently trying to rewrite whole backend over to NodeJS, with async/await and TypeScript. I hope I can make a PR of it later, otherwise I likk just make it as a fork of romm :)

@gantoine gantoine mentioned this issue Jun 19, 2023
Merged
@VermiumSifell
Copy link

Btw, I'm currently trying to rewrite whole backend over to NodeJS, with async/await and TypeScript. I hope I can make a PR of it later, otherwise I likk just make it as a fork of romm :)

Most stuff rewritten now. screenshots and some minor fixes left!
image

@zurdi15
Copy link
Member

zurdi15 commented Jun 30, 2023

Wow dude you are fast. Sorry for the late response, I am sadly out of time right now to work too much on this, and didn't think too much about this NodeJS refactor of the backend.

I am not an expert in NodeJS but I don't want RomM to be so fragmented this early in the development having two different backends or having a fork, so If @gantoine agree with this (as he is the main contributor other than me) we can discuss to having the full backend replacement in the main fork.

What is the advantage of doing it in NodeJS against doing it with python fastapi?

@gantoine
Copy link
Member

So I've worked with both Python/Fast and Node, and I find both to be perfectly capable frameworks, especially at the scale of a project like this one. Type safety is useful even on the backend, but we don't need to switch to a Node to reap those benefits (python > 3.5 has built-in typing).

@zurdi15 Are you familiar/have you worked with with Node (or other JS-on-the-backend)?

@VermiumSifell What's your motivation for this change? And if we do decide to make the switch at some point, we should consider Node alternatives like Deno or Bun.

@VermiumSifell
Copy link

Mostly because I know of NodeJS a lot more, and find TypeScript amazing, I think of keeping this as a fork of romm, my motivation is to improve the download mechanism and I wanted to rewrite it to NodeJS too. So just something I prefer :P

@gantoine
Copy link
Member

Yeah that's totally understandable, we like to work with what we know. 😄 That being said, unless we can come up with quantifiable benefits to migrating off FastAPI/Python, IMO our time would be better spend improving the current codebase. Node and Python are about equal in performance, have a lot of developers who know the languages/frameworks, support async/concurrency, and support typing.

While your contributions to this project would be greatly appreciated, you're always welcome to fork and maintain a version written in Node. If you happen to have time on your hands, and your fork outpaces this project in terms of functionality over the next weeks/months, we should absolutely revisit the idea of migrating over.

Oh and please let me know when your rewrite is available, I'd love to take a look through the code!

@zurdi15 zurdi15 mentioned this issue Aug 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gantoine gantoine

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow for direct rom download + server-size ZIP
4 participants
@gantoine @Raptime @zurdi15 @VermiumSifell