Skip to content

Conversation

@scottopell
Copy link

Fix for #286

Current state:

  • Add a timeout for http requests
  • Use timeout in UploadButton and correct types
  • File Selection modal should detect that no ListTorrentResponse is present and only present file destination picker

@ikatson
Copy link
Owner

ikatson commented Dec 2, 2024

Hmm, I think doing it in the backend is the only correct way. The backend has a "timeout" query parameter that you can pass, and then you don't need to make such a heavy change to UI code. This is the first thing I would do. Although I'm not sure what a reasonable default would be. I'm fine having NO timeout by default. So if you need it, maybe let the user configure it in UI when adding a torrent? E.g. the user clicks "add torrent through magnet", and in the dialog there's an option to set the timeout, which by default is empty.

But a better change would be to add a new way of adding torrent to the backend - this will spawn a tokio task to add a torrent with a larger timeout. The problem is that it won't show up in UI at all until resolved, and won't survive restarts.
For it to do both, we need a new torrent state, or maybe a separate store for torrents that are "being resolved".

@ikatson
Copy link
Owner

ikatson commented Dec 2, 2024

The backend has a "timeout" query parameter that you can pass, and then you don't need to make such a heavy change to UI code.

Uh, I remember writing this code, but I don't see it in main. I guess I never merged it in

Edit: it's here, unmerged https://github.com/ikatson/rqbit/compare/http-timeout?expand=1

@ikatson
Copy link
Owner

ikatson commented Dec 2, 2024

Btw, if you add torrent without listing, it will still "block" the request until the magnet it resolved. And I guess if you close the browser window, it will cancel the request, which will cancel the future that was adding the torrent.

So the only reasonable way to do it is to spawn a task that would do it all. But then you'd need something in the backend to prevent spawning futures for adding the same torrents. For showing them in the UI and restoring that during restarts.

All this TBH smells like a moderately sized backend change that must be made first without touching the UI. So I'd pause for now and design this upfront in more detail before making the changes

@ikatson
Copy link
Owner

ikatson commented Dec 5, 2024

All this TBH smells like a moderately sized backend change that must be made first without touching the UI. So I'd pause for now and design this upfront in more detail before making the changes

FYI I'm trying to do this backend change and it's quite large and complex. Breaks a lot of assumptions and touches everything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants