From 6fa9c79a7dd545ae7d35a4abdd4ecaba3ef2a1b2 Mon Sep 17 00:00:00 2001 From: Gronod Date: Thu, 28 May 2026 16:22:11 +0100 Subject: [PATCH] fix: rTorrent null-safety, configurable SAB_HISTORY_LIMIT, lastError visibility (#68) - RTorrentClient: guard d.multicall2 returning non-array, per-row try/catch, explicit Number()/String() coercions, _extractArrInfo null-safe - RTorrentClient.getClientStatus: coerce rates through Number.isFinite - SABnzbdClient: history limit now reads SAB_HISTORY_LIMIT env var (default 10) - DownloadClient: added _recordLastError, _clearLastError, getLastError on base - All four clients call _recordLastError on failure, _clearLastError on success - DownloadClientRegistry.getAllClientStatuses: includes lastError in result - GET /api/status/status: exposes downloadClients[] array with per-client lastError - Tests: RTorrentClient null-safety + lastError, SABnzbd history limit + lastError, downloadClients.test expectation updated for new lastError field --- CHANGELOG.md | 1 + server/clients/DownloadClient.js | 35 ++++++++++ server/clients/QBittorrentClient.js | 6 ++ server/clients/RTorrentClient.js | 78 +++++++++++++++++++---- server/clients/SABnzbdClient.js | 33 +++++++++- server/clients/TransmissionClient.js | 7 +- server/routes/status.js | 12 +++- server/utils/downloadClients.js | 8 ++- tests/unit/clients/RTorrentClient.test.js | 64 +++++++++++++++++++ tests/unit/clients/SABnzbdClient.test.js | 59 +++++++++++++++++ tests/unit/downloadClients.test.js | 3 +- 11 files changed, 284 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 992bb16..d99922e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **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 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 79c6aa9..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; } } @@ -174,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}`); @@ -188,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 []; } } @@ -198,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, @@ -205,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; } } 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 2943fef..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; } } 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/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/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/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 }); });