From 6b73727d4eec7b02a1796945bf696ab9d817a737 Mon Sep 17 00:00:00 2001 From: Gronod Date: Thu, 28 May 2026 15:36:33 +0100 Subject: [PATCH] fix(queue): extract shared arr cache helper, annotate season packs, null-guard flatMap (closes #61) --- CHANGELOG.md | 1 + server/routes/webhook.js | 25 +--- server/utils/arrQueueHelpers.js | 97 ++++++++++++++++ server/utils/poller.js | 25 +--- tests/unit/utils/arrQueueHelpers.test.js | 142 +++++++++++++++++++++++ 5 files changed, 246 insertions(+), 44 deletions(-) create mode 100644 server/utils/arrQueueHelpers.js create mode 100644 tests/unit/utils/arrQueueHelpers.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a28d21..c872f82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed +- **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). - **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/routes/webhook.js b/server/routes/webhook.js index e759c53..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'); @@ -207,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))`); } @@ -237,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))`); } 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/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/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(); + }); +});