Repository navigation
fix(bot): expose degraded-extractor signal and honest play error - #1999
Conversation
registration reporting truthy never meant the extractor actually works - the bot stayed healthy while youtube playback silently died. add a lucky_music_extractor_degraded gauge (scraped for alerting, per the issue's own priority order) driven by loadYoutubeExtractor's existing success/failure signal, and use it to give /play users an honest 'sources unreachable' message instead of a misleading 'no results found' when the extractor is known-degraded.
📝 WalkthroughWalkthroughThe PR adds centralized extractor health tracking, exposes degradation through Prometheus, updates YouTube registration state, and formats ChangesExtractor degradation and play error handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PlayerFactory
participant ExtractorHealth
participant PrometheusGauge
participant PlayHandler
participant ErrorResolver
PlayerFactory->>ExtractorHealth: update YouTube degradation
ExtractorHealth->>PrometheusGauge: set extractor gauge
PlayHandler->>ErrorResolver: format playback error
ErrorResolver->>ExtractorHealth: query YouTube degradation
ErrorResolver-->>PlayHandler: return temporary or sanitized message
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Failed to generate code suggestions for PR |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds extractor health tracking for the music player. It introduces a new extractorHealth module that maintains an in-process set of degraded extractors alongside a new Prometheus gauge (musicExtractorDegradedGauge), wires the player factory's YouTube extractor loading to mark the extractor degraded/healthy at various registration outcomes, and adds a resolvePlayErrorMessage helper that swaps the "no results found" error for a "sources unreachable" message when YouTube is flagged degraded. The play handler is updated to use this new error resolver instead of calling createUserFriendlyError directly. New spec files cover extractorHealth and resolvePlayErrorMessage. The surface area spans the bot's player handlers, the play command error path, and the Prometheus monitoring module.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds extractor health tracking for the music player. It introduces a new extractorHealth module that maintains an in-process set of degraded extractors and mirrors that state into a new Prometheus gauge (lucky_music_extractor_degraded). The YouTube extractor loading path in playerFactory is updated to mark the extractor degraded/healthy at its relevant success and failure points. It also adds a resolvePlayErrorMessage helper that substitutes a "sources unreachable" message for "no results found" errors when YouTube is flagged degraded, and wires it into the play command's error embed in place of the previous createUserFriendlyError call. New unit tests cover the health module and the error message resolution.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a new extractor health tracking mechanism for the music player. It introduces extractorHealth.ts, which maintains an in-process set of degraded extractors and mirrors that state to a new Prometheus gauge (lucky_music_extractor_degraded). The YouTube extractor loading logic in playerFactory.ts now calls setExtractorDegraded at success/failure points. It also adds playErrorMessage.ts, which wraps the existing error sanitizer and substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged degraded; playHandler.ts is updated to use this resolver instead of calling the sanitizer directly. New spec files cover resolvePlayErrorMessage and the extractorHealth state/gauge behavior. Surface area spans the play command handlers, the player factory, and the Prometheus monitoring module.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds an extractorHealth module that tracks per-extractor degraded state in-process and mirrors it to a new lucky_music_extractor_degraded Prometheus gauge. The YouTube extractor loading in playerFactory now calls setExtractorDegraded at its various success/failure points, and the play command's error handling routes through a new resolvePlayErrorMessage helper that substitutes a "sources unreachable" message for "no results" errors when YouTube is flagged degraded. Surface area also includes new unit tests for extractorHealth and resolvePlayErrorMessage, plus the new gauge registration in the monitoring module.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a shared extractor health tracking mechanism for the music player. It introduces a new extractorHealth module that maintains an in-process set of degraded extractors and mirrors that state into a new Prometheus gauge (lucky_music_extractor_degraded), which playerFactory now updates when the YouTube extractor registration succeeds or fails. It also adds a resolvePlayErrorMessage helper that substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged degraded, and wires this into the play command's error handling in place of the direct createUserFriendlyError call. Spec files are included for both the new health module and the error-message resolver.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/functions/music/commands/play/handlers/playErrorMessage.ts`:
- Around line 4-6: Update the comment near the “No results found” message to
remove the inaccurate YouTube session-initialization example, or explicitly
state that functional failures after successful registration are not detected by
isExtractorDegraded('youtube'). Keep the comment aligned with the tracked
conditions: missing extractor exports and failed registration retries.
🪄 Autofix
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 Plus
Run ID: 2b88ef02-2feb-4557-bbd3-dcb25d7922d5
📒 Files selected for processing (7)
packages/bot/src/functions/music/commands/play/handlers/playErrorMessage.spec.tspackages/bot/src/functions/music/commands/play/handlers/playErrorMessage.tspackages/bot/src/functions/music/commands/play/handlers/playHandler.tspackages/bot/src/handlers/player/extractorHealth.spec.tspackages/bot/src/handlers/player/extractorHealth.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/utils/monitoring/prometheus.ts
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a new music extractor health-tracking mechanism to the bot. It introduces an extractorHealth module that maintains an in-process set of degraded extractors and mirrors that state to a new Prometheus gauge (musicExtractorDegradedGauge), with the player factory now calling setExtractorDegraded('youtube', ...) at YouTube extractor registration success/failure points. It also adds a resolvePlayErrorMessage helper used by the play handler, which substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged degraded, otherwise falling back to the existing sanitized error message. The surface area includes two new spec files (for extractorHealth and playErrorMessage), the new extractorHealth.ts and playErrorMessage.ts source files, a new gauge in the prometheus module, and a wiring change in playHandler.ts swapping createUserFriendlyError for resolvePlayErrorMessage.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a "degraded extractor" health-tracking mechanism for the music player. It introduces a new extractorHealth module that maintains an in-process set of degraded extractors and mirrors that state to a new Prometheus gauge (lucky_music_extractor_degraded), and wires playerFactory to mark the YouTube extractor as degraded/healthy at the relevant registration outcomes. On the user-facing side, it adds a resolvePlayErrorMessage helper (and swaps it into the play command's error path) that substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged degraded, otherwise falling back to the existing sanitized error message. The PR also adds unit tests covering extractorHealth and resolvePlayErrorMessage. Surface area spans packages/bot's player handlers, play command handlers, and Prometheus monitoring setup.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds a new resolvePlayErrorMessage helper and wires it into the /play error path so that "no results found" errors are replaced with a "sources unreachable" message when the YouTube extractor is flagged as degraded. It introduces a new extractorHealth module that tracks degraded extractor state in-process and mirrors it to a new musicExtractorDegradedGauge Prometheus metric, and updates playerFactory to set that degraded state during YouTube extractor loading (on missing export, successful registration, and post-retry failure). Accompanying spec files are added for both the new error-message resolution and the extractor-health module. Surface area: bot play command error handling, player extractor loading, and Prometheus monitoring.
No blocking issues surfaced. 3 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR adds an extractor health tracking mechanism for the music player. It introduces a new extractorHealth module that maintains an in-process set of degraded extractors and updates a corresponding Prometheus gauge (lucky_music_extractor_degraded), and wires the YouTube extractor loading in playerFactory to mark the extractor degraded/healthy at the relevant registration outcomes. It also adds a resolvePlayErrorMessage helper that substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged as degraded, and updates the play handler to use it instead of the generic error sanitizer. New spec files cover the extractor health state/gauge behavior and the error message resolution logic.
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR introduces a new extractor health tracking mechanism for the music player. It adds an extractorHealth module that maintains an in-memory set of degraded extractors and mirrors that state to a new Prometheus gauge (musicExtractorDegradedGauge), with the YouTube extractor loading path in playerFactory now calling setExtractorDegraded on success/failure. On the command side, it adds a resolvePlayErrorMessage helper that substitutes a "sources unreachable" message for "no results found" errors when the YouTube extractor is flagged as degraded, and wires it into the play handler's error embed in place of the previous createUserFriendlyError call. The surface area also includes new unit tests for both the health module and the error message resolver.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 38 functions depend on the 33 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
executePlayHandler()— 1 callers, 15 callees
Verification — 38 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 37 function(s) in the blast radius were not formally verified this run
· 1 grounded finding(s) anchored inline below.
|
🤖 I have created a release *beep* *boop* --- <details><summary>2.39.3</summary> ## [2.39.3](v2.39.2...v2.39.3) (2026-08-12) ### Bug Fixes * **bot:** expose degraded-extractor signal and honest play error ([#1999](#1999)) ([8348bd6](8348bd6)) * **bot:** unlink dead Last.fm sessions on error 9 and notify once ([#1946](#1946)) ([d22b8bd](d22b8bd)) * **ci:** add a second retry for docker-build ([#2007](#2007)) ([9347c55](9347c55)) * **ci:** extract node_modules before per-workspace build steps run ([#2006](#2006)) ([ae577d1](ae577d1)) * **ci:** free disk space before Docker builds to prevent BuildKit GC race ([#2003](#2003)) ([5bcd388](5bcd388)) * **ci:** isolate docker cache scope by branch ([#2012](#2012)) ([5d25ec8](5d25ec8)) * **ci:** pin node:24-alpine base image by digest ([#2004](#2004)) ([f154bfe](f154bfe)) * **ci:** skip gha cache-from on docker-build retries ([#2008](#2008)) ([89dbb90](89dbb90)) * **ci:** split deps-production per production target ([#2005](#2005)) ([43aacce](43aacce)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
1 participant
What
Per #1929's incident: on 2026-07-30 the YouTube extractor registered successfully (truthy return value) while
youtubei.jshad already fallen back to non-functional locally-generated session data (DNS outage). Every existing signal (docker healthcheck,/healthz) stayed green because they only check the Discord gateway./playfailed with a generic "No results found" and nothing else indicated anything was wrong.The issue's own "Options worth weighing" section explicitly ranks the fix: (3) a Prometheus metric first, (2) better user-facing wording second, and explicitly says to avoid (1) a boot-time smoke-probe (adds latency) and never add a YouTube reachability check to the container healthcheck (would restart-loop during transient outages).
Fix
lucky_music_extractor_degraded{extractor}gauge, driven by the registration success/failure signalloadYoutubeExtractoralready has (no new network calls, no boot latency).extractorHealth.ts: single source of truth updating both the gauge and an in-process readable flag together./play's error path: when the underlying error is discord-player'sNoResultError("No results found...") and the YouTube extractor is known-degraded, show "Music sources are currently unreachable. Please try again in a few minutes." instead of the misleading generic message.Known gap (by design, per the issue)
This catches registration failing, not the exact "registers successfully but is functionally broken" case from the original incident — that would need the smoke-probe the issue explicitly says to avoid for latency reasons. The gauge + honest message still materially improve the situation: any future registration failure is now visible and alertable, and users get a truthful message instead of "no results" whenever we do know something's wrong.
Fixes
Addresses #1929 (metric + user-facing message; the underlying network-outage/DNS root cause is out of scope per the issue itself)
Testing
extractorHealth.spec.ts,playErrorMessage.spec.ts(both pure-function, no heavy mocking needed)resolvePlayErrorMessageextracted fromplayHandler.tsinto its own file specifically so it's testable without mocking that file's much larger dependency graph (it has zero pre-existing test coverage)test:botsuite green (246/247, 1 pre-existing skip unchanged)type:checkcleanSummary by cubic
Exposes a degraded-extractor signal and shows a clear
/playerror when sources are unreachable. Adds a Prometheus gauge and centralized health state to address #1929 without smoke probes or healthcheck changes.lucky_music_extractor_degraded{extractor}updates on YouTube extractor registration success/failure (no new network calls or boot latency).extractorHealth.ts(setExtractorDegraded,isExtractorDegraded) updates both the gauge and an in-process flag./playreplaces "No results found" with "Music sources are currently unreachable. Please try again in a few minutes." when a no-results error occurs and YouTube is degraded.Written for commit f4659c5. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Monitoring
Tests