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

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;
   }
 
   /**

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;
     }
   }

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' };

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;
     }
   }

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;
     }
   }