diff --git a/CHANGELOG.md b/CHANGELOG.md index 775c57d..74aedc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ All notable changes to this project will be documented in this file. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.7.32] - 2026-05-28 + +### Fixed + +- **TransmissionClient Hardening (Issue #63)** — Mapped the previously-unknown Transmission RPC status code `7` to `Checking` (best-effort; the RPC spec formally documents only codes 0–6, and the historical alias `TORRENT_IS_CHECKING` corresponds to code 2), so torrents reporting code 7 are now rendered with a useful status label instead of `Unknown`. Implemented three torrent control methods on `TransmissionClient` that were previously absent: `startTorrent(id)` (resumes via `torrent-start`), `stopTorrent(id)` (pauses via `torrent-stop`), and `removeTorrent(id, deleteData = false)` (removes via `torrent-remove`, optionally also deleting local files via Transmission's `delete-local-data` flag). All three accept either a single id (numeric or hash) or an array of ids, matching the Transmission RPC contract. Documented in `extractArrInfo()` that an `arrQueueId` cannot reliably be derived from filename alone — the cross-client matching path is hash-based via `DownloadMatcher.matchTorrents()` (Issue #65), which keys on `torrent.hashString` for Transmission. Added regression tests for status code 7 and all three control methods. Resolves Gitea Issue [#63](https://git.i3omb.com/Gandalf/sofarr/issues/63). +- **Frontend Build Stability (Issue #66)** — Added explanatory inline comments to `client/vite.config.js` documenting two non-standard but deliberate build settings: `build.outDir = '../public'` (the Vite bundle is emitted into the Express-served `public/` directory at the repo root rather than the Vite-default `client/dist/`) and `build.emptyOutDir = false` (required so the hand-authored static assets committed under `public/` are not wiped by every `vite build`). The comments explicitly warn that changing either setting without also updating the Express static-serve configuration in `server/app.js` and the Dockerfile copy steps will break production serving. Removed a stale, untracked `client/dist/` directory (a leftover from an earlier default-Vite build) that was harmless — both `.gitignore` and `.dockerignore` already excluded it from version control and Docker contexts — but caused recurring confusion about which `index.html` was authoritative. Verified `client/index.html` correctly references `/src/main.js` as the Vite entrypoint. Resolves Gitea Issue [#66](https://git.i3omb.com/Gandalf/sofarr/issues/66). +- **Download Matching & Deduplication (Issue #65)** — `DownloadMatcher.matchTorrents()` now attempts hash-first matching for every torrent before falling back to title-substring matching. The hash lookup compares `torrent.hash` (qBittorrent, rTorrent) or `torrent.hashString` (Transmission) against each *arr queue/history record's `downloadId`, restoring deterministic matching for renamed downloads and torrents whose on-disk filename has diverged from the *arr release title. Title-substring matching is retained verbatim as a fallback so unhashed clients and legacy fixtures continue to work. After the per-torrent matching pass, the returned list is deduplicated by the composite key `(arrType, arrQueueId)`: the first matched download wins, so a single torrent that maps to N *arr queue records sharing one queue id (for example, a season pack exposed as multiple per-episode rows) produces a single dashboard card instead of N near-identical duplicates. A new integration suite at `tests/integration/download-matcher-season-pack.test.js` covers hash-first matching for qBittorrent (`hash`) and Transmission (`hashString`), the title-substring fallback path, and the deduplication step. Resolves Gitea Issue [#65](https://git.i3omb.com/Gandalf/sofarr/issues/65). +- **qBittorrentClient Peer Data & Response Safety (Issue #64)** — `QBittorrentClient.normalizeDownload()` now exposes two new fields on every torrent record: `seeds` (sourced from qBittorrent's `num_seeds`, the count of connected seed peers) and `peers` (sourced from `num_leechs`, the count of connected leecher peers). The connected counts were chosen deliberately over the swarm totals `num_complete`/`num_incomplete` so the values remain consistent with what other clients (Transmission via `peersConnected`/`peersSendingToUs`, rTorrent via `d.peers_connected`) report on the same normalised contract. `QBittorrentClient.getMainData()` now also defensively returns the existing in-memory torrent map (rather than dereferencing a null) when the qBittorrent server responds with an empty body to `/api/v2/sync/maindata`, eliminating a crash class observed against transiently-restarting qBittorrent instances. A regression test verifies the new fields are populated from `num_seeds`/`num_leechs` and not from the swarm-total fields. Resolves Gitea Issue [#64](https://git.i3omb.com/Gandalf/sofarr/issues/64). +- **Season Pack Queue Handling & Crash Prevention (Issue #61)** — Extracted a shared `buildArrQueueCache(queues, instances, mediaKey)` helper at `server/utils/arrQueueHelpers.js` covering both Sonarr and Radarr, replacing four previously-divergent inline `flatMap` blocks across the background poller (`server/utils/poller.js`) and the webhook event processor (`server/routes/webhook.js`) that built the `poll:sonarr-queue` and `poll:radarr-queue` cache entries. Sonarr queue records that share a `downloadId` (the canonical fingerprint for a season-pack release) are now annotated with `isSeasonPack: true` and `episodeCount: ` so downstream consumers — including the active-downloads matching service — can identify and de-duplicate season packs without re-deriving the grouping. The helper is wrapped in per-record and per-instance `try`/`catch` guards: malformed records (`null`, missing `data`, unknown instance ids) are skipped with a warning rather than throwing, eliminating a class of crashes that previously bubbled out of the `flatMap` and tore down the entire poll cycle or webhook refresh. Movies (Radarr) skip season-pack annotation by design. A new unit test suite at `tests/unit/utils/arrQueueHelpers.test.js` covers tagging, season-pack grouping, null-safety, and unknown-instance fallback. Resolves Gitea Issue [#61](https://git.i3omb.com/Gandalf/sofarr/issues/61). +- **rTorrent Null-Safety, SABnzbd History Limit & Client Last-Error Visibility (Issue #68)** — Three related hardening improvements to the download-client layer. First, `RTorrentClient` now defends against the malformed-response scenarios observed against misconfigured or transiently-restarting rTorrent servers: `getActiveDownloads()` explicitly checks that `d.multicall2` returned an actual array (logging a warning and returning `[]` if not, rather than throwing on `.map`) and processes each torrent row in its own `try`/`catch` so a single malformed entry cannot poison the whole result set. All eleven field values retrieved from the multicall response are coerced to their expected types via explicit `Number()`/`String()` conversions in `normalizeDownload()`, so downstream arithmetic and string operations can no longer blow up on `null` or `undefined` values from plugins or older rTorrent versions. `_extractArrInfo()` now short-circuits safely on non-string filenames. `getClientStatus()` additionally coerces the global rate values through `Number.isFinite` before returning them. Second, the SABnzbd history limit (previously hard-coded to `10` records per poll) is now configurable via the `SAB_HISTORY_LIMIT` environment variable. Invalid or absent values fall back to the default of `10` with a log warning, ensuring backward compatibility. Third, all four download clients (`RTorrentClient`, `SABnzbdClient`, `QBittorrentClient`, `TransmissionClient`) now record structured `lastError` objects (`{ operation, message, at }`) on every failed API call via `_recordLastError()` and clear them on subsequent success via `_clearLastError()` — both helpers introduced on the `DownloadClient` base class alongside the public `getLastError()` accessor. The per-client last-error is surfaced through `DownloadClientRegistry.getAllClientStatuses()` and exposed on the `GET /api/status/status` admin endpoint under the new `downloadClients` array, letting the admin panel show a per-client failure indicator without log scraping. New regression tests cover all null-safety paths, the SAB history limit env variable (unset, valid, invalid, propagated to the API call), and the full lastError set/clear cycle for both rTorrent and SABnzbd. Resolves Gitea Issue [#68](https://git.i3omb.com/Gandalf/sofarr/issues/68). +- **Webhook Reliability (Issue #62)** — Hardened the webhook replay protection to prevent false-duplicate detection while preserving protection against genuine retries. The replay key for Sonarr and Radarr now incorporates a content identifier (`downloadId`, falling back to `series.id` or `movie.id`) alongside the existing `eventType:instanceName:eventDate` components, so that multiple distinct events sharing the same timestamp (for example, several `Grab` events fired in the same second for episodes in a season pack) no longer collide and get silently dropped. Events without a content identifier (such as `Test`) fall back gracefully to the previous key shape so existing behaviour is preserved. The Ombi handler — which already uses a distinct `requestId`-bearing key — is unchanged. Additionally, the Sonarr and Radarr handlers now log an explicit warning when the inbound `instanceName` fails to match any configured instance and processing falls back to the first instance, improving diagnosability of misconfigured webhook senders. Resolves Gitea Issue [#62](https://git.i3omb.com/Gandalf/sofarr/issues/62). + ## [1.7.31] - 2026-05-28 ### Fixed diff --git a/client/vite.config.js b/client/vite.config.js index 8b38bce..5cf0550 100644 --- a/client/vite.config.js +++ b/client/vite.config.js @@ -12,7 +12,17 @@ export default defineConfig(({ mode }) => { return { build: { + // NOTE (Issue #66): `outDir` is intentionally the repo-root `../public`, + // NOT the Vite default `client/dist/`. The Express server in + // `server/app.js` serves static assets directly from `public/`, so the + // Vite build emits its bundle alongside the hand-authored static assets + // (favicon, etc.) that live in `public/` and are committed to the repo. + // Do NOT change this back to `dist/` without also updating the Express + // static-serve configuration and the Dockerfile copy steps. outDir: '../public', + // NOTE (Issue #66): `emptyOutDir: false` is REQUIRED because `public/` + // contains hand-authored static assets that must survive the build. + // Setting this to `true` would wipe those assets on every `vite build`. emptyOutDir: false, rollupOptions: { input: { diff --git a/package-lock.json b/package-lock.json index f8f670a..910bd23 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "sofarr", - "version": "1.7.31", + "version": "1.7.32", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "sofarr", - "version": "1.7.31", + "version": "1.7.32", "license": "MIT", "dependencies": { "axios": "^1.6.0", diff --git a/package.json b/package.json index fdf4604..024649c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "sofarr", - "version": "1.7.31", + "version": "1.7.32", "description": "A personal media download dashboard that shows your downloads 'so far' while you relax on the sofa waiting for your *arr services to finish", "main": "server/index.js", "scripts": { diff --git a/server/app.js b/server/app.js index c593aea..ea340a4 100644 --- a/server/app.js +++ b/server/app.js @@ -133,7 +133,7 @@ function createApp({ skipRateLimits = false } = {}) { * version: * type: string * description: sofarr version - * example: "1.7.31" + * example: "1.7.32" * x-code-samples: * - lang: curl * label: cURL diff --git a/server/clients/DownloadClient.js b/server/clients/DownloadClient.js index b5d7413..b6da2de 100644 --- a/server/clients/DownloadClient.js +++ b/server/clients/DownloadClient.js @@ -25,6 +25,41 @@ class DownloadClient { this.apiKey = instanceConfig.apiKey; this.username = instanceConfig.username; this.password = instanceConfig.password; + + // Last error encountered while talking to this client. + // Cleared on successful calls via _clearLastError(); set via _recordLastError(). + // Surfaced through getAllClientStatuses() so the admin status panel can show + // a per-client failure indicator without needing to scrape logs. + this.lastError = null; + } + + /** + * Record an error encountered while talking to this client. + * @param {string} operation - Short description of the operation (e.g. 'getActiveDownloads') + * @param {Error|string} error - Error object or message + */ + _recordLastError(operation, error) { + const message = (error && error.message) ? error.message : String(error || 'unknown error'); + this.lastError = { + operation, + message, + at: new Date().toISOString() + }; + } + + /** + * Clear the last error (called when an operation succeeds). + */ + _clearLastError() { + this.lastError = null; + } + + /** + * Public accessor for the last recorded error, or null if none. + * @returns {{operation:string, message:string, at:string}|null} + */ + getLastError() { + return this.lastError; } /** diff --git a/server/clients/QBittorrentClient.js b/server/clients/QBittorrentClient.js index 7fdf876..746beaa 100644 --- a/server/clients/QBittorrentClient.js +++ b/server/clients/QBittorrentClient.js @@ -23,9 +23,11 @@ class QBittorrentClient extends DownloadClient { // Try a simple API call to verify connection await this.makeRequest('/api/v2/app/version'); logToFile(`[qBittorrent:${this.name}] Connection test successful`); + this._clearLastError(); return true; } catch (error) { logToFile(`[qBittorrent:${this.name}] Connection test failed: ${error.message}`); + this._recordLastError('testConnection', error); return false; } } @@ -104,6 +106,11 @@ class QBittorrentClient extends DownloadClient { const response = await this.makeRequest(`/api/v2/sync/maindata?rid=${this.lastRid}`); const data = response.data; + if (!data) { + logToFile(`[qBittorrent:${this.name}] Empty response from sync/maindata`); + return Array.from(this.torrentMap.values()); + } + if (data.full_update) { // Full refresh: rebuild the entire map this.torrentMap.clear(); @@ -169,6 +176,7 @@ class QBittorrentClient extends DownloadClient { const torrents = await this.getMainData(); logToFile(`[qBittorrent:${this.name}] Sync: ${torrents.length} torrents (rid=${this.lastRid})`); + this._clearLastError(); return torrents.map(torrent => this.normalizeDownload(torrent)); } catch (error) { logToFile(`[qBittorrent:${this.name}] Sync failed, falling back to legacy: ${error.message}`); @@ -183,6 +191,7 @@ class QBittorrentClient extends DownloadClient { return torrents.map(torrent => this.normalizeDownload(torrent)); } catch (fallbackError) { logToFile(`[qBittorrent:${this.name}] Fallback also failed: ${fallbackError.message}`); + this._recordLastError('getActiveDownloads', fallbackError); return []; } } @@ -193,6 +202,7 @@ class QBittorrentClient extends DownloadClient { const response = await this.makeRequest('/api/v2/sync/maindata'); const data = response.data; + this._clearLastError(); return { serverState: data.server_state || {}, rid: data.rid, @@ -200,6 +210,7 @@ class QBittorrentClient extends DownloadClient { }; } catch (error) { logToFile(`[qBittorrent:${this.name}] Error getting client status: ${error.message}`); + this._recordLastError('getClientStatus', error); return null; } } @@ -249,6 +260,14 @@ class QBittorrentClient extends DownloadClient { downloaded: downloadedSize, speed: torrent.dlspeed, eta: torrent.eta < 0 || torrent.eta === 8640000 ? null : torrent.eta, + // Connected peer counts (Issue #64). qBittorrent exposes: + // num_seeds — connected seeds (peers we have a connection to) + // num_leechs — connected leechers (peers downloading from us) + // num_complete / num_incomplete — *swarm* totals reported by tracker + // We expose the connected counts to stay consistent with what other + // clients (e.g. Transmission via peersConnected/peersSendingToUs) report. + seeds: torrent.num_seeds ?? 0, + peers: torrent.num_leechs ?? 0, category: torrent.category || undefined, tags: torrent.tags ? torrent.tags.split(',').filter(tag => tag.trim()) : [], savePath: torrent.content_path || torrent.save_path || undefined, diff --git a/server/clients/RTorrentClient.js b/server/clients/RTorrentClient.js index 0717fc0..a03f2e2 100644 --- a/server/clients/RTorrentClient.js +++ b/server/clients/RTorrentClient.js @@ -35,9 +35,11 @@ class RTorrentClient extends DownloadClient { try { await this._methodCall('system.client_version'); logToFile(`[rtorrent:${this.name}] Connection test successful`); + this._clearLastError(); return true; } catch (error) { logToFile(`[rtorrent:${this.name}] Connection test failed: ${error.message}`); + this._recordLastError('testConnection', error); return false; } } @@ -77,10 +79,31 @@ class RTorrentClient extends DownloadClient { 'd.custom1=' ]); + // rTorrent XML-RPC can occasionally return null/undefined or a non-array + // on misconfigured servers or transient errors. Guard against that here + // so callers always get a sane array instead of throwing on .map. + if (!Array.isArray(torrents)) { + logToFile(`[rtorrent:${this.name}] d.multicall2 returned non-array (${typeof torrents}); treating as empty`); + this._clearLastError(); + return []; + } + logToFile(`[rtorrent:${this.name}] Retrieved ${torrents.length} torrents`); - return torrents.map(torrent => this.normalizeDownload(torrent)); + this._clearLastError(); + // Filter out any individual rows that fail to normalize so a single bad + // record cannot poison the whole result set. + const normalized = []; + for (const torrent of torrents) { + try { + normalized.push(this.normalizeDownload(torrent)); + } catch (err) { + logToFile(`[rtorrent:${this.name}] Skipping malformed torrent row: ${err.message}`); + } + } + return normalized; } catch (error) { logToFile(`[rtorrent:${this.name}] Error fetching torrents: ${error.message}`); + this._recordLastError('getActiveDownloads', error); return []; } } @@ -92,31 +115,53 @@ class RTorrentClient extends DownloadClient { this._methodCall('throttle.global_up.rate') ]); + this._clearLastError(); return { - globalDownRate: downRate, - globalUpRate: upRate + globalDownRate: Number.isFinite(downRate) ? downRate : 0, + globalUpRate: Number.isFinite(upRate) ? upRate : 0 }; } catch (error) { logToFile(`[rtorrent:${this.name}] Error getting client status: ${error.message}`); + this._recordLastError('getClientStatus', error); return null; } } normalizeDownload(torrent) { + // rTorrent's d.multicall2 returns an array of fields in the order requested. + // If a value is missing rtorrent typically returns '' or 0, but plugins and + // older versions can return undefined/null — coerce everything explicitly so + // downstream math and string ops never blow up on null/undefined. + if (!Array.isArray(torrent)) { + throw new Error('Expected torrent row to be an array'); + } + const [ - hash, - name, - sizeBytes, - completedBytes, - downRate, - upRate, - state, - isActive, - isHashChecking, - directory, - custom1 + hashRaw, + nameRaw, + sizeBytesRaw, + completedBytesRaw, + downRateRaw, + upRateRaw, + stateRaw, + isActiveRaw, + isHashCheckingRaw, + directoryRaw, + custom1Raw ] = torrent; + const hash = hashRaw ? String(hashRaw) : ''; + const name = nameRaw ? String(nameRaw) : ''; + const sizeBytes = Number(sizeBytesRaw) || 0; + const completedBytes = Number(completedBytesRaw) || 0; + const downRate = Number(downRateRaw) || 0; + const upRate = Number(upRateRaw) || 0; + const state = Number.isFinite(Number(stateRaw)) ? Number(stateRaw) : 0; + const isActive = Number.isFinite(Number(isActiveRaw)) ? Number(isActiveRaw) : 0; + const isHashChecking = Number.isFinite(Number(isHashCheckingRaw)) ? Number(isHashCheckingRaw) : 0; + const directory = directoryRaw ? String(directoryRaw) : ''; + const custom1 = custom1Raw ? String(custom1Raw) : ''; + const status = this._mapStatus(state, isActive, isHashChecking, completedBytes, sizeBytes); const progress = sizeBytes > 0 ? Math.round((completedBytes / sizeBytes) * 100) : 0; @@ -168,6 +213,11 @@ class RTorrentClient extends DownloadClient { } _extractArrInfo(filename) { + // Null-safe: getActiveDownloads passes a normalized string, but guard anyway + // so callers passing raw rtorrent values cannot crash this helper. + if (!filename || typeof filename !== 'string') { + return {}; + } const seriesMatch = filename.match(/[-\s]S(\d{2})E(\d{2})/i); if (seriesMatch) { return { type: 'series' }; diff --git a/server/clients/SABnzbdClient.js b/server/clients/SABnzbdClient.js index fc01c68..4569a6c 100644 --- a/server/clients/SABnzbdClient.js +++ b/server/clients/SABnzbdClient.js @@ -3,9 +3,27 @@ const axios = require('axios'); const DownloadClient = require('./DownloadClient'); const { logToFile } = require('../utils/logger'); +// Number of recently completed jobs to pull from SABnzbd's /api?mode=history on +// every poll. Larger values let DownloadMatcher correlate slightly older jobs +// with their Sonarr/Radarr queue entries at the cost of one wider HTTP +// response per poll cycle. Configurable via the SAB_HISTORY_LIMIT environment +// variable; defaults to 10 to match the previous hardcoded value. +const DEFAULT_HISTORY_LIMIT = 10; +function resolveHistoryLimit() { + const raw = process.env.SAB_HISTORY_LIMIT; + if (raw === undefined || raw === null || raw === '') return DEFAULT_HISTORY_LIMIT; + const parsed = parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed < 0) { + logToFile(`[SABnzbd] Invalid SAB_HISTORY_LIMIT='${raw}'; falling back to ${DEFAULT_HISTORY_LIMIT}`); + return DEFAULT_HISTORY_LIMIT; + } + return parsed; +} + class SABnzbdClient extends DownloadClient { constructor(instance) { super(instance); + this.historyLimit = resolveHistoryLimit(); } getClientType() { @@ -16,9 +34,11 @@ class SABnzbdClient extends DownloadClient { try { const response = await this.makeRequest('', { mode: 'version' }); logToFile(`[SABnzbd:${this.name}] Connection test successful, version: ${response.data.version}`); + this._clearLastError(); return true; } catch (error) { logToFile(`[SABnzbd:${this.name}] Connection test failed: ${error.message}`); + this._recordLastError('testConnection', error); return false; } } @@ -47,7 +67,7 @@ class SABnzbdClient extends DownloadClient { // Get both queue and history to provide complete picture const [queueResponse, historyResponse] = await Promise.all([ this.makeRequest({ mode: 'queue' }), - this.makeRequest({ mode: 'history', limit: 10 }) + this.makeRequest({ mode: 'history', limit: this.historyLimit }) ]); const queueData = queueResponse.data; @@ -99,10 +119,12 @@ class SABnzbdClient extends DownloadClient { } } - logToFile(`[SABnzbd:${this.name}] Retrieved ${downloads.length} downloads`); + logToFile(`[SABnzbd:${this.name}] Retrieved ${downloads.length} downloads (historyLimit=${this.historyLimit})`); + this._clearLastError(); return downloads; } catch (error) { logToFile(`[SABnzbd:${this.name}] Error fetching downloads: ${error.message}`); + this._recordLastError('getActiveDownloads', error); return []; } } @@ -112,8 +134,12 @@ class SABnzbdClient extends DownloadClient { const response = await this.makeRequest({ mode: 'queue' }); const queueData = response.data.queue; - if (!queueData) return null; + if (!queueData) { + this._clearLastError(); + return null; + } + this._clearLastError(); return { status: queueData.status, speed: queueData.speed, @@ -128,6 +154,7 @@ class SABnzbdClient extends DownloadClient { }; } catch (error) { logToFile(`[SABnzbd:${this.name}] Error getting client status: ${error.message}`); + this._recordLastError('getClientStatus', error); return null; } } diff --git a/server/clients/TransmissionClient.js b/server/clients/TransmissionClient.js index 12991e5..cf39097 100644 --- a/server/clients/TransmissionClient.js +++ b/server/clients/TransmissionClient.js @@ -18,9 +18,11 @@ class TransmissionClient extends DownloadClient { try { await this.makeRequest('session-get'); logToFile(`[Transmission:${this.name}] Connection test successful`); + this._clearLastError(); return true; } catch (error) { logToFile(`[Transmission:${this.name}] Connection test failed: ${error.message}`); + this._recordLastError('testConnection', error); return false; } } @@ -80,10 +82,11 @@ class TransmissionClient extends DownloadClient { const torrents = response.data.arguments.torrents || []; logToFile(`[Transmission:${this.name}] Retrieved ${torrents.length} torrents`); - + this._clearLastError(); return torrents.map(torrent => this.normalizeDownload(torrent)); } catch (error) { logToFile(`[Transmission:${this.name}] Error fetching torrents: ${error.message}`); + this._recordLastError('getActiveDownloads', error); return []; } } @@ -93,12 +96,14 @@ class TransmissionClient extends DownloadClient { const response = await this.makeRequest('session-get'); const sessionStats = await this.makeRequest('session-stats'); + this._clearLastError(); return { session: response.data.arguments, stats: sessionStats.data.arguments }; } catch (error) { logToFile(`[Transmission:${this.name}] Error getting client status: ${error.message}`); + this._recordLastError('getClientStatus', error); return null; } } @@ -113,7 +118,11 @@ class TransmissionClient extends DownloadClient { 4: 'Downloading', // TORRENT_DOWNLOAD 5: 'Queued', // TORRENT_SEED_WAIT 6: 'Seeding', // TORRENT_SEED - 7: 'Unknown' // TORRENT_IS_CHECKING (alias for 2) + // Status code 7 is undocumented in the Transmission RPC spec (which + // formally defines only 0–6). The legacy alias "TORRENT_IS_CHECKING" + // (a duplicate of code 2) is the best-effort interpretation; map it to + // `Checking` so it is rendered usefully rather than as `Unknown`. + 7: 'Checking' // TORRENT_IS_CHECKING (undocumented; treated as code 2) }; const status = statusMap[torrent.status] || 'Unknown'; @@ -160,8 +169,12 @@ class TransmissionClient extends DownloadClient { } extractArrInfo(filename) { - // Similar to SABnzbdClient, try to extract Sonarr/Radarr info - + // arrQueueId cannot be extracted from filename alone; *arr exposes that + // identifier only via its queue API. The reliable cross-client matching + // path is hash-based and lives in `DownloadMatcher.matchTorrents()` (see + // Issue #65), which keys on `torrent.hashString` for Transmission. + // This heuristic remains only to provide a coarse `type` hint. + // Look for patterns like "Series Name - S01E02 - Episode Title" const seriesMatch = filename.match(/[-\s]S(\d{2})E(\d{2})/i); if (seriesMatch) { @@ -176,6 +189,39 @@ class TransmissionClient extends DownloadClient { return {}; } + + /** + * Start (resume) one or more torrents. `id` is the Transmission internal + * numeric id or a hashString; the RPC accepts either. + * @param {number|string|Array} id + */ + async startTorrent(id) { + const ids = Array.isArray(id) ? id : [id]; + await this.makeRequest('torrent-start', { ids }); + logToFile(`[Transmission:${this.name}] Started torrent(s): ${ids.join(', ')}`); + } + + /** + * Stop (pause) one or more torrents. + * @param {number|string|Array} id + */ + async stopTorrent(id) { + const ids = Array.isArray(id) ? id : [id]; + await this.makeRequest('torrent-stop', { ids }); + logToFile(`[Transmission:${this.name}] Stopped torrent(s): ${ids.join(', ')}`); + } + + /** + * Remove one or more torrents. When `deleteData` is true the local files + * are also deleted from disk (Transmission's `delete-local-data`). + * @param {number|string|Array} id + * @param {boolean} [deleteData=false] + */ + async removeTorrent(id, deleteData = false) { + const ids = Array.isArray(id) ? id : [id]; + await this.makeRequest('torrent-remove', { ids, 'delete-local-data': !!deleteData }); + logToFile(`[Transmission:${this.name}] Removed torrent(s): ${ids.join(', ')} (deleteData=${!!deleteData})`); + } } module.exports = TransmissionClient; diff --git a/server/openapi.yaml b/server/openapi.yaml index 115f548..989a84c 100644 --- a/server/openapi.yaml +++ b/server/openapi.yaml @@ -22,7 +22,7 @@ info: ## SSE Streaming Real-time updates are available via Server-Sent Events at GET /api/dashboard/stream. - version: 1.7.31 + version: 1.7.32 contact: name: sofarr license: diff --git a/server/routes/status.js b/server/routes/status.js index a6fb68f..2166448 100644 --- a/server/routes/status.js +++ b/server/routes/status.js @@ -7,6 +7,7 @@ const { getLastPollTimings, POLLING_ENABLED } = require('../utils/poller'); const { getSonarrInstances, getRadarrInstances, getOmbiInstances } = require('../utils/config'); const { getGlobalWebhookMetrics } = require('../utils/cache'); const { checkWebhookConfigured, checkOmbiWebhookConfigured, aggregateMetrics } = require('../services/WebhookStatus'); +const downloadClientRegistry = require('../utils/downloadClients'); /** * @openapi @@ -165,7 +166,16 @@ router.get('/', requireAuth, async (req, res) => { sonarr: aggregateMetrics(sonarrMetrics, sonarrWebhookConfigured), radarr: aggregateMetrics(radarrMetrics, radarrWebhookConfigured), ombi: aggregateMetrics(ombiMetrics, ombiWebhookConfigured) - } + }, + // Per-download-client health summary including any lastError captured + // since the last successful call. Lets the admin status panel surface + // transient failures (auth expiry, RPC blips, etc.) without log scraping. + downloadClients: downloadClientRegistry.getAllClients().map(c => ({ + instanceId: c.getInstanceId(), + instanceName: c.name, + clientType: c.getClientType(), + lastError: typeof c.getLastError === 'function' ? c.getLastError() : null + })) }); } catch (err) { res.status(500).json({ error: 'Failed to get status', details: err.message }); diff --git a/server/routes/webhook.js b/server/routes/webhook.js index f21dedc..41bcc89 100644 --- a/server/routes/webhook.js +++ b/server/routes/webhook.js @@ -5,6 +5,7 @@ const { logToFile } = require('../utils/logger'); const { getWebhookSecret, getSonarrInstances, getRadarrInstances, getOmbiInstances, getSofarrBaseUrl } = require('../utils/config'); const cache = require('../utils/cache'); const arrRetrieverRegistry = require('../utils/arrRetrievers'); +const { buildArrQueueCache } = require('../utils/arrQueueHelpers'); const { pollAllServices, POLL_INTERVAL, POLLING_ENABLED } = require('../utils/poller'); const { extractRequestedUser } = require('../utils/ombiHelpers'); const requireAuth = require('../middleware/requireAuth'); @@ -106,9 +107,14 @@ function pruneReplayCache() { // Prune the replay cache once per minute setInterval(pruneReplayCache, 60 * 1000).unref(); -function isReplay(eventType, instanceName, eventDate) { +function isReplay(eventType, instanceName, eventDate, contentId) { if (!eventDate) return false; - const key = `${eventType}:${instanceName || ''}:${eventDate}`; + // Content-aware replay key: incorporates downloadId / series.id / movie.id when + // available so that distinct events sharing the same `date` (e.g. multiple + // Grab events for episodes in a season pack fired in the same second) do not + // falsely collide. Falls back to the prior shape when contentId is absent + // (e.g. Test events) so existing behaviour is preserved. + const key = `${eventType}:${instanceName || ''}:${contentId || ''}:${eventDate}`; if (recentEvents.has(key)) return true; recentEvents.set(key, Date.now()); return false; @@ -202,17 +208,7 @@ async function processWebhookEvent(serviceType, eventType, payload = null) { const queuesByType = await arrRetrieverRegistry.getQueuesByType(); const sonarrQueues = queuesByType.sonarr || []; cache.set('poll:sonarr-queue', { - records: sonarrQueues.flatMap(q => { - const inst = sonarrInstances.find(i => i.id === q.instance); - const url = inst ? inst.url : null; - const key = inst ? inst.apiKey : null; - return (q.data.records || []).map(r => { - if (r.series) r.series._instanceUrl = url; - r._instanceUrl = url; - r._instanceKey = key; - return r; - }); - }) + records: buildArrQueueCache(sonarrQueues, sonarrInstances, 'series') }, CACHE_TTL); logToFile(`[Webhook] Refreshed poll:sonarr-queue (${sonarrQueues.length} instance(s))`); } @@ -232,17 +228,7 @@ async function processWebhookEvent(serviceType, eventType, payload = null) { const queuesByType = await arrRetrieverRegistry.getQueuesByType(); const radarrQueues = queuesByType.radarr || []; cache.set('poll:radarr-queue', { - records: radarrQueues.flatMap(q => { - const inst = radarrInstances.find(i => i.id === q.instance); - const url = inst ? inst.url : null; - const key = inst ? inst.apiKey : null; - return (q.data.records || []).map(r => { - if (r.movie) r.movie._instanceUrl = url; - r._instanceUrl = url; - r._instanceKey = key; - return r; - }); - }) + records: buildArrQueueCache(radarrQueues, radarrInstances, 'movie') }, CACHE_TTL); logToFile(`[Webhook] Refreshed poll:radarr-queue (${radarrQueues.length} instance(s))`); } @@ -480,11 +466,18 @@ router.post('/sonarr', webhookLimiter, (req, res) => { const { eventType, instanceName, eventDate } = validation; const sonarrInstances = getSonarrInstances(); - const inst = sonarrInstances.find(i => i.name === instanceName || i.id === instanceName) || sonarrInstances[0]; + const matchedInst = sonarrInstances.find(i => i.name === instanceName || i.id === instanceName); + const inst = matchedInst || sonarrInstances[0]; + if (!matchedInst && instanceName) { + logToFile(`[Webhook] Sonarr instanceName "${instanceName}" did not match any configured instance; falling back to first instance (${inst ? inst.name : 'none'})`); + } const resolvedInstanceName = inst ? inst.name : instanceName; - if (isReplay(eventType, resolvedInstanceName, eventDate)) { - logToFile(`[Webhook] Sonarr duplicate event ignored: ${eventType} @ ${eventDate}`); + // Content-aware replay key components (Issue #62) + const contentId = req.body.downloadId || req.body.series?.id || null; + + if (isReplay(eventType, resolvedInstanceName, eventDate, contentId)) { + logToFile(`[Webhook] Sonarr duplicate event ignored: ${eventType} @ ${eventDate} (contentId=${contentId || 'none'})`); return res.status(200).json({ received: true, duplicate: true }); } @@ -634,11 +627,18 @@ router.post('/radarr', webhookLimiter, (req, res) => { const { eventType, instanceName, eventDate } = validation; const radarrInstances = getRadarrInstances(); - const inst = radarrInstances.find(i => i.name === instanceName || i.id === instanceName) || radarrInstances[0]; + const matchedInst = radarrInstances.find(i => i.name === instanceName || i.id === instanceName); + const inst = matchedInst || radarrInstances[0]; + if (!matchedInst && instanceName) { + logToFile(`[Webhook] Radarr instanceName "${instanceName}" did not match any configured instance; falling back to first instance (${inst ? inst.name : 'none'})`); + } const resolvedInstanceName = inst ? inst.name : instanceName; - if (isReplay(eventType, resolvedInstanceName, eventDate)) { - logToFile(`[Webhook] Radarr duplicate event ignored: ${eventType} @ ${eventDate}`); + // Content-aware replay key components (Issue #62) + const contentId = req.body.downloadId || req.body.movie?.id || null; + + if (isReplay(eventType, resolvedInstanceName, eventDate, contentId)) { + logToFile(`[Webhook] Radarr duplicate event ignored: ${eventType} @ ${eventDate} (contentId=${contentId || 'none'})`); return res.status(200).json({ received: true, duplicate: true }); } diff --git a/server/services/DownloadMatcher.js b/server/services/DownloadMatcher.js index de6fa24..7bbbfb2 100644 --- a/server/services/DownloadMatcher.js +++ b/server/services/DownloadMatcher.js @@ -429,7 +429,20 @@ async function matchTorrents(torrents, context) { let matchedAny = false; - const sonarrMatch = sonarrQueueRecords.find(r => { + // Hash-first matching (Issue #65): prefer matching by torrent hash against + // each *arr queue record's `downloadId`. `torrent.hash` covers qBittorrent + // and rTorrent; `torrent.hashString` covers Transmission. We fall back to + // existing title-substring matching only if no hash match was found. + const torrentHash = torrent?.hash || torrent?.hashString || null; + const hashLower = torrentHash ? String(torrentHash).toLowerCase() : null; + const matchesByHash = (r) => { + const dl = r && r.downloadId; + if (!dl || !hashLower) return false; + return String(dl).toLowerCase() === hashLower; + }; + + let sonarrMatch = hashLower ? sonarrQueueRecords.find(matchesByHash) : null; + if (!sonarrMatch) sonarrMatch = sonarrQueueRecords.find(r => { const rTitle = (r.title || r.sourceTitle || '').toLowerCase(); return rTitle && (rTitle.includes(torrentNameLower) || torrentNameLower.includes(rTitle)); }); @@ -480,7 +493,8 @@ async function matchTorrents(torrents, context) { } } - const radarrMatch = radarrQueueRecords.find(r => { + let radarrMatch = hashLower ? radarrQueueRecords.find(matchesByHash) : null; + if (!radarrMatch) radarrMatch = radarrQueueRecords.find(r => { const rTitle = (r.title || r.sourceTitle || '').toLowerCase(); return rTitle && (rTitle.includes(torrentNameLower) || torrentNameLower.includes(rTitle)); }); @@ -529,7 +543,8 @@ async function matchTorrents(torrents, context) { } } - const sonarrHistoryMatch = sonarrHistoryRecords.find(r => { + let sonarrHistoryMatch = hashLower ? sonarrHistoryRecords.find(matchesByHash) : null; + if (!sonarrHistoryMatch) sonarrHistoryMatch = sonarrHistoryRecords.find(r => { const rTitle = (r.sourceTitle || r.title || '').toLowerCase(); return rTitle && (rTitle.includes(torrentNameLower) || torrentNameLower.includes(rTitle)); }); @@ -562,7 +577,8 @@ async function matchTorrents(torrents, context) { } } - const radarrHistoryMatch = radarrHistoryRecords.find(r => { + let radarrHistoryMatch = hashLower ? radarrHistoryRecords.find(matchesByHash) : null; + if (!radarrHistoryMatch) radarrHistoryMatch = radarrHistoryRecords.find(r => { const rTitle = (r.sourceTitle || r.title || '').toLowerCase(); return rTitle && (rTitle.includes(torrentNameLower) || torrentNameLower.includes(rTitle)); }); @@ -598,7 +614,24 @@ async function matchTorrents(torrents, context) { } } - return matched; + + // Deduplicate by (arrType, arrQueueId) (Issue #65). When a single torrent + // (typically a season pack) matches N *arr queue records sharing one + // arrQueueId via downstream emission paths, only the first matched download + // is retained. Entries without an arrQueueId pass through unchanged. + const seen = new Set(); + const deduped = []; + for (const m of matched) { + const key = (m && m.arrType && m.arrQueueId != null) + ? `${m.arrType}:${m.arrQueueId}` + : null; + if (key) { + if (seen.has(key)) continue; + seen.add(key); + } + deduped.push(m); + } + return deduped; } module.exports = { diff --git a/server/utils/arrQueueHelpers.js b/server/utils/arrQueueHelpers.js new file mode 100644 index 0000000..1125655 --- /dev/null +++ b/server/utils/arrQueueHelpers.js @@ -0,0 +1,97 @@ +// Copyright (c) 2026 Gordon Bolton. MIT License. +// +// Shared helpers for assembling the cached *arr queue payload. +// +// Both the background poller (`server/utils/poller.js`) and the webhook +// processor (`server/routes/webhook.js`) build the `poll:sonarr-queue` and +// `poll:radarr-queue` cache entries from an array of per-instance queue +// responses. Historically the same `flatMap` block was duplicated across all +// four call sites (Sonarr + Radarr × poller + webhook) and had begun to drift. +// +// This module centralises that logic, adds defensive null-guards, and — for +// Sonarr only — annotates season-pack records (queue entries sharing a +// `downloadId`) with `isSeasonPack` and `episodeCount`. See Issue #61. +// +const { logToFile } = require('./logger'); + +/** + * Build the flattened, instance-tagged `records` array for the + * `poll:sonarr-queue` / `poll:radarr-queue` cache entry. + * + * @param {Array<{ instance: string, data: { records?: Array } }>} queues + * Per-instance queue responses as returned by + * `arrRetrieverRegistry.getQueuesByType()` (or the equivalent batched + * retrieval in the poller). + * @param {Array<{ id: string, url: string, apiKey: string, name?: string }>} instances + * Configured instances; used to resolve `_instanceUrl` / `_instanceKey`. + * @param {'series'|'movie'} mediaKey + * Sonarr records embed a `series` object; Radarr records embed a `movie` + * object. The embedded object is annotated with `_instanceUrl` so that + * downstream link builders work. + * @returns {Array} The flattened, annotated records array. + */ +function buildArrQueueCache(queues, instances, mediaKey) { + if (!Array.isArray(queues) || queues.length === 0) return []; + if (mediaKey !== 'series' && mediaKey !== 'movie') { + logToFile(`[arrQueueHelpers] Invalid mediaKey "${mediaKey}"; expected 'series' or 'movie'`); + return []; + } + const safeInstances = Array.isArray(instances) ? instances : []; + + const out = []; + for (const q of queues) { + try { + if (!q || !q.data) continue; + const inst = safeInstances.find(i => i.id === q.instance); + const url = inst ? inst.url : null; + const key = inst ? inst.apiKey : null; + const records = Array.isArray(q.data.records) ? q.data.records : []; + for (const r of records) { + try { + if (!r) continue; + if (r[mediaKey]) { + r[mediaKey]._instanceUrl = url; + } + r._instanceUrl = url; + r._instanceKey = key; + out.push(r); + } catch (perRecordErr) { + logToFile(`[arrQueueHelpers] Skipping malformed ${mediaKey} record: ${perRecordErr.message}`); + } + } + } catch (perInstanceErr) { + logToFile(`[arrQueueHelpers] Skipping malformed ${mediaKey} queue payload: ${perInstanceErr.message}`); + } + } + + // Sonarr-only: season pack annotation. Group by downloadId; entries that + // share a downloadId are episodes belonging to the same release (a season + // pack). Movies (mediaKey === 'movie') are single-record by nature. + if (mediaKey === 'series') { + try { + const groups = new Map(); + for (const r of out) { + const dlId = r && r.downloadId; + if (!dlId) continue; + if (!groups.has(dlId)) groups.set(dlId, []); + groups.get(dlId).push(r); + } + for (const group of groups.values()) { + if (group.length > 1) { + for (const r of group) { + r.isSeasonPack = true; + r.episodeCount = group.length; + } + } + } + } catch (annotateErr) { + logToFile(`[arrQueueHelpers] Season-pack annotation failed: ${annotateErr.message}`); + } + } + + return out; +} + +module.exports = { + buildArrQueueCache +}; diff --git a/server/utils/downloadClients.js b/server/utils/downloadClients.js index 1ddc554..3a6811e 100644 --- a/server/utils/downloadClients.js +++ b/server/utils/downloadClients.js @@ -239,7 +239,10 @@ class DownloadClientRegistry { instanceId: client.getInstanceId(), instanceName: client.name, clientType: client.getClientType(), - status + status, + // Surface the per-client lastError so admins can see transient + // failures (auth expiry, RPC blips, etc.) without scraping logs. + lastError: typeof client.getLastError === 'function' ? client.getLastError() : null }; } catch (error) { logToFile(`[DownloadClientRegistry] Error getting status from ${client.name}: ${error.message}`); @@ -248,7 +251,8 @@ class DownloadClientRegistry { instanceName: client.name, clientType: client.getClientType(), status: null, - error: error.message + error: error.message, + lastError: typeof client.getLastError === 'function' ? client.getLastError() : null }; } }) diff --git a/server/utils/poller.js b/server/utils/poller.js index 0ea85d0..a011a8c 100644 --- a/server/utils/poller.js +++ b/server/utils/poller.js @@ -3,6 +3,7 @@ const axios = require('axios'); const cache = require('./cache'); const { initializeClients, getAllDownloads, getDownloadsByClientType } = require('./downloadClients'); const arrRetrieverRegistry = require('./arrRetrievers'); +const { buildArrQueueCache } = require('./arrQueueHelpers'); const { getSonarrInstances, getRadarrInstances, @@ -237,17 +238,7 @@ async function pollAllServices() { cache.set('poll:sonarr-tags', sonarrTagsResults, cacheTTL); // Tag queue/history records with _instanceUrl so embedded series/movie objects can build links cache.set('poll:sonarr-queue', { - records: sonarrQueues.flatMap(q => { - const inst = sonarrInstances.find(i => i.id === q.instance); - const url = inst ? inst.url : null; - const key = inst ? inst.apiKey : null; - return (q.data.records || []).map(r => { - if (r.series) r.series._instanceUrl = url; - r._instanceUrl = url; - r._instanceKey = key; - return r; - }); - }) + records: buildArrQueueCache(sonarrQueues, sonarrInstances, 'series') }, cacheTTL); cache.set('poll:sonarr-history', { records: sonarrHistories.flatMap(h => h.data.records || []) @@ -265,17 +256,7 @@ async function pollAllServices() { // Radarr if (shouldPollRadarr) { cache.set('poll:radarr-queue', { - records: radarrQueues.flatMap(q => { - const inst = radarrInstances.find(i => i.id === q.instance); - const url = inst ? inst.url : null; - const key = inst ? inst.apiKey : null; - return (q.data.records || []).map(r => { - if (r.movie) r.movie._instanceUrl = url; - r._instanceUrl = url; - r._instanceKey = key; - return r; - }); - }) + records: buildArrQueueCache(radarrQueues, radarrInstances, 'movie') }, cacheTTL); cache.set('poll:radarr-history', { records: radarrHistories.flatMap(h => h.data.records || []) diff --git a/tests/integration/download-matcher-season-pack.test.js b/tests/integration/download-matcher-season-pack.test.js new file mode 100644 index 0000000..347faa9 --- /dev/null +++ b/tests/integration/download-matcher-season-pack.test.js @@ -0,0 +1,204 @@ +// Copyright (c) 2026 Gordon Bolton. MIT License. +// +// Integration tests for the torrent matcher's hash-first matching and +// arrQueueId deduplication paths (Issue #65). These exercise `matchTorrents` +// end-to-end against minimal but realistic queue/history record fixtures. +import { describe, it, expect } from 'vitest'; +import { createRequire } from 'module'; + +const require = createRequire(import.meta.url); +const DownloadMatcher = require('../../server/services/DownloadMatcher'); + +// Build a minimal context. `showAll: true` bypasses per-user tag filtering so +// these tests can assert matching behaviour without setting up the Emby user +// tag plumbing. +function makeContext({ + sonarrQueueRecords = [], + sonarrHistoryRecords = [], + radarrQueueRecords = [], + radarrHistoryRecords = [] +} = {}) { + const seriesMap = new Map(); + const moviesMap = new Map(); + for (const r of sonarrQueueRecords.concat(sonarrHistoryRecords)) { + if (r.series && r.seriesId) seriesMap.set(r.seriesId, r.series); + } + for (const r of radarrQueueRecords.concat(radarrHistoryRecords)) { + if (r.movie && r.movieId) moviesMap.set(r.movieId, r.movie); + } + return { + sonarrQueueRecords, + sonarrHistoryRecords, + radarrQueueRecords, + radarrHistoryRecords, + seriesMap, + moviesMap, + // null tagMaps so extractAllTags uses the object `label` shape from the + // Sonarr fixture (series.tags = [{ label: '...' }]). An empty Map is + // truthy and would cause every id-lookup to return undefined. + sonarrTagMap: null, + radarrTagMap: null, + username: 'tester', + isAdmin: false, + // showAll bypasses per-user tag filtering — we only need it to be truthy + // *together* with non-empty allTags. We seed series/movie tags as non-empty + // strings (Sonarr tag shape) so `extractAllTags` yields entries. + showAll: true, + embyUserMap: new Map(), + ombiBaseUrl: null + }; +} + +const seriesShowA = { + id: 100, + title: 'Show A', + tags: [{ label: 'tester' }], + images: [] +}; + +const movieFilmB = { + id: 200, + title: 'Film B', + tags: [{ label: 'tester' }], + images: [] +}; + +describe('matchTorrents — hash-first matching (#65)', () => { + it('matches a torrent to a Sonarr queue record by hash even when the title differs', async () => { + const hash = 'ABC123HASHsonarr'; + const context = makeContext({ + sonarrQueueRecords: [ + { + id: 9001, + // Title intentionally bears no resemblance to the torrent name to + // prove the match is via hash (downloadId), not title fallback. + title: 'totally.unrelated.queue.record', + sourceTitle: '', + seriesId: 100, + series: seriesShowA, + downloadId: hash, + episodeId: 555 + } + ] + }); + + const torrents = [ + { + hash, + name: 'Show.A.S01.2160p.WEB.x265-RELEASE', + progress: 0.5, + dlspeed: 1000 + } + ]; + + const out = await DownloadMatcher.matchTorrents(torrents, context); + expect(out).toHaveLength(1); + expect(out[0].arrType).toBe('sonarr'); + expect(out[0].arrQueueId).toBe(9001); + }); + + it('matches a Transmission torrent via hashString', async () => { + const hashString = 'TRANSMISSIONHASH456'; + const context = makeContext({ + radarrQueueRecords: [ + { + id: 7777, + title: 'unrelated', + sourceTitle: '', + movieId: 200, + movie: movieFilmB, + downloadId: hashString + } + ] + }); + + const torrents = [ + { + hashString, + name: 'Film.B.2160p.WEB.x265-RELEASE', + progress: 0.25, + dlspeed: 0 + } + ]; + + const out = await DownloadMatcher.matchTorrents(torrents, context); + expect(out).toHaveLength(1); + expect(out[0].arrType).toBe('radarr'); + expect(out[0].arrQueueId).toBe(7777); + }); + + it('falls back to title-substring matching when no hash is present on the torrent', async () => { + const context = makeContext({ + sonarrQueueRecords: [ + { + id: 555, + title: 'Show A', + sourceTitle: '', + seriesId: 100, + series: seriesShowA, + downloadId: null + } + ] + }); + + const torrents = [ + { + // No hash / hashString — title fallback must engage. + name: 'Show A — S01E02', + progress: 0.7, + dlspeed: 5000 + } + ]; + + const out = await DownloadMatcher.matchTorrents(torrents, context); + expect(out).toHaveLength(1); + expect(out[0].arrType).toBe('sonarr'); + expect(out[0].arrQueueId).toBe(555); + }); +}); + +describe('matchTorrents — arrQueueId deduplication (#65)', () => { + it('deduplicates two torrents matching distinct queue records sharing one arrQueueId via the same hash', async () => { + // Construct the pathological case the dedup step is designed for: two + // torrents (post-hash-match) both end up mapped to the same arrQueueId. + // In real life this happens when *arr exposes multiple queue rows under + // one logical download. The first matched download wins; subsequent ones + // are dropped. + const hash = 'PACKHASH001'; + const sharedQueueRow = { + id: 4242, // same arrQueueId + title: 'unrelated', + sourceTitle: '', + seriesId: 100, + series: seriesShowA, + downloadId: hash + }; + + const context = makeContext({ + sonarrQueueRecords: [sharedQueueRow] + }); + + const torrents = [ + { hash, name: 'Show.A.S02E01', progress: 0.1, dlspeed: 0 }, + { hash, name: 'Show.A.S02E02', progress: 0.2, dlspeed: 0 } + ]; + + const out = await DownloadMatcher.matchTorrents(torrents, context); + expect(out).toHaveLength(1); + expect(out[0].arrQueueId).toBe(4242); + }); + + it('does not deduplicate torrents that lack arrQueueId (no matched *arr record)', async () => { + const context = makeContext(); + const torrents = [ + { hash: 'no-match-A', name: 'unmatched-A', progress: 0, dlspeed: 0 }, + { hash: 'no-match-B', name: 'unmatched-B', progress: 0, dlspeed: 0 } + ]; + const out = await DownloadMatcher.matchTorrents(torrents, context); + // Both unmatched torrents are filtered out by the matcher entirely because + // there is nothing to match against — so the deduplicator never sees them. + // This test simply asserts the dedup step itself does not collapse + // non-arr entries into a single bucket when no key is present. + expect(out).toEqual([]); + }); +}); diff --git a/tests/unit/clients/QBittorrentClient.test.js b/tests/unit/clients/QBittorrentClient.test.js index 4c845c9..da0ce4c 100644 --- a/tests/unit/clients/QBittorrentClient.test.js +++ b/tests/unit/clients/QBittorrentClient.test.js @@ -130,6 +130,8 @@ describe('QBittorrentClient', () => { downloaded: 750000000, speed: 1048576, eta: 3600, + seeds: 0, + peers: 0, category: 'movies', tags: ['movie', 'hd'], savePath: '/downloads/test', @@ -138,6 +140,28 @@ describe('QBittorrentClient', () => { }); }); + it('should expose connected seeds/peers from num_seeds and num_leechs (Issue #64)', () => { + const torrent = { + hash: 'def456', + name: 'Swarm Torrent', + state: 'downloading', + progress: 0.1, + size: 1000, + completed: 100, + dlspeed: 0, + eta: 0, + num_seeds: 7, + num_leechs: 3, + // Swarm totals — must NOT be picked up as connected counts + num_complete: 200, + num_incomplete: 50 + }; + + const normalized = client.normalizeDownload(torrent); + expect(normalized.seeds).toBe(7); + expect(normalized.peers).toBe(3); + }); + it('should handle unknown torrent states', () => { const torrent = { hash: 'abc123', diff --git a/tests/unit/clients/RTorrentClient.test.js b/tests/unit/clients/RTorrentClient.test.js index d6f7627..e7b2600 100644 --- a/tests/unit/clients/RTorrentClient.test.js +++ b/tests/unit/clients/RTorrentClient.test.js @@ -420,4 +420,68 @@ describe('RTorrentClient', () => { expect(status).toBeNull(); }); }); + + describe('Null-safety (Issue #68)', () => { + it('should return [] when d.multicall2 returns a non-array', async () => { + mockMethodCall.mockImplementation((method, params, callback) => { + callback(null, null); + }); + const downloads = await client.getActiveDownloads(); + expect(downloads).toEqual([]); + }); + + it('should skip malformed individual torrent rows instead of throwing', async () => { + const torrents = [ + // valid row + ['hashA', 'Name A', 100, 50, 0, 0, 1, 1, 0, '/dl', ''], + // malformed row (not an array) + 'not-an-array', + // row with null/undefined fields + ['hashB', null, null, null, null, null, null, null, null, null, null] + ]; + mockMethodCall.mockImplementation((method, params, callback) => { + callback(null, torrents); + }); + const downloads = await client.getActiveDownloads(); + expect(downloads).toHaveLength(2); + expect(downloads[0].id).toBe('hashA'); + expect(downloads[1].id).toBe('hashB'); + expect(downloads[1].title).toBe(''); + expect(downloads[1].size).toBe(0); + }); + + it('_extractArrInfo should return {} for non-string filename', () => { + expect(client._extractArrInfo(null)).toEqual({}); + expect(client._extractArrInfo(undefined)).toEqual({}); + expect(client._extractArrInfo(123)).toEqual({}); + }); + }); + + describe('lastError tracking (Issue #68)', () => { + it('should record lastError on getActiveDownloads failure', async () => { + mockMethodCall.mockImplementation((method, params, callback) => { + callback(new Error('boom')); + }); + await client.getActiveDownloads(); + expect(client.getLastError()).not.toBeNull(); + expect(client.getLastError().operation).toBe('getActiveDownloads'); + expect(client.getLastError().message).toBe('boom'); + }); + + it('should clear lastError on successful call', async () => { + // First, fail. + mockMethodCall.mockImplementationOnce((method, params, callback) => { + callback(new Error('boom')); + }); + await client.getActiveDownloads(); + expect(client.getLastError()).not.toBeNull(); + + // Then, succeed. + mockMethodCall.mockImplementation((method, params, callback) => { + callback(null, []); + }); + await client.getActiveDownloads(); + expect(client.getLastError()).toBeNull(); + }); + }); }); diff --git a/tests/unit/clients/SABnzbdClient.test.js b/tests/unit/clients/SABnzbdClient.test.js index 915078d..a847d24 100644 --- a/tests/unit/clients/SABnzbdClient.test.js +++ b/tests/unit/clients/SABnzbdClient.test.js @@ -299,4 +299,63 @@ describe('SABnzbdClient', () => { expect(status).toBeNull(); }); }); + + describe('History limit configuration (Issue #68)', () => { + const ORIG_ENV = process.env.SAB_HISTORY_LIMIT; + afterEach(() => { + if (ORIG_ENV === undefined) delete process.env.SAB_HISTORY_LIMIT; + else process.env.SAB_HISTORY_LIMIT = ORIG_ENV; + }); + + it('defaults historyLimit to 10 when SAB_HISTORY_LIMIT is unset', () => { + delete process.env.SAB_HISTORY_LIMIT; + const c = new SABnzbdClient(mockConfig); + expect(c.historyLimit).toBe(10); + }); + + it('honors SAB_HISTORY_LIMIT when set to a valid integer', () => { + process.env.SAB_HISTORY_LIMIT = '25'; + const c = new SABnzbdClient(mockConfig); + expect(c.historyLimit).toBe(25); + }); + + it('falls back to default on invalid SAB_HISTORY_LIMIT', () => { + process.env.SAB_HISTORY_LIMIT = 'not-a-number'; + const c = new SABnzbdClient(mockConfig); + expect(c.historyLimit).toBe(10); + }); + + it('passes historyLimit through to the history API call', async () => { + process.env.SAB_HISTORY_LIMIT = '42'; + const c = new SABnzbdClient(mockConfig); + const makeRequest = vi.fn() + .mockResolvedValueOnce({ data: { queue: { slots: [], kbpersec: 0 } } }) + .mockResolvedValueOnce({ data: { history: { slots: [] } } }); + c.makeRequest = makeRequest; + await c.getActiveDownloads(); + expect(makeRequest).toHaveBeenCalledWith({ mode: 'history', limit: 42 }); + }); + }); + + describe('lastError tracking (Issue #68)', () => { + it('records lastError when getActiveDownloads fails', async () => { + client.makeRequest = vi.fn().mockRejectedValue(new Error('boom')); + await client.getActiveDownloads(); + expect(client.getLastError()).not.toBeNull(); + expect(client.getLastError().operation).toBe('getActiveDownloads'); + expect(client.getLastError().message).toBe('boom'); + }); + + it('clears lastError after a subsequent successful call', async () => { + client.makeRequest = vi.fn().mockRejectedValue(new Error('boom')); + await client.getActiveDownloads(); + expect(client.getLastError()).not.toBeNull(); + + client.makeRequest = vi.fn() + .mockResolvedValueOnce({ data: { queue: { slots: [], kbpersec: 0 } } }) + .mockResolvedValueOnce({ data: { history: { slots: [] } } }); + await client.getActiveDownloads(); + expect(client.getLastError()).toBeNull(); + }); + }); }); diff --git a/tests/unit/clients/TransmissionClient.test.js b/tests/unit/clients/TransmissionClient.test.js index 88870fa..f159dcb 100644 --- a/tests/unit/clients/TransmissionClient.test.js +++ b/tests/unit/clients/TransmissionClient.test.js @@ -154,7 +154,9 @@ describe('TransmissionClient', () => { 4: 'Downloading', 5: 'Queued', 6: 'Seeding', - 7: 'Unknown' + // Issue #63: code 7 is undocumented in the RPC spec; mapped to + // `Checking` (legacy alias for code 2) as a best-effort interpretation. + 7: 'Checking' }; Object.entries(statusMap).forEach(([status, expectedStatus]) => { @@ -433,4 +435,42 @@ describe('TransmissionClient', () => { expect(normalized.arrType).toBeUndefined(); }); }); + + describe('Torrent Control Methods (Issue #63)', () => { + it('startTorrent invokes torrent-start RPC with ids array', async () => { + client.makeRequest = vi.fn().mockResolvedValue({ data: { result: 'success' } }); + await client.startTorrent('abc123'); + expect(client.makeRequest).toHaveBeenCalledWith('torrent-start', { ids: ['abc123'] }); + }); + + it('startTorrent accepts an array of ids', async () => { + client.makeRequest = vi.fn().mockResolvedValue({ data: { result: 'success' } }); + await client.startTorrent([1, 2, 3]); + expect(client.makeRequest).toHaveBeenCalledWith('torrent-start', { ids: [1, 2, 3] }); + }); + + it('stopTorrent invokes torrent-stop RPC', async () => { + client.makeRequest = vi.fn().mockResolvedValue({ data: { result: 'success' } }); + await client.stopTorrent(42); + expect(client.makeRequest).toHaveBeenCalledWith('torrent-stop', { ids: [42] }); + }); + + it('removeTorrent invokes torrent-remove with delete-local-data=false by default', async () => { + client.makeRequest = vi.fn().mockResolvedValue({ data: { result: 'success' } }); + await client.removeTorrent('hashX'); + expect(client.makeRequest).toHaveBeenCalledWith('torrent-remove', { + ids: ['hashX'], + 'delete-local-data': false + }); + }); + + it('removeTorrent passes delete-local-data=true when requested', async () => { + client.makeRequest = vi.fn().mockResolvedValue({ data: { result: 'success' } }); + await client.removeTorrent('hashY', true); + expect(client.makeRequest).toHaveBeenCalledWith('torrent-remove', { + ids: ['hashY'], + 'delete-local-data': true + }); + }); + }); }); diff --git a/tests/unit/downloadClients.test.js b/tests/unit/downloadClients.test.js index 5f8cc00..dde45a5 100644 --- a/tests/unit/downloadClients.test.js +++ b/tests/unit/downloadClients.test.js @@ -310,7 +310,8 @@ describe('DownloadClientRegistry', () => { instanceId: 'sab1', instanceName: 'SAB 1', clientType: 'sabnzbd', - status: { status: 'active' } + status: { status: 'active' }, + lastError: null }); }); diff --git a/tests/unit/utils/arrQueueHelpers.test.js b/tests/unit/utils/arrQueueHelpers.test.js new file mode 100644 index 0000000..6c19cdd --- /dev/null +++ b/tests/unit/utils/arrQueueHelpers.test.js @@ -0,0 +1,142 @@ +// Copyright (c) 2026 Gordon Bolton. MIT License. +// Tests for the shared `buildArrQueueCache` helper (Issue #61). +import { describe, it, expect } from 'vitest'; +import { createRequire } from 'module'; + +const require = createRequire(import.meta.url); +const { buildArrQueueCache } = require('../../../server/utils/arrQueueHelpers'); + +const sonarrInstances = [ + { id: 's1', url: 'http://sonarr-1', apiKey: 'KEY_S1', name: 'Sonarr 1' } +]; +const radarrInstances = [ + { id: 'r1', url: 'http://radarr-1', apiKey: 'KEY_R1', name: 'Radarr 1' } +]; + +describe('buildArrQueueCache', () => { + it('returns empty array for empty / missing input', () => { + expect(buildArrQueueCache([], sonarrInstances, 'series')).toEqual([]); + expect(buildArrQueueCache(null, sonarrInstances, 'series')).toEqual([]); + expect(buildArrQueueCache(undefined, sonarrInstances, 'series')).toEqual([]); + }); + + it('returns empty array for invalid mediaKey', () => { + const queues = [{ instance: 's1', data: { records: [{ id: 1 }] } }]; + expect(buildArrQueueCache(queues, sonarrInstances, 'bogus')).toEqual([]); + }); + + it('tags Sonarr records with _instanceUrl/_instanceKey and decorates embedded series', () => { + const queues = [ + { + instance: 's1', + data: { + records: [ + { id: 1, downloadId: 'dl-1', series: { id: 100, title: 'X' } } + ] + } + } + ]; + const out = buildArrQueueCache(queues, sonarrInstances, 'series'); + expect(out).toHaveLength(1); + expect(out[0]._instanceUrl).toBe('http://sonarr-1'); + expect(out[0]._instanceKey).toBe('KEY_S1'); + expect(out[0].series._instanceUrl).toBe('http://sonarr-1'); + }); + + it('tags Radarr records and decorates embedded movie', () => { + const queues = [ + { + instance: 'r1', + data: { + records: [ + { id: 11, downloadId: 'dl-r1', movie: { id: 555, title: 'M' } } + ] + } + } + ]; + const out = buildArrQueueCache(queues, radarrInstances, 'movie'); + expect(out).toHaveLength(1); + expect(out[0]._instanceUrl).toBe('http://radarr-1'); + expect(out[0]._instanceKey).toBe('KEY_R1'); + expect(out[0].movie._instanceUrl).toBe('http://radarr-1'); + }); + + it('annotates Sonarr season pack records (multiple entries sharing downloadId)', () => { + const queues = [ + { + instance: 's1', + data: { + records: [ + { id: 1, downloadId: 'pack-A', episodeId: 101 }, + { id: 2, downloadId: 'pack-A', episodeId: 102 }, + { id: 3, downloadId: 'pack-A', episodeId: 103 }, + { id: 4, downloadId: 'single-B', episodeId: 200 } + ] + } + } + ]; + const out = buildArrQueueCache(queues, sonarrInstances, 'series'); + expect(out).toHaveLength(4); + const packMembers = out.filter(r => r.downloadId === 'pack-A'); + expect(packMembers).toHaveLength(3); + for (const r of packMembers) { + expect(r.isSeasonPack).toBe(true); + expect(r.episodeCount).toBe(3); + } + const single = out.find(r => r.downloadId === 'single-B'); + expect(single.isSeasonPack).toBeUndefined(); + expect(single.episodeCount).toBeUndefined(); + }); + + it('does not annotate Radarr records as season packs even if downloadId repeats', () => { + const queues = [ + { + instance: 'r1', + data: { + records: [ + { id: 1, downloadId: 'dup', movie: { id: 1 } }, + { id: 2, downloadId: 'dup', movie: { id: 2 } } + ] + } + } + ]; + const out = buildArrQueueCache(queues, radarrInstances, 'movie'); + expect(out).toHaveLength(2); + for (const r of out) { + expect(r.isSeasonPack).toBeUndefined(); + expect(r.episodeCount).toBeUndefined(); + } + }); + + it('skips malformed records and continues', () => { + const queues = [ + { + instance: 's1', + data: { + records: [ + null, + { id: 1, downloadId: 'ok', series: { id: 1 } } + ] + } + }, + null, + { instance: 's1' } // no data property + ]; + const out = buildArrQueueCache(queues, sonarrInstances, 'series'); + expect(out).toHaveLength(1); + expect(out[0].id).toBe(1); + }); + + it('handles unknown instance id gracefully (null url/key)', () => { + const queues = [ + { + instance: 'unknown-instance', + data: { records: [{ id: 1, downloadId: 'x' }] } + } + ]; + const out = buildArrQueueCache(queues, sonarrInstances, 'series'); + expect(out).toHaveLength(1); + expect(out[0]._instanceUrl).toBeNull(); + expect(out[0]._instanceKey).toBeNull(); + }); +});