Backport upstream's fix for Decypharr treating a provider 400 as permanent - #72
Merged
Merged
Conversation
Decypharr v2.5 maps an unrecognised provider error code to a PERMANENT error
(pkg/manager/link/errors.go, the ErrorCodeToLinkError default branch). TorBox
answers a link-validation request with a bare HTTP 400, so a transient
upstream hiccup became permanent in two ways:
- resolveLinkWithRetry returns at the first attempt on a permanent error, so
its four tries and exponential backoff were never used. The logs show no
"link fetch failed, retrying" line at all, which is the tell.
- fetchAndValidate memoises the failure against the link URL, and that URL is
derived from torrent_id/file_id, so the same request rebuilt the same URL.
Measured on this NAS: 70 of those errors in 72 hours, 41 of them inside the
01:00-03:00 window of the TorBox rate-limit incident. The titles they touched
sat at 0% until queue-cleanup removed them, and the arr could not replace them
either, because an item at the cutoff makes it reject every alternative.
Upstream fixed this in PR #402, which is open and unmerged. decypharr/ builds
the pinned upstream commit plus that change:
- apply-patch.py rewrites the file by exact string replacement and fails
loudly when an anchor has drifted or the file is already patched, so the
build cannot silently ship an unpatched binary. It is applied by commit
(0dd1cbb), not by tag, so the anchors cannot move under it.
- patch_test.go runs inside the image build and asserts that a 400 is
retryable and that sibling codes are unchanged. That test fails against
unpatched v2.5, so it is a real gate rather than a passing formality. It
also asks for the test names first: "go test -run <pattern>" exits 0 with
"no tests to run" when nothing matches, which would have made the gate go
quiet and still pass.
- The runtime image is upstream's own recipe, so it is v2.5 plus one
behaviour change -- same size, same entrypoint, same healthcheck.
The image is built and published by .github/workflows/decypharr-image.yml, so
the NAS pulls a real tag instead of building anything.
Verified on the NAS before this commit: image built for aarch64, the patch
string is present in the binary (0 occurrences in the upstream image), the
container recreated healthy on the existing config volume, and the version
banner reports v2.5-patch1-patched.
Also documents the distinction this turned on: the 403 rate limit and this 400
are separate events an hour apart, and it is the 400 that wedges the queue.
The first published attempt failed at the build gate with
`/bin/sh: syntax error: unexpected "("`. Docker is documented as unescaping
`$$` to a literal `$` in RUN, and it does not do so in this build: the shell
received `$$(...)` verbatim, which is not command substitution.
Reproduced in isolation with a two-line Dockerfile, so this is the builder's
behaviour rather than something about the surrounding stage.
The check now lives in decypharr/verify-patch.sh, which is plain shell that
shellcheck and sh can both read, instead of an inline RUN fighting the quoting.
Its three paths are each verified:
- patch applied, tests present -> go test passes
- patch missing -> the 400 test fails, exit 1
- test file absent -> "not in this tree; refusing to build", exit 1
The image builds with it on the NAS for aarch64.
`all image tags exist on their registry` and the Decypharr image workflow run on the same push. The check asked ghcr whether the tag exists while the build was still uploading it, so it reported the tag missing and turned a green publish into a red required check -- observed on the first two runs of this branch, with the build succeeding minutes later. Images this repository publishes are now skipped there. That check was never the right question for them: the tag's existence is decided by the workflow that builds it, and what actually matters -- that the tag compose consumes is the tag something publishes -- is asserted in tests/decypharr-patch.bats, where it cannot race. Coincidental evidence the skip is narrow: the same job still probes the other five ghcr.io images in the compose files and passes.
tests/shellcheck.bats derives that list from TARGETS and fails in both directions, so a new production shell file is a red suite until it is either swept or recorded as unswept. verify-patch.sh gets the second treatment: it is a build-time guard whose oracle is a Go compiler, so a generated sweep would report survivors that say nothing. Its behaviour is asserted in tests/decypharr-patch.bats instead, and the README says which of those is which rather than leaving the next reader to wonder why the one entry with an oracle is sitting in the list of files without one.
`shell_files` enumerates with `git ls-files`, and the NAS has no git binary -- it drives this repo through a containerised alpine/git. The test that derives the no-sweep list had no skip for that, so it failed with "derived no-sweep list is empty; the discovery is broken", which reads as a broken repository rather than a missing tool. Found by running the corpus on the NAS, which is the only host that can run it with GNU sed: `no-sweep-list-blind-to-a-new-target` came back ERRORED with "the test is already failing unmutated - a later failure would prove nothing" instead of KILLED. The corpus was right to refuse it. My earlier claim that the remaining errors there were "environmental" was only half true: the environmental part was that git is absent, and the defect was that the guard turned that into a red suite. The other test that enumerates with git ls-files already had exactly this skip and says so in its own comment. No new corpus entry: this change adds no new guard, and the entries that guard tests/shellcheck.bats already exist.
leonardoazeredo
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Decypharr v2.5 maps an unrecognised provider error code to a permanent error
(
pkg/manager/link/errors.go, theErrorCodeToLinkErrordefault branch). TorBoxanswers a link-validation request with a bare HTTP 400, so a transient upstream
hiccup became permanent in two ways:
resolveLinkWithRetryreturns at the first attempt on a permanent error, so itsfour tries and exponential backoff were never used. The log shows no
link fetch failed, retryingline at all, which is the tell.fetchAndValidatememoises the failure against the link URL, and that URL isderived from
torrent_id/file_id, so the same request rebuilt the same URL.Measured on this NAS: 70 of those errors in 72 hours, 41 of them inside the
01:00–03:00 window of the TorBox rate-limit incident. The titles they touched sat
at 0% until
queue-cleanupremoved them, and the arr could not replace themeither, because an item at the cutoff makes it reject every alternative.
Upstream fixed this in PR #402,
which is open and unmerged.
decypharr/builds the pinned upstream commitplus that change.
What is in here
decypharr/apply-patch.pyerrors.goby exact string replacement; refuses on anchor drift, a double-apply, or an unreadable filedecypharr/patch_test.godecypharr/verify-patch.shdecypharr/Dockerfile.github/workflows/decypharr-image.ymldocker-compose.arr-stack.ymltests/decypharr-patch.batsThe source is fetched by commit (
0dd1cbb), not by tag, so the patch anchorscannot move under it.
The gate is a real one
go test -run <pattern>exits 0 reporting "no tests to run" when nothingmatches, so a gate built only on that would go silent — and still pass — the
moment the test file stopped being copied in.
verify-patch.shasks for the testnames with
-listfirst and refuses to continue without them.All three of its paths verified, not asserted:
go testpassesA build or a gate that cannot fail is the failure mode this repo has a mutation
framework for, so both halves are demonstrated rather than described.
Verified on the NAS before merging
ghcr.io/sirrobot01/decypharr:v2.5healthy, version bannerv2.5-patch1-patched,existing config volume intact
compose-architecture.sh3/3 KILLED;no-sweep-list.sh2 KILLED, 3 SKIPPED for want of a host
git(the runner refuses to judge ratherthan scoring them)
Three defects found in this change's own tooling, and fixed here
$$is not unescaped by this Docker builder. The first published attemptdied with
/bin/sh: syntax error: unexpected "(". Reproduced in a two-lineDockerfile, then the check moved into a real script.
reporting the tag missing while it was still uploading — a red required check
against a green build. Images this repo publishes are now skipped there, and
tests/decypharr-patch.batsasserts the tag compose consumes is the tagsomething publishes, where it cannot race.
has no
gitbinary. Found by running the corpus there: the entry came backERRORED with "the test is already failing unmutated" instead of KILLED.
Not in scope
The other two findings from the same diagnosis are untouched and still open:
the usenet path (decisive test: one failing Usenet-Crawler NZB submitted straight
to TorBox's usenet zone) and the download-client priority mismatch (SABnzbd
recentTvPriority/olderTvPriority= −100 against Decypharr's 0 on both arrs).The 400 itself is still a real upstream answer and will still appear in the logs.
What changes is that it now costs a retry rather than the file.