Gandalf/sofarr
Public Access
0
0
Fork 0
Code Issues 1 Pull Requests Actions 1 Packages Projects Releases 59 Wiki Activity

BUG: Webhook regression after #62 — Ombi handler broken due to isReplay() signature change #70

New Issue
Closed
opened 2026-05-28 17:30:29 +01:00 by Gandalf · 1 comment
Gandalf commented 2026-05-28 17:30:29 +01:00
Owner
Copy Link
Copy Source

Related Issues:

  • #62 (Webhook reliability — replay key redesign)
  • #61 (Season packs + webhook-triggered queue refreshes)

Description

After the changes in #62 (commit 593ad7967), webhooks are no longer working reliably. Specifically:

  • Real Ombi webhooks are failing or being incorrectly treated as duplicates.
  • The webhook test buttons (especially for Ombi) now report errors.
  • Sonarr and Radarr webhooks appear to work, but Ombi is broken.

This is a regression introduced by an incomplete update when the isReplay() function signature was changed.

Root Cause

In commit 593ad7967 (fix for #62), the isReplay() function was updated from 3 parameters to 4 parameters:

Before (v1.7.31):

function isReplay(eventType, instanceName, eventDate)

After (develop):

function isReplay(eventType, instanceName, eventDate, contentId)

However, the Ombi handler was never updated.

Current Broken Code (Ombi handler in server/routes/webhook.js):

const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString();

if (isReplay(eventType, instanceName, `${requestId}-${eventDate}`)) {
    logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`);
    return res.status(200).json({ received: true, duplicate: true });
}

Problems:

  1. Only 3 arguments are passed instead of 4.
  2. The third argument is a composite string (${requestId}-${eventDate}) instead of a proper eventDate.
  3. contentId becomes undefined, breaking the new replay key format:
    eventType:instanceName:contentId:eventDate

This causes malformed replay keys, incorrect duplicate detection, and failures in the Ombi webhook path (including the test simulation endpoint).

Investigation Findings

  • Sonarr & Radarr handlers: Correctly updated to pass 4 arguments with proper contentId extraction (downloadId or series.id/movie.id).
  • Ombi handler: Still using the old 3-argument call pattern.
  • The /api/ombi/webhook/test endpoint (in server/routes/ombi.js) internally POSTs to /api/webhook/ombi, so it is directly affected by this bug.
  • No other files appear to call isReplay() with the wrong signature.

Impact

  • Ombi integration is broken — real webhooks and test buttons fail.
  • Users relying on Ombi for request tracking see no updates.
  • This undermines the reliability improvements introduced in #62.
  • Potential for missed or duplicate Ombi events in the dashboard.

How to Resolve (Exact Fix)

Update the Ombi handler in server/routes/webhook.js to use the new isReplay() signature consistently with Sonarr/Radarr:

Recommended Patch:

Replace the current Ombi replay check (around line 819) with:

const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString();
const contentId = requestId || null;   // Use requestId as content identifier for Ombi

if (isReplay(eventType, instanceName, eventDate, contentId)) {
    logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`);
    return res.status(200).json({ received: true, duplicate: true });
}

Full Context (recommended replacement block):

// === Ombi Handler ===
const notificationType = req.body.notificationType || req.body.NotificationType;
const eventType = notificationType || req.body.eventType || req.body.EventType;

if (!eventType || !OMBI_EVENTS.has(eventType)) {
    return res.status(400).json({ error: 'Invalid or missing notificationType' });
}

const instanceName = req.body.applicationUrl || req.body.ApplicationUrl || 'ombi';
const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString();
const requestId = req.body.requestId || req.body.RequestId;
const contentId = requestId || null;

if (isReplay(eventType, instanceName, eventDate, contentId)) {
    logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`);
    return res.status(200).json({ received: true, duplicate: true });
}

Suggested Commit Message

fix(webhooks): correct Ombi isReplay() call after signature change (closes #69)

Suggested Labels

  • Kind/Bug
  • Priority: High
  • Area/Webhooks
  • Regression

Cross-References

  • Caused by incomplete fix in #62
  • Affects Ombi webhook functionality introduced/fixed in #61
  • Blocks full reliability of real-time updates

This is a quick, low-risk fix. Once applied, Ombi webhooks and test buttons should work correctly again.

**Related Issues:** - #62 (Webhook reliability — replay key redesign) - #61 (Season packs + webhook-triggered queue refreshes) --- ## Description After the changes in **#62** (commit `593ad7967`), webhooks are no longer working reliably. Specifically: - Real Ombi webhooks are failing or being incorrectly treated as duplicates. - The **webhook test buttons** (especially for Ombi) now report errors. - Sonarr and Radarr webhooks appear to work, but Ombi is broken. This is a **regression** introduced by an incomplete update when the `isReplay()` function signature was changed. --- ## Root Cause In commit `593ad7967` (fix for #62), the `isReplay()` function was updated from 3 parameters to **4 parameters**: **Before (v1.7.31):** ```js function isReplay(eventType, instanceName, eventDate) ``` **After (develop):** ```js function isReplay(eventType, instanceName, eventDate, contentId) ``` **However, the Ombi handler was never updated.** ### Current Broken Code (Ombi handler in `server/routes/webhook.js`): ```js const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString(); if (isReplay(eventType, instanceName, `${requestId}-${eventDate}`)) { logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`); return res.status(200).json({ received: true, duplicate: true }); } ``` ### Problems: 1. Only **3 arguments** are passed instead of 4. 2. The third argument is a composite string (`${requestId}-${eventDate}`) instead of a proper `eventDate`. 3. `contentId` becomes `undefined`, breaking the new replay key format: `eventType:instanceName:contentId:eventDate` This causes malformed replay keys, incorrect duplicate detection, and failures in the Ombi webhook path (including the test simulation endpoint). --- ## Investigation Findings - **Sonarr & Radarr handlers**: Correctly updated to pass 4 arguments with proper `contentId` extraction (`downloadId` or `series.id`/`movie.id`). - **Ombi handler**: Still using the old 3-argument call pattern. - The `/api/ombi/webhook/test` endpoint (in `server/routes/ombi.js`) internally POSTs to `/api/webhook/ombi`, so it is directly affected by this bug. - No other files appear to call `isReplay()` with the wrong signature. --- ## Impact - **Ombi integration is broken** — real webhooks and test buttons fail. - Users relying on Ombi for request tracking see no updates. - This undermines the reliability improvements introduced in #62. - Potential for missed or duplicate Ombi events in the dashboard. --- ## How to Resolve (Exact Fix) Update the Ombi handler in `server/routes/webhook.js` to use the new `isReplay()` signature consistently with Sonarr/Radarr: ### Recommended Patch: **Replace** the current Ombi replay check (around line 819) with: ```js const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString(); const contentId = requestId || null; // Use requestId as content identifier for Ombi if (isReplay(eventType, instanceName, eventDate, contentId)) { logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`); return res.status(200).json({ received: true, duplicate: true }); } ``` ### Full Context (recommended replacement block): ```js // === Ombi Handler === const notificationType = req.body.notificationType || req.body.NotificationType; const eventType = notificationType || req.body.eventType || req.body.EventType; if (!eventType || !OMBI_EVENTS.has(eventType)) { return res.status(400).json({ error: 'Invalid or missing notificationType' }); } const instanceName = req.body.applicationUrl || req.body.ApplicationUrl || 'ombi'; const eventDate = req.body.requestedDate || req.body.RequestedDate || new Date().toISOString(); const requestId = req.body.requestId || req.body.RequestId; const contentId = requestId || null; if (isReplay(eventType, instanceName, eventDate, contentId)) { logToFile(`[Webhook] Ombi duplicate event ignored: ${eventType} requestId=${requestId}`); return res.status(200).json({ received: true, duplicate: true }); } ``` --- ## Suggested Commit Message ``` fix(webhooks): correct Ombi isReplay() call after signature change (closes #69) ``` --- ## Suggested Labels - `Kind/Bug` - `Priority: High` - `Area/Webhooks` - `Regression` --- ## Cross-References - Caused by incomplete fix in **#62** - Affects Ombi webhook functionality introduced/fixed in **#61** - Blocks full reliability of real-time updates --- **This is a quick, low-risk fix.** Once applied, Ombi webhooks and test buttons should work correctly again.
Gandalf added the Kind/Bug
Priority
High
2
 labels 2026-05-28 17:30:29 +01:00
Gandalf commented 2026-05-28 17:40:47 +01:00
Author
Owner
Copy Link
Copy Source

Resolved in commit a4d398e.

Resolved in commit a4d398e.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Area/Docker
Docker image and container packaging
 Area/Download Clients
Download client integrations
 Area/Frontend
Client-side UI and build tooling
 Area/History
History and completed downloads views
 Area/Logging
Log streaming, file handling, and debug infrastructure
 Area/Matching
Matching and correlation logic
 Area/Proxy
Upstream service proxy routes
 Area/SSE
Server-Sent Events and real-time streaming
 Area/Webhooks
Webhook processing and reliability
 Compat/Breaking
Breaking change that won't be backward compatible
 Compat/Non-Breaking
Non-breaking compatibility change
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label Kind/Bug
Priority
High
2
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Gandalf
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Gandalf/sofarr#70
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?