Repository navigation
feat(live-notif): youtube polling, message ttl cleanup, api backoff - #1762
Conversation
…L, backoff Close spec requirements for live-notification service: - GAP 1: Twitch poll interval 2min→5min (TWITCH_POLL_INTERVAL_MS const) - GAP 2: YouTube polling every 10min (channels.list + search.list; quota math documented: 14.4k units/day exceeds free 10k quota) - GAP 3: Message tracking + TTL cleanup (in-memory Map with 4h expiry; future: Prisma LiveNotificationMessage model added to schema) - GAP 4: Exponential backoff on 429/5xx with Retry-After respects (backoffFetch helper, max 3 attempts, max 30s delay with jitter) YouTube env: YOUTUBE_API_KEY + YOUTUBE_CHANNEL_ID (both optional; absent→disabled with log) Dedup per streamId/broadcastId (lastNotifiedStreamId/lastNotifiedYoutubeBroadcastId) Message deletion: fail-soft on 404 (Unknown Message) or Discord API errors Tests: 27 passing (interval constants, YouTube dedup+offline, TTL cleanup, message tracking, backoff on 429/503, Retry-After, Twitch baseline)
One search.list call (channelId+eventType=live) at 30-min interval: 4.8k units/day, inside the 10k/day free quota. Drops the broken forUsername/playlistId params and the unused Prisma model+migration. Issue #130 asked 10-min polling; quota math makes 30-min the safe interval (documented in code).
📝 WalkthroughWalkthroughThe notification service now polls Twitch and YouTube independently, retries rate-limited or failed API requests, posts and deduplicates both notification types, tracks Discord messages for TTL cleanup, and expands lifecycle and error-path coverage. ChangesLive notification service
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PollingScheduler
participant NotificationService
participant TwitchAPI
participant YouTubeDataAPI
participant DiscordTextChannel
PollingScheduler->>NotificationService: run Twitch tick
NotificationService->>TwitchAPI: fetch current stream with retry
TwitchAPI-->>NotificationService: stream result
NotificationService->>DiscordTextChannel: send Twitch embed
PollingScheduler->>NotificationService: run YouTube tick
NotificationService->>YouTubeDataAPI: search for live broadcast with retry
YouTubeDataAPI-->>NotificationService: broadcast result
NotificationService->>DiscordTextChannel: send YouTube embed
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Sonar gate: new_security_rating flagged Math.random jitter (S2245 x2); replaced with crypto.randomInt (not security-sensitive, gate-compliant). new_coverage 78.1% -> adds 6 tests: start/stop lifecycle, tick error paths, youtube search mapping and non-ok response.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/bot/src/services/CriativariaLiveNotificationService.ts (1)
187-197: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDelete live announcements on offline
checkAndNotifyTwitchandcheckAndNotifyYoutubeonly clear the last-seen IDs when the stream ends; the posted “🔴 está ao vivo” message still stays until the 4h TTL cleanup runs. Delete the tracked message immediately on offline, or viewers can see a stale live notice for hours.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.ts` around lines 187 - 197, When no stream is returned in checkAndNotifyTwitch and checkAndNotifyYoutube, delete the previously tracked live-announcement message before clearing its stream/message tracking state. Reuse the existing message deletion mechanism and safely handle missing messages, ensuring the offline path removes the announcement immediately rather than waiting for TTL cleanup.
🧹 Nitpick comments (3)
packages/bot/src/services/CriativariaLiveNotificationService.ts (2)
326-342: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
messagesToDelete.push(msgId).
msgIdis already pushed unconditionally at Line 326, so the push inside thecatchat Line 341 is a duplicate. Harmless (the final map delete is idempotent) but confusing.♻️ Proposed tidy-up
} else { warnLog({ message: `CriativariaLiveNotification: failed to delete message ${msgId}`, error: err, }) } - messagesToDelete.push(msgId)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.ts` around lines 326 - 342, Remove the redundant messagesToDelete.push(msgId) inside the catch block of the message deletion logic; messagesToDelete is already updated before the try block. Keep the existing error handling and warnLog behavior unchanged.
129-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winYouTube startup guard is inconsistent with the intended contract; comment is stale.
The PR states YouTube is disabled when
YOUTUBE_API_KEYorYOUTUBE_CHANNEL_IDis absent, butstart()only checksYOUTUBE_API_KEY. With the key set but the channel id missing, the 30‑min interval is registered even thoughcheckAndNotifyYoutube(Line 261) returns early every tick — wasted polling with no notifications. Also, the comment says "10 min interval" but the interval is 30 min.♻️ Proposed fix
- // Start YouTube polling (10 min interval) if key is present - const youtubeApiKey = process.env.YOUTUBE_API_KEY - if (youtubeApiKey) { + // Start YouTube polling (30 min interval) if key and channel are present + const youtubeApiKey = process.env.YOUTUBE_API_KEY + const youtubeChannelId = process.env.YOUTUBE_CHANNEL_ID + if (youtubeApiKey && youtubeChannelId) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.ts` around lines 129 - 142, Update start() to enable YouTube polling only when both YOUTUBE_API_KEY and YOUTUBE_CHANNEL_ID are present; otherwise log that polling is disabled and do not create the interval. Update the stale comment to reflect the actual 30-minute polling interval, keeping the guard consistent with checkAndNotifyYoutube.packages/bot/src/services/CriativariaLiveNotificationService.spec.ts (1)
156-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocumentation test asserts on exact comment strings.
Asserting
sourcecontains'14.4k units/day'/'10k/day free quota'couples the test to the wording of an inline comment; a harmless reword of the quota comment will break CI. Consider testing observable behavior (e.g. the 30‑min interval value, already covered) instead of comment text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts` around lines 156 - 164, Remove the source-inspection test “YouTube search endpoint quota math documented in code” and its exact comment-string assertions. Rely on the existing observable behavior test for the 30-minute interval instead of coupling tests to inline comment wording.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts`:
- Around line 336-338: Update the fetch mock in the retry test to return a
`headers` object with a `get` method for 503 responses, matching the `Response`
shape expected by `backoffFetch` and allowing the test to exercise the intended
5xx retry path.
In `@packages/bot/src/services/CriativariaLiveNotificationService.ts`:
- Around line 64-67: Update backoffFetch to validate the parsed Retry-After
value before using it: parse the header as seconds, and only apply the capped
server-provided delay when the result is finite and valid. For missing,
non-numeric, or HTTP-date values, use the existing exponential backoff with
jitter and its 30-second cap.
---
Outside diff comments:
In `@packages/bot/src/services/CriativariaLiveNotificationService.ts`:
- Around line 187-197: When no stream is returned in checkAndNotifyTwitch and
checkAndNotifyYoutube, delete the previously tracked live-announcement message
before clearing its stream/message tracking state. Reuse the existing message
deletion mechanism and safely handle missing messages, ensuring the offline path
removes the announcement immediately rather than waiting for TTL cleanup.
---
Nitpick comments:
In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts`:
- Around line 156-164: Remove the source-inspection test “YouTube search
endpoint quota math documented in code” and its exact comment-string assertions.
Rely on the existing observable behavior test for the 30-minute interval instead
of coupling tests to inline comment wording.
In `@packages/bot/src/services/CriativariaLiveNotificationService.ts`:
- Around line 326-342: Remove the redundant messagesToDelete.push(msgId) inside
the catch block of the message deletion logic; messagesToDelete is already
updated before the try block. Keep the existing error handling and warnLog
behavior unchanged.
- Around line 129-142: Update start() to enable YouTube polling only when both
YOUTUBE_API_KEY and YOUTUBE_CHANNEL_ID are present; otherwise log that polling
is disabled and do not create the interval. Update the stale comment to reflect
the actual 30-minute polling interval, keeping the guard consistent with
checkAndNotifyYoutube.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 32e44814-0ae5-4452-aed8-9424aaad50b2
📒 Files selected for processing (2)
packages/bot/src/services/CriativariaLiveNotificationService.spec.tspackages/bot/src/services/CriativariaLiveNotificationService.ts
- P1: fix retry-after http-date bug in backoffFetch (naN crash on date parsing) - parse as integer seconds first, then http-date format, fallback to exponential - P2: make youtube independent of twitch env vars in start() - restructure: channel required for both, twitch/youtube each optional - P3: clarify 30-min youtube interval in code comment (not 10 min) - test hygiene: switch backoff tests to jest.spyOn(globalThis, 'fetch') - fix 503 mock objects: add headers property to prevent TypeError - new tests: retry-after http-date, garbage retry-after, empty items, youtube without twitch
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/bot/src/services/CriativariaLiveNotificationService.spec.ts (2)
509-520: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd retry coverage for YouTube requests.
These tests only cover an empty YouTube response, while the new retry/backoff behavior is exercised only through Twitch. Add YouTube 429 and 5xx cases, including
Retry-After, to catch API-specific wiring regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts` around lines 509 - 520, The YouTube request tests lack coverage for retryable responses. Extend the tests around fetchYoutubeLiveBroadcast to verify 429 and 5xx responses trigger the configured retry/backoff behavior, including handling of the Retry-After header, and assert the expected fetch attempts and final result.
378-466: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid wall-clock sleeps in retry tests.
The integer and HTTP-date cases block the suite for roughly three seconds and are timing-sensitive in CI. Use Jest fake timers or inject the sleep function so the scheduled delay can be asserted without waiting in real time.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts` around lines 378 - 466, Update the Retry-After tests around fetchStream to avoid real wall-clock delays: use Jest fake timers or inject a controllable sleep implementation, advance timers to cover the integer and HTTP-date delays, and assert the retry occurs without elapsed-time thresholds. Ensure timers and mocks are restored in cleanup for both tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts`:
- Line 539: Complete the unterminated describe invocation labeled “existing
tests” in CriativariaLiveNotificationService.spec.ts by adding the closing
quote, callback, and required closing syntax so the test file parses and
executes successfully.
---
Nitpick comments:
In `@packages/bot/src/services/CriativariaLiveNotificationService.spec.ts`:
- Around line 509-520: The YouTube request tests lack coverage for retryable
responses. Extend the tests around fetchYoutubeLiveBroadcast to verify 429 and
5xx responses trigger the configured retry/backoff behavior, including handling
of the Retry-After header, and assert the expected fetch attempts and final
result.
- Around line 378-466: Update the Retry-After tests around fetchStream to avoid
real wall-clock delays: use Jest fake timers or inject a controllable sleep
implementation, advance timers to cover the integer and HTTP-date delays, and
assert the retry occurs without elapsed-time thresholds. Ensure timers and mocks
are restored in cleanup for both tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 318709b2-6bc2-4e74-982e-f9a4e27b8e13
📒 Files selected for processing (2)
packages/bot/src/services/CriativariaLiveNotificationService.spec.tspackages/bot/src/services/CriativariaLiveNotificationService.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bot/src/services/CriativariaLiveNotificationService.ts
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/bot/src/services/CriativariaLiveNotificationService.ts">
<violation number="1" location="packages/bot/src/services/CriativariaLiveNotificationService.ts:69">
P3: A valid `Retry-After: 0` now falls through to exponential jitter, delaying an API retry by roughly 1–2 seconds instead of honoring the server's immediate-retry instruction. Accept zero delta-seconds (and validate the numeric form rather than `parseInt` prefixes).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
A stray unterminated describe('existing tests line broke the whole
suite compile (0 tests ran). Removed; suite back to 37 passing.
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
toUTCString truncates milliseconds so a +2000ms date could wait only ~1001ms on slow CI. Use +3000ms; assertion unchanged (>=1900ms proves the date path delayed rather than retrying immediately).
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 1 unresolved issue from previous reviews.
Re-trigger cubic
|
🤖 I have created a release *beep* *boop* --- <details><summary>2.35.0</summary> ## [2.35.0](v2.34.0...v2.35.0) (2026-07-13) ### Features * **bot:** /bulk-kick — proof-of-pattern for the bulk-* command family ([#1802](#1802)) ([1c11f3e](1c11f3e)) * **bot:** add lucky_bot_gateway_connected zombie-detection gauge ([#1774](#1774)) ([6cc5287](6cc5287)) * **bot:** playback progress bar in nowplaying/songinfo embed ([#1797](#1797)) ([1138b59](1138b59)) * **bot:** post server-count stats to Top.gg for listing visibility ([#1789](#1789)) ([e8ca6b9](e8ca6b9)) * **bot:** temporary support ticket channels (/ticket) ([#1803](#1803)) ([49601a1](49601a1)) * **live-notif:** youtube polling, message ttl cleanup, api backoff ([#1762](#1762)) ([a99b85a](a99b85a)) * **remind:** channel and role broadcast reminders ([#1767](#1767)) ([#1807](#1807)) ([83ade79](83ade79)) ### Bug Fixes * **backend:** artist suggestions 503 not 500 on upstream timeout ([#1787](#1787)) ([eadc20e](eadc20e)) * **bot:** guard skipReason telemetry against null prisma client ([#1773](#1773)) ([d73421e](d73421e)) * **ci:** stop auto-update workflow racing on merge push ([#1811](#1811)) ([2ddd202](2ddd202)) * **csp:** allow Cloudflare Insights beacon in script-src/connect-src ([#1788](#1788)) ([f20c4cf](f20c4cf)) * **deps:** bump eslint in lock to satisfy npm@12 ci (unblock release) ([#1809](#1809)) ([68fd0be](68fd0be)) * paginate bulk-move message fetch to respect discord api limit ([#1776](#1776)) ([5b5d2fc](5b5d2fc)) * **weekly-digest:** trigger on Sunday and add new-guides RSS section ([#1761](#1761)) ([f429fc6](f429fc6)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).



Closes the real gaps from gap analysis against web-app issue #130 (Twitch notification pipeline already existed): Twitch poll normalized to 5 min; YouTube live detection via a single quota-safe search.list (30-min interval = 4.8k units/day vs 14.4k that the spec'd 10-min would burn against the 10k/day free quota — deviation documented in code); posted-notification tracking with auto-delete on stream end or 4h TTL (in-memory; restart-orphan limitation documented — Prisma model deliberately NOT added); exponential backoff honoring Retry-After on 429/5xx for both APIs. New env (optional): YOUTUBE_API_KEY + YOUTUBE_CHANNEL_ID — absent disables YouTube path with a startup log. 27/27 spec tests. eventsub files untouched (PR #1749 owns them). Cross-repo: Criativaria-Projects/web-app#130.
Summary by cubic
Adds YouTube live polling and 4h TTL cleanup to the live notification service, with exponential backoff and a 5-minute Twitch poll. Uses a quota-safe 30-minute YouTube interval (single
search.list) withcrypto.randomIntjitter; aligns with web-app #130.New Features
search.listevery 30 minutes; disabled withoutYOUTUBE_API_KEYandYOUTUBE_CHANNEL_ID.Bug Fixes
Written for commit 10d4bf7. Summary will update on new commits.
Summary by CodeRabbit
Retry-Afterhandling.