feat: webhooks - #191
Conversation
Add the shared webhook data types (NotificationType, DiscordMentionType, WebhookKeyValue, WebhookDestination, WebhookItemTypes, WebhookDto, WebhookTestResult) and the six API client endpoints under /remux/webhooks that the server, dashboard, and dispatcher will build on.
The endpoint test asserted only four of the six webhook endpoints, leaving CreateWebhook and UpdateWebhook entirely unchecked. Assert path and method for all six, and add body tests proving both mutating endpoints send the serialized WebhookDto as a JSON body.
Add the webhooks SQLite table and the Webhook repository backing it. The destination, notification_types, user_filter, and item_types columns are stored as JSON and decoded through #[sqlx(json)], so the tagged WebhookDestination enum and the key/value lists round-trip unchanged. create assigns a fresh uuid and ignores the id carried by the incoming DTO; update rewrites every mutable column while preserving created_at and bumping updated_at. Timestamps are bound from Utc::now() rather than left to the column defaults so they keep sub-second resolution.
send_all_properties, trim_whitespace and skip_empty_message_body are bound to three adjacent same-typed placeholders in create and update. The fixture set all three to true and the update patch set all three to false, so any permutation of those binds still passed every test. Give the three bools distinct values in the fixture and the inverse in the update patch, and assert them literally on the rows read back from the database. Swapping any two binds now fails an assertion.
Three booleans range over two values, so any single fixture leaves one symmetric pair. The previous true/false/true shape has send_all_properties and skip_empty_message_body both true, so swapping those two binds wrote the same values into the same columns and stayed invisible in both create and update. Add FLAG_CASES, the three one-hot combinations, and drive create and update through each of them. Every case detects the two swaps involving its single true, so the three together cover all three pairwise swaps. update is seeded with the inverse triple first, forcing all three columns to be written. Verified by applying each of the three swaps to create and to update in turn: all six now fail, where the two 1<->3 swaps previously passed.
Add WebhookEvent, the internal event type covering the fifteen notification types, and WebhookService, which owns a broadcast channel and the background task that turns events into deliveries. Events carry the data already in hand where they are raised, so emitting is a non-blocking send on a bounded broadcast channel and never touches the database. ItemDeleted boxes the media row because it has to be captured before the DELETE runs. The dispatcher owns the only receiver. It caches the enabled webhooks, the union of the notification types they subscribe to, and a Handlebars registry; the CRUD endpoints will mark that snapshot dirty and it is reloaded before the next event. A lagged receive is logged and skipped rather than ending the loop, and each delivery is spawned so one slow endpoint cannot stall the hooks behind it. matches() is a pure function over the three filter rules. An empty notification_types list matches nothing, mirroring the Jellyfin webhook plugin. An empty user_filter accepts every user, and events that carry no user are exempt from it entirely. The item_types toggles apply only when the event is about an item, mapping Movie, Episode, Series, Season and Album to their own flags, Track to songs, and every other kind to videos. payload, template and sender are minimal stubs so this compiles on its own; payload building, Handlebars helpers and HTTP delivery follow.
spawn_dispatcher called tx.subscribe() inside the spawned task. tokio::spawn only queues the task, and a broadcast channel discards sends made while it has no subscriber, so every event emitted between the spawn in init_app and the task's first poll was dropped. run_startup_tasks runs immediately after that spawn and is the path that will emit ItemAdded, so this would have turned into silently lost startup events. Create the receiver before the spawn and move it into the task. Use PlayMethod for PlaybackEventData.play_method instead of Option<String>. The enum already exists in remux-sdks with EnumString and Display, and it is the value already in hand at the emission site, so a typo can no longer reach a customer webhook unnoticed. Disable handlebars' default features. Its only default is preserve_json_order, which turns on serde_json/preserve_order for the whole build graph and changes serde_json::Map from sorted to insertion order everywhere. Under resolver 3 that also made map semantics depend on whether the build was driven from the workspace or from the dashboard crate. Nothing needed here is feature-gated, and indexmap drops back out of serde_json in the lock file. Record the cache invariant on reload: the dispatcher task is the only writer, so holding the read guard across enrich_item cannot deadlock. Also apply cargo fmt, which the previous commit left dirty in four places.
Strict parity with jellyfin-plugin-webhook's DiscordClient: the destination's options are injected into the handlebars dictionary (MentionType, EmbedColor, AvatarUrl, Username, BotUsername) and the operator's template renders the whole Discord payload, so a template copied from the plugin works verbatim. The server-side envelope in the sender is removed.
…retry only transient failures - never log a webhook URL path or query: it is the credential (Discord's token lives there). Scheme and host only, plus the hook id. Also strips the URL that reqwest::Error embeds in its own Display. - delivery slots are keyed by hook id instead of a single global pool, so one blackholing endpoint can no longer starve every other webhook. - retry only transport errors, 5xx, 408 and 429, and honour Retry-After (and Discord's X-RateLimit-Reset-After) instead of hammering a rate limit. - always expose EmbedColor to Discord templates, defaulted, so the plugin's stock template cannot render an invalid "color": "". - skip generic destination fields with an empty key or value, as the plugin does. - tests now assert the bytes actually posted, and cover both branches of the bounded spawn.
…on 429 Duration::from_secs_f64 panics outside Duration's range and the cap was applied after the conversion, so a remote endpoint answering with `Retry-After: 1e30` panicked the delivery task. Clamp the f64 first. Also restrict the rate-limit headers to 429: Discord sends X-RateLimit-Reset-After on responses generally, so a 5xx carrying 0 was collapsing the exponential backoff into three immediate retries.
…results on a failed delete
Config::torrent_peer_port defaults to Some(6881), which becomes the ten-port
listen range 6881..6891 — a process-wide cap of about ten concurrent test
servers, and the whole reason the crate's tests only passed under
--test-threads=1. Leave it unset in the test config so no fixed peer port is
claimed; the suite is now green under default parallelism.
Also swap the webhook filter test's positive hook from {{Name}}, which
a_playback_start_reaches_a_configured_webhook already pins byte for byte, to
{{NotificationUsername}} — the variable a playback event routes through
From<&db::User> for UserEventData, pinned end to end nowhere else.
Five cross-task issues found by the final review, plus four cheap minors.
- The stock Discord template used the plugin's `{{{triple}}}` interpolations,
which the plugin needs because its Handlebars escapes for HTML. remux escapes
for JSON instead, so a title containing `"` or `\` rendered a body Discord
answered 400 to — Fatal, so no retry, one warn, and nothing for the operator.
All seven are now double braces. The constant moved to remux-sdks so the
server can render the very template the dashboard ships, which is the test
gap that let this through: nothing anywhere exercised it.
- A DB failure during the startup reload left the cache empty with `dirty`
down, disabling every webhook until an admin touched one or the process
restarted. The error branch re-raises the flag, as its doc comment already
promised.
- A template syntax error reached the operator as "Template not found: <uuid>",
and was never rejected at write time. `test_body` now compiles the single
template directly and propagates the parse error, and create/update refuse an
unparseable template with a 400 carrying handlebars' own message — derived
from the operator's template, so nothing leaks.
- The saturation warning was reachable from an unauthenticated caller through
the AuthenticationFailure emission, one warn line per dropped delivery. It and
the per-event render-failure warning are now throttled to one line per hook
per minute, carrying a suppressed count.
- A failed webhook test logged nothing at all — `deliver_logged` is not on that
path, contrary to its doc comment — so the operator saw a bare status and the
server saw nothing. It now writes its own warn under the same redaction, and
the Discord section documents that `{{ServerUrl}}` needs `public_url`.
Minors: replace the mock-swap retry test with a call-count endpoint (the
branch's likeliest CI flake), skip item-scoped events whose item could not be
resolved instead of firing past the item-type filter with an empty body, correct
the reload comment about `dirty`, and note that `public_url`'s env var is the
bare `PUBLIC_URL`.
# Conflicts: # crates/remux-server/src/api/users.rs # crates/remux-server/src/db/mod.rs # crates/remux-server/src/lib.rs
…db field The upstream merge dropped ExternalIds::series_imdb. Season and episode ids derive from the series' external ids plus their season/episode numbers, so the child rows now carry the series' ids directly.
| /// the operator's own template — never from a remote response — so it is safe | ||
| /// to hand back over the API. | ||
| pub(crate) fn validate(template: &str) -> Result<(), handlebars::TemplateError> { | ||
| Handlebars::new().register_template_string(VALIDATION_NAME, template) |
There was a problem hiding this comment.
validate() uses a bare Handlebars::new() while delivery uses fresh_registry().
Helper typos pass save-time validation and silently drop every delivery at render time. Should use fresh_registry() in validate() too.
| { | ||
| for item in new_items.iter() { | ||
| ctx.webhooks | ||
| .emit(WebhookEvent::ItemAdded { item_id: item.id }); |
There was a problem hiding this comment.
Per-item emission can overflow the 4096-cap broadcast channel on large scans. RecvError::Lagged is only logged, so events are silently dropped. Worth batching ItemAdded instead.
| tokio::spawn(async move { | ||
| // Held for the whole delivery, retries included: the slot is the | ||
| // ceiling on work owed to one endpoint, not on one HTTP round-trip. | ||
| let _permit = permit; |
There was a problem hiding this comment.
The permit is held across retries and Retry-After sleeps. In a 429 burst all 4 slots can be pinned sleeping, dropping new deliveries for the hook until the retries finish.
| // Ask for another attempt: the flag was consumed before the | ||
| // call, so nothing else will set it. | ||
| self.inner | ||
| .dirty |
There was a problem hiding this comment.
reload() re-sets dirty and immediately retries on the next inbound event when DB access fails. During an outage, that effectively becomes one failing DB query per playback event. 💥
Can we add retry backoff to reload failures?
| /// | ||
| /// TODO: entries are never removed. `WebhookService::reload` in `mod.rs` knows | ||
| /// the live hook set and is the natural place to prune from. | ||
| pub(crate) struct DeliverySlots { |
There was a problem hiding this comment.
DeliverySlots never gets pruned, so removed/rotated webhooks leave stale entries and the map can grow unbounded over time (memory leaks). Worth pruning on dirty reload (build active webhook keys, then retain matching entries).
| /// | ||
| /// Hand-written, so a variant added to the SDK must be added here too — the | ||
| /// array's declared length is what pins the count. | ||
| const NOTIFICATION_TYPES: [NotificationType; 15] = [ |
There was a problem hiding this comment.
We can derive EnumIter on NotificationType and replace this const list with NotificationType::iter(). This keeps the checkbox list automatically in sync, so a new variant won't silently disappear from the UI.
|
|
||
| /// The content type to send when the operator has not named one. A deviation | ||
| /// from the plugin, which sends everything as `text/plain`. | ||
| pub(crate) fn detect_content_type(body: &str) -> &'static str { |
There was a problem hiding this comment.
detect_content_type parses the full body as JSON just to set a header, then discards the result. Might be worth using a simple first non-whitespace byte check instead to avoid unnecessary parsing work.
|
|
||
| /// `base * 2^attempt` plus jitter in `[0, base/2)`, mirroring | ||
| /// `remux_utils::retry!`. | ||
| fn backoff(base_ms: u64, attempt: u32) -> Duration { |
There was a problem hiding this comment.
backoff() duplicates the remux_utils::retry! formula. Consider extracting to a shared utility to keep both in sync.
`retry!` computed `base * 2^attempt` plus jitter inline, and the webhook sender re-derived the same formula because it retries only *some* failures and cannot use the macro. Two copies of one curve drift. `retry::backoff(base_ms, attempt)` is now the single definition and the macro calls it, so the hand-rolled loops can too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MA1Q5i4k7y7K58nKfADW4
The dashboard's subscription checkboxes came from a hand-written array of all 15 variants, pinned only by its declared length. A variant added to the SDK had to be added there too or it silently disappeared from the UI. `NotificationType::iter()` now feeds the list, in the same declaration order the form already treated as canonical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018MA1Q5i4k7y7K58nKfADW4
…tries
Five review findings on the webhook service. They land together because
they interlock through `mod.rs` signatures.
- Template validation only parsed. Handlebars resolves a helper name at
*render* time, so `{{url_encod Name}}` saved cleanly and then dropped
every delivery. `validate` now renders against the synthetic payload the
admin test button uses, through a registry that carries the custom
helpers. A stored template with a helper typo can no longer be saved
until it is fixed.
- A library scan emitted `ItemAdded` faster than the dispatcher drains it,
overflowing the 4096-event channel into a `Lagged` line. `Pacer` slows
the burst instead of batching it, which would break the one-event-per-item
contract the plugin's templates rely on. Two bounds keep a sick
dispatcher from stalling the scan.
- A failed `reload` left `dirty` raised, so a database outage became one
failing query per event — at a rate an unauthenticated caller can drive
through `AuthenticationFailure`. Retries now back off to 8s.
- `DeliverySlots` never dropped an entry, so a deleted hook kept its
semaphore until restart. `reload` prunes against the live hook set,
keeping any entry whose permit is still out.
- `detect_content_type` built a whole `Value` tree to answer a yes/no
question; `IgnoredAny` runs the same parser without it.
The delivery permit still spans retries and `Retry-After` sleeps, which
review flagged. That is deliberate and now documented: releasing it would
turn dropped events into tasks parked on a semaphore, and keep pushing at
an endpoint that just asked us to stop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MA1Q5i4k7y7K58nKfADW4
|
Nice, looking good. I'll test it out. |
…dden The nested-collection refactor (lostb1t#205) dropped the two branches that populate child_count for Collection rows (manual membership via media_relations, and the per-collection smart/catalog COUNT). With child_count left as None, exclude_childless retained every collection, so an empty promoted smart collection showed up in /UserViews again. Both doc comments still promised empty smart/catalog collections get dropped, and the /UserViews handler still passes exclude_childless, so this was collateral damage rather than an intended behavior change. Restores both branches and realigns the retain-block comment. Collection stays out of the parent_id child-count list on purpose: in the new model a collection's parent_id children are nested collections, not content, and group containers short-circuit the retain anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HH1WVbqGvyo97ojxuwmjUr
# Conflicts: # crates/remux-server/src/db/media.rs
|
@lostb1t Do you want me to close this ? And we will do this later ? |
|
You can leave it open. Ill get to it eventually but currently other things have priority |
Try to implement the Webhooks #186
remux doesn't have Jellyfin's plugin system and isn't going to get one, but the thing people actually use plugins for is webhooks piping library and playback events into Discord, n8n, Home Assistant, whatever. So this adds them natively.
The goal was to be a drop-in replacement for jellyfin-plugin-webhook: same event names, same template variables, same Handlebars helpers. If you have a Discord template from the plugin, you should be able to paste it in and have it work.
What's in it
Server events go through an internal broadcast channel to a background dispatcher, which filters per webhook, builds the payload dictionary, renders the operator's template and delivers over HTTP.
Plugin*events, PendingRestart and SubtitleDownloadFailure nothing in remux maps to them. UserLockedOut is out too, we don't have lockout.if_equals,if_exist,link_to,url_encode,json_encode) and theSendAllProperties/TrimWhitespace/SkipEmptyMessageBodyoptions./remux/webhooks(additive, lowercase, doesn't touch the Jellyfin surface) plus a/testendpoint for the dashboard button.Delivery is fire-and-forget with a short retry: transport errors, 5xx, 408 and 429 get three attempts with exponential backoff, and 429 honours
Retry-After. Fatal 4xx aren't retried. Deliveries are bounded to 4 in flight per webhook deliberately per-hook and not global, so one dead Discord URL can't starve your working Gotify hook.Deliberate deviations from the plugin
Worth knowing when comparing behaviour:
{{{triple}}}to opt out. Ours escapes",\and control chars instead, so the shipped Discord template uses{{double}}otherwise a movie calledThe "Burbsproduces invalid JSON and Discord 400s.{{{triple}}}is still there as the raw escape hatch.FormatColorCodesliceshexCode[1..6]and silently drops the last hex digit, so#AA5CC3renders as a different colour. Ours parses all six.text/plain. An explicitContent-Typeheader still wins.Discord itself follows the plugin exactly: the destination options are injected as template variables and the operator's template renders the whole payload. No server-side envelope.
New config
public_urlthe server's own externally reachable URL. The stock Discord template uses it for the thumbnail and the deep link, and it renders empty when unset (I'd rather ship an empty string than guess wrong). Note the env var is the barePUBLIC_URL, which some environments already set.Drive-by fix
The test harness wasn't overriding
torrent_peer_port, so every test server tried to bind in 6881..6891 and we were capped at ~10 concurrent. That's whycargo test -p remux-serverneeded--test-threads=1. Fixed the suite now runs in parallel, 10s instead of 79s.Screenshots