Repository navigation
feat(clients): add Debrid-Link as a torrent client - #1380
Conversation
I hit this while building the Debrid-Link client in #1380. The new tests there changed the test count, that reshuffled the xdist workers, and these two went red. They were related to this branch, but were tests related to another PR I did which didn't have proper tests.. `test_sets_session_from_header` and `test_reads_remote_user_wsgi_fallback` both assert that a proxy-authenticated user comes back with `is_admin is True`. Since #1356 that only holds for the bootstrap account, because `_proxy_default_is_admin` returns True only while `has_admin()` is False. Both tests assume they're provisioning the first account, and only one of them can be. Run with `-n 0` so nothing is sharded: | | | |---|---| | either test alone | passes | | both, file order | the second fails | | both, reversed | the second fails | | both, across 2 workers | both pass | Reversing the order moving which one breaks is what makes it an ordering problem rather than a real one. It stays green on CI because the suite runs with `-n auto` and `tests/conftest.py` calls `mkdtemp` at module level, which runs once per worker process. Each worker gets its own `CONFIG_DIR` and its own `users.db`, the two tests land on different workers, and each one is genuinely first in its own database. That passed but it's luck rather than design, and any change to the test count can put them back together. So this gives every test in the file an empty user table and stops the question of who ran first from mattering. While I was fixed that I added coverage for the rule itself, which I had failed to test for: - the bootstrap account is an admin and the next one isn't - `PROXY_AUTH_DEFAULT_ROLE=admin` promotes later accounts - a user already in the database keeps its stored role instead of picking up the default Tests only, no source changes. The full suite passes serially now, where it had those two failures before, and it's still green under `-n auto`.
calibrain
left a comment
There was a problem hiding this comment.
Hi, thank you !
I reviewed the PR (and also asked Claude to take a look)
Here are some of the point it found
I think its a decent idea to move to some kind of shared torrent library, from where other specific torrent implementation can stem out (ie TorBos already implements a bunch of these checks, etc.)
Blockers before merge:
- A torrent can write files outside the download folder. File names come straight from the torrent, and a name containing
../can write anywhere the container can. TorBox already has a check for exactly this (_safe_relative_path); this client should reuse it. - Debrid-Link's error messages are thrown away. The PR says failures can arrive with HTTP 200, but the docs say errors come back as 4xx/5xx. The client stops at the HTTP status before reading the body, so a bad key shows as "401 Client Error" instead of
badToken. The tests pass only because the mock never raises on a bad status. - The torrent fields it checks aren't in the documented API. It reads
error,errorStringanddownloaded, but the docs only list a numericstatusfield. A dead or paused torrent would sit at "downloading" forever. - Downloads can finish incomplete. The file list is captured once and never refreshed, even though the docs say it can be incomplete at first. On top of that, files with the same name overwrite each other, and torrents with many files come back as one ZIP (
isZip), which the client doesn't handle. For audiobooks, any of these can import missing chapters with no error.
Worth raising:
- It polls
/seedbox/listevery 2 seconds. Hitting the rate limit (floodDetected) blocks the account's API use for an hour, and the handler fails the download on the first error of any kind. - Each file is held in memory twice while saving.
- The error message includes the signed download link, which then shows up in logs and the UI.
- Cancelling doesn't stop the background download.
Most of these come from copying the older Real-Debrid client rather than the hardened TorBox one. Moving the shared pieces into a common debrid module would stop the next client from repeating this.
Note : I think the poll every 2 second is worth fixing, same for the "A torrent can write files outside the download folder"
Debrid-Link is the fourth debrid service alongside AllDebrid, Real-Debrid
and TorBox, and it slots into the same DownloadClient interface.
Two things about its v2 API shape the client. Every endpoint wraps its
payload in a {success, value} envelope and can report failure over an
HTTP 200, so _request_value decides the outcome from the body rather than
the status code. And a completed seedbox torrent already carries a direct
downloadUrl on each file, so there is no unrestrict or link-request round
trip the way the other three need.
Endpoints used: account/infos to test the key and confirm premium time
remains, seedbox/add for a magnet as JSON or a .torrent as multipart,
seedbox/list to poll, and seedbox/{id}/remove to clean up.
Closes calibrain#1315
Debrid-Link needs the same guard against torrent file names that escape the download folder, so the check moves to torrent_utils.safe_relative_path and TorBox calls it. TorBox's behaviour and messages are unchanged.
Rebuilt against Debrid-Link's v2 docs, reusing what TorBox already does.
- File names go through safe_relative_path, so a torrent can't write outside
the download folder, and two files with the same name no longer overwrite
each other.
- The response body is read before the status code. Errors come back as a
4xx/5xx with `{success: false, error}`, so a bad key now reports
`badToken` instead of "401 Client Error". The undocumented
`error_description` field is no longer read.
- Only documented fields are used. A torrent is done at status 8 (seeding)
or 100 (finished), or at downloadPercent 100, and is only retrieved once
every file's downloadPercent reaches 100. Paused, queued, verifying and
`srvMaint` show as waiting messages. The API documents no error status, so
a torrent whose progress hasn't moved for an hour fails instead of sitting
at "downloading" forever.
- `wait` means the torrent is held for file selection, not queued, so one
reported that way is started through the config endpoint with no files
left out. Shelfmark doesn't ask for `wait`, but this keeps such a torrent
from never starting.
- Once ready, the file list is fetched again with `?id=`, which the docs give
for seeing the files inside a torrent listed as a single ZIP. A ZIP that
still comes back is downloaded and left to post-processing to extract.
- Status checks hit Debrid-Link at most every 15 seconds per torrent, however
often the handler polls. floodDetected pauses checks for the documented
hour instead of failing the download, and temporary errors (network,
internalError, 5xx) are retried for ten minutes before they fail it.
- Files are written from the download buffer instead of copied out of it, the
signed download URL stays out of error messages, and remove() cancels a
retrieval in progress before cleaning up.
06e1c51 to
4d00c18
Compare
|
Thanks for going through this so carefully. You were right that I'd copied the older Real-Debrid client instead of starting from TorBox, and it showed. I've pushed a rework that goes by the v2 docs rather than my guesses, and rebased onto current main. The blockers
The others
On the shared debrid module, the path check is the first piece. I'd rather do the rest as its own PR than grow this one, if that works for you. One thing hasn't changed: I still haven't run this against the live API, because I don't have a Debrid-Link key. It's all tested against responses shaped like the docs' examples. If you or @Tatsu941 can give it a real torrent before merging, I'd feel a lot better about it, and I understand if you'd rather hold it until someone can. |
|
Thank you very much ! |
#1425) Follow-up to #1380. - Fetch the file list with ?ids=, the documented parameter. There is no ?id=, so a many-file torrent was not expanded and the call could fail with badArguments at the last step of every download. - Back off status checks for 60s after floodDetected, doubling to a 240s cap, instead of an hour. The orchestrator cancels a download after five minutes without a change, so the hour-long pause cancelled every active download. Only a flood on the status check starts it, and a successful check resets it. - Drop the client-side one-hour stall timer. The orchestrator stall timer always fires first. - Keep progress at 50% when file retrieval starts instead of dropping to 0% until the first file arrives. - Report queued, paused and verifying torrents as QUEUED, PAUSED and CHECKING, so a queued torrent gets the handler queue grace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
calibrain said in #1315 that he'd review and merge a Debrid-Link client if someone took a stab at it, so here it is. It's the fourth debrid service alongside AllDebrid, Real-Debrid and TorBox and it uses the same DownloadClient interface as the other three. No frontend changes, the settings UI picks it up from the backend field definitions.
I can't test this against the live API. I don't use debrid and I don't have a Debrid-Link key. Everything here is built from their v2 API docs and cross-checked against two independent reference implementations, so the request shapes should be right, but nobody has run a real magnet through it yet.
@Tatsu941 you opened the issue and mentioned they give developers free premium. Would you be able to try it? The things most worth confirming are the field names on a seedbox torrent, that
premiumLeftis the right premium check, and that an API error comes back the way the client expects. Happy to fix whatever turns up.Two things about their API shaped how this is written.
Every v2 endpoint wraps its payload in a
{success, value}envelope and a failure can arrive with an HTTP 200, so_request_valuedecides the outcome from the body instead of the status code.A completed seedbox torrent already carries a direct
downloadUrlon every file. The other three clients each need a separate unrestrict or link-request call per file and this one doesn't, which is why it came out shorter than them despite doing the same job.Endpoints used:
GET account/infosPOST seedbox/addGET seedbox/listDELETE seedbox/{id}/removeRuff and basedpyright are clean and the 36 new tests pass. The two failures in
tests/e2e/test_proxy_auth_middleware.pyare already on main and fail the same way on a pristine checkout. They assume the first proxy-authenticated user becomes an admin, which only holds while the user table has no admin in it, so they depend on how many users earlier tests created.One thing for review: the download loop uses
download_urllike the other three clients, which holds each file in memory. I kept the existing pattern rather than do something different in a new-provider PR, but say so if you'd rather it streamed here.Closes #1315