fix(sabnzbd): mark history slot Failed when reported path is missing#605
fix(sabnzbd): mark history slot Failed when reported path is missing#605firestaerter3 wants to merge 2 commits into
Conversation
calculateHistoryStoragePath already detects when the path it's about to report doesn't exist on disk, but only logs a WARN and ships the slot to Sonarr/Radarr with status=Completed anyway. The ARRs then queue the item and loop on FileNotFoundException at import time forever. This change returns an exists bool alongside the path. Both callers (respondSABnzbdHistory, respondSABnzbdHistoryByIDs) overwrite the slot to Failed via a new markHistorySlotMissing helper, so the ARR removes the item cleanly through its normal failed-download path. Does not retry symlink creation; that's a follow-up. This is the minimal fix to stop the loop for items whose symlink under Import.ImportDir was never created at import time. Refs: javi11#596
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffe4e22ffe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !exists { | ||
| markHistorySlotMissing(&slot, finalPath) |
There was a problem hiding this comment.
This now rewrites any history slot to Failed whenever os.Stat on the reported path returns not-exist, but respondSABnzbdHistoryByIDs also serves rows reconstructed from persistent import history (importHistoryToQueueItem), which represent past successful imports rather than current filesystem state. In this repo, files can be legitimately removed later (for example via ARR delete/upgrade handling in internal/api/arrs_handlers.go), so a previously successful item can be reclassified as failed long after completion, causing Sonarr/Radarr to run failed-download handling on stale IDs instead of treating them as historical completions.
Useful? React with 👍 / 👎.
…path Persistent ImportHistory rows that respondSABnzbdHistory and respondSABnzbdHistoryByIDs reconstruct via importHistoryToQueueItem represent past successful imports. Their on-disk file may have been legitimately deleted later by ARR upgrade/cleanup handlers, so calculateHistoryStoragePath will report exists=false for them on a fresh poll. Without gating, the previous commit would reclassify those historical successes as Failed, causing Sonarr/Radarr to run failed-download handling on stale completed imports. Track item provenance via a pointer-keyed set populated only when the item came from the live queue with status=Completed. Apply markHistorySlotMissing only for those, leaving historical rows untouched. Refs: javi11#596
|
Good catch, thanks. Fixed in 1932a1e. The fix now gates Applied to both |
|
Dou you tested this on your setup and is working as expected? as first glance seems fine |
2 participants
Summary
If altmount's
calculateHistoryStoragePathfinds that the reported symlink path doesn't exist on disk, the SAB-history response still ships the slot withstatus=Completed. Sonarr/Radarr then queue the item and hitFileNotFoundExceptionon import.This is the loop described in #596. The fix marks the slot as
Failedwhen the path is missing, so the ARR removes it cleanly through its normal failed-download path.What changed
internal/api/sabnzbd_handlers.go:calculateHistoryStoragePathnow returns(path string, exists bool). The two callers (respondSABnzbdHistory,respondSABnzbdHistoryByIDs) override the slot'sStatustoFailedand set aFailMessagewhen!exists.internal/api/sabnzbd_types.go: smallmarkHistorySlotMissinghelper to overwrite an existing slot's status.internal/api/sabnzbd_types_test.go: unit tests for the missing-path override, including pre-existingfail_messagepreservation and nil safety.What this does NOT do
It does not retry symlink creation. If
Import.ImportDiris writable, new imports create symlinks normally; this only ensures historical items where creation already failed get reported to the ARR cleanly instead of looping forever.Test plan
sabnzbd_types_test.go.go testlocally (Go 1.26.0 toolchain plus a DNS issue on my end fetching modules). Relying on CI here. Happy to iterate if anything fails.Fixes #596.