Repository navigation
fix(bot): harden player retry and bridge fallbacks - #550
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR consolidates queue metadata typing by introducing a shared Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/bot/src/handlers/player/playerFactory.ts (1)
280-291: Prefer a typed/structured bridge exhaustion error at the new early-exit path.This new throw is plain
Error; consider attaching a stablecodeand retryability metadata so callers/ops can classify it consistently.♻️ Suggested adjustment
- throw new Error(`Bridge exhausted: no stream for "${track.title}"`) + throw Object.assign( + new Error(`Bridge exhausted: no stream for "${track.title}"`), + { + code: 'ERR_BRIDGE_EXHAUSTED', + retryable: true, + details: { provider: 'soundcloud', reason: 'cooldown_open' }, + }, + )As per coding guidelines, "Define clear, stable error codes ... Provide optional metadata ... Mark
retryablevsnonRetryableerrors where helpful for operations."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/playerFactory.ts` around lines 280 - 291, The early-exit currently throws a plain Error when providerHealthService.isAvailable('soundcloud') is false; replace this with a structured, typed error (e.g., BridgeExhaustedError) that carries a stable code (like "BRIDGE_EXHAUSTED"), retryability metadata (retryable: false), and contextual fields (title, cleanedTitle, url) so callers/ops can classify and act on it. Create or import a BridgeExhaustedError class (or factory) and throw new BridgeExhaustedError({ message: `no stream for "${track.title}"`, code: 'BRIDGE_EXHAUSTED', retryable: false, meta: { title: track.title, cleanedTitle, url: track.url }}) in place of the current throw; keep the existing warnLog call. Ensure the symbol BridgeExhaustedError is exported/typed consistently for downstream handling.packages/bot/src/handlers/player/trackHandlers.ts (1)
141-149: Consider de-duplicating scheduled retry timers per guild.
Rapid consecutive failures can queue multiple 5s retries for the same guild. A small in-memory guard map would keep this bounded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/trackHandlers.ts` around lines 141 - 149, Multiple rapid failures can schedule many 5s retries for the same guild; add an in-memory guard map keyed by queue.guild.id to de-duplicate timers: before calling setTimeout in the replenish retry block check a Map (e.g., scheduledRetryMap) for an existing timer for queue.guild.id, skip scheduling if present, otherwise store the returned timeout handle in the map and ensure you remove the key when the timeout handler runs (and after replenishQueue resolves or rejects) so future retries can be scheduled; reference replenishQueue, queue.guild.id and warnLog when implementing the guard and cleanup logic.packages/bot/src/types/QueueMetadata.ts (1)
2-2: Optional cleanup: avoid leaf type importing from barrel.
QueueMetadata.tsimportingCustomClientfrom./indexintroduces a barrel dependency loop at the type layer. Consider movingCustomClientto its own module and re-exporting both fromindex.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/types/QueueMetadata.ts` at line 2, QueueMetadata.ts currently imports the CustomClient type from the barrel index, creating a type-level dependency loop; create a new module (e.g., customClient.ts) that exports the CustomClient type, update QueueMetadata.ts to import CustomClient from that new module instead of './index', and then re-export CustomClient from index.ts so other code can still import from the barrel without reintroducing the loop; ensure all references to CustomClient (in QueueMetadata and elsewhere) are updated to the new module or the re-export as appropriate.packages/bot/src/handlers/player/errorHandlers.ts (1)
27-27: Optional: Extract a small helper for queue metadata access.The repeated cast pattern works, but a helper would reduce drift and keep metadata reads consistent across handlers.
♻️ Proposed refactor
+const getQueueMetadata = (queue: GuildQueue): QueueMetadata | undefined => + queue.metadata as QueueMetadata | undefined + async function notifyChannelStreamFailed( queue: GuildQueue, trackTitle: string, ): Promise<void> { - const channel = (queue.metadata as QueueMetadata | undefined)?.channel + const channel = getQueueMetadata(queue)?.channel @@ const requestedBy: User | undefined = queue.currentTrack?.requestedBy ?? - (queue.metadata as QueueMetadata | undefined)?.requestedBy ?? + getQueueMetadata(queue)?.requestedBy ?? undefined @@ const requestedByUser: User | undefined = currentTrack.requestedBy ?? - (queue.metadata as QueueMetadata | undefined)?.requestedBy ?? + getQueueMetadata(queue)?.requestedBy ?? undefinedAlso applies to: 186-187, 222-223
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/bot/src/handlers/player/errorHandlers.ts` at line 27, Extract a small utility to centralize safe access to the queue metadata (e.g., a function like getQueueMetadata(queue): QueueMetadata | undefined or getQueueChannel(queue): string | undefined) and replace the repeated casts such as "(queue.metadata as QueueMetadata | undefined)?.channel" in the error handlers with that helper; update all occurrences (including the other spots noted) to call the helper so metadata reading and casting is consistent and easier to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/bot/src/handlers/player/errorHandlers.ts`:
- Line 27: Extract a small utility to centralize safe access to the queue
metadata (e.g., a function like getQueueMetadata(queue): QueueMetadata |
undefined or getQueueChannel(queue): string | undefined) and replace the
repeated casts such as "(queue.metadata as QueueMetadata | undefined)?.channel"
in the error handlers with that helper; update all occurrences (including the
other spots noted) to call the helper so metadata reading and casting is
consistent and easier to maintain.
In `@packages/bot/src/handlers/player/playerFactory.ts`:
- Around line 280-291: The early-exit currently throws a plain Error when
providerHealthService.isAvailable('soundcloud') is false; replace this with a
structured, typed error (e.g., BridgeExhaustedError) that carries a stable code
(like "BRIDGE_EXHAUSTED"), retryability metadata (retryable: false), and
contextual fields (title, cleanedTitle, url) so callers/ops can classify and act
on it. Create or import a BridgeExhaustedError class (or factory) and throw new
BridgeExhaustedError({ message: `no stream for "${track.title}"`, code:
'BRIDGE_EXHAUSTED', retryable: false, meta: { title: track.title, cleanedTitle,
url: track.url }}) in place of the current throw; keep the existing warnLog
call. Ensure the symbol BridgeExhaustedError is exported/typed consistently for
downstream handling.
In `@packages/bot/src/handlers/player/trackHandlers.ts`:
- Around line 141-149: Multiple rapid failures can schedule many 5s retries for
the same guild; add an in-memory guard map keyed by queue.guild.id to
de-duplicate timers: before calling setTimeout in the replenish retry block
check a Map (e.g., scheduledRetryMap) for an existing timer for queue.guild.id,
skip scheduling if present, otherwise store the returned timeout handle in the
map and ensure you remove the key when the timeout handler runs (and after
replenishQueue resolves or rejects) so future retries can be scheduled;
reference replenishQueue, queue.guild.id and warnLog when implementing the guard
and cleanup logic.
In `@packages/bot/src/types/QueueMetadata.ts`:
- Line 2: QueueMetadata.ts currently imports the CustomClient type from the
barrel index, creating a type-level dependency loop; create a new module (e.g.,
customClient.ts) that exports the CustomClient type, update QueueMetadata.ts to
import CustomClient from that new module instead of './index', and then
re-export CustomClient from index.ts so other code can still import from the
barrel without reintroducing the loop; ensure all references to CustomClient (in
QueueMetadata and elsewhere) are updated to the new module or the re-export as
appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b06ffc2-b963-43e1-bca5-3f56471525b7
📒 Files selected for processing (13)
packages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.tspackages/bot/src/types/index.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/tests/handlers/player/playerFactory.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: SonarCloud Scan
- GitHub Check: Quality Gates
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{js,jsx,ts,tsx,vue,html}
📄 CodeRabbit inference engine (.cursor/rules/accessibility-openness.mdc)
Provide accessible UI components using semantic HTML and ARIA attributes where necessary
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dependency-injection.mdc)
**/*.{ts,tsx,js,jsx}: Prefer constructor injection for classes that require dependencies
Avoid global mutable singletons unless necessary
Use explicit interfaces for external dependencies to make testing easier
**/*.{ts,tsx,js,jsx}: Include required references in PRs/code for non-trivial logic: TypeScript (official docs), MDN (JavaScript reference), and official docs for any runtime/framework/libraries used (e.g., Node.js, React) as applicable.
Before assuming behavior of an API, include the doc link and a ≤25-word quote when the change relies on it.
**/*.{ts,tsx,js,jsx}: Prefer named exports for clear usage and easier refactors in TypeScript/JavaScript
Keep import order consistent: external first, then internal modules
Remove dead code and unused imports
**/*.{ts,tsx,js,jsx}: Use PascalCase naming convention for React/UI components
Use camelCase naming convention for variables and functions
Use UPPER_SNAKE_CASE naming convention for constants
Maintain consistent import grouping and ordering within the project, keeping third-party imports separate from local imports
For external data sources (HTTP, database), always validate and sanitize input using type guards or schema validatorsImplement TypeScript typecheck and linter in CI quality checks
**/*.{ts,tsx,js,jsx}: Use TypeScript for enhanced type safety
Implement error handling and error logging
Avoid commenting code unless extremely necessary - code should explain itself with descriptive names
Leave NO todos, placeholders or missing pieces in the code
Variables and functions must use camelCase
Constants must use UPPER_SNAKE_CASE
Use arrow functions for methods and computed properties
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Maintain consistent import grouping/order: external imports first, then internal modules
Use named exports for clear usage and easier refactors
Always validate and sanitize external data (HTTP, DB) at the boundary using type guards ...
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{js,jsx,ts,tsx}: Never throw strings. ThrowError(or typed subclasses) with descriptive messages
Include causal error ascausewhen available for better debugging
Define clear, stable error codes (e.g.,ERR_AUTH_EXPIRED,ERR_NETWORK_TIMEOUT)
Provide optional metadata (e.g.,details,retryable,status,correlationId) in error objects
Use domain error classes per area (e.g.,AuthenticationError,ValidationError,NetworkError)
Log errors with structure (message, code, stack, cause, correlationId, user context where appropriate)
MarkretryablevsnonRetryableerrors where helpful for operations
Set timeouts and handle aborts/cancellations; avoid dangling requests in API/network code
Implement backoff for transient failures; avoid infinite retries
Map HTTP status → domain errors; 4xx vs 5xx behave differently (e.g., retry for 5xx/network)
**/*.{js,jsx,ts,tsx}: Use functional components with hooks in React/React Native. Avoid class components.
Keep components focused on a single responsibility; extract complex logic into custom hooks.
Keep state local when possible. Use Context/Zustand/Redux only when necessary for state management.
If props or state traverse more than 3 levels, consider using context or a feature-scoped store instead of prop drilling.
Use performance optimization techniques:React.memo,useMemo,useCallback,Suspense(web), and virtualization for long lists; avoid unnecessary re-renders.
Web accessibility: use semantic HTML, labels, focus management, keyboard navigation, andaria-*attributes as needed.
React Native accessibility: use accessibility props (accessible,accessibilityLabel), proper roles and labels.
Identify and extract repetitive UI components proactively tocomponents/with clear props and minimal coupling.
Web styles: prefer co-located styles or design system tokens; avoid global style leakage.
React Native styles: preferStyleSheet.create, design tokens, and theme providers; avoid in...
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Use
index.tsonly to re-export a small, intentional surface per module
Files:
packages/bot/src/types/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
Introduce interfaces at module boundaries to enable testing and substitutions
**/*.{ts,tsx}: Avoid usinganytype in TypeScript. If unavoidable, useunknownwith type guards and justify with a code comment
Preferinterfacefor defining public object shapes in TypeScript, usetypefor unions and utility types
Use TypeScript utility types such asPartial,Pick,Omit,Readonly, andRecordwhen appropriate
UseI{Name}naming convention for interfaces in TypeScript
UseT{Name}naming convention for type aliases and utility types in TypeScript
**/*.{ts,tsx}: Prefer types over interfaces for most cases
Don't ever useany- type safety always
Avoid enums; use const objects instead
For complex types, create a separate file to declare them and import them
Avoid usinganytype; if unavoidable, useunknownwith type guards and justify with code comment
Preferinterfacefor public API shapes; usetypefor unions and utility types
Use TypeScript utility types (Partial, Pick, Omit, Readonly, Record)
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/documentation.mdc)
**/*.{js,ts,tsx,jsx}: Minimize comments in code; explain the 'why' when non-obvious, let code express the 'what' through clear naming
Document trade-offs briefly when deviating from ideal patterns
**/*.{js,ts,tsx,jsx}: Store secrets, ports, and hosts in environment variables (.env,.env.example) and never hardcode them
Avoid redundant or decorative AI comments; code should be self-explanatory and only commented when logic is non-obvious; prefer refactoring over lengthy comments
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
packages/bot/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
packages/bot/**/*.{ts,tsx}: UseuseMainPlayer()fromdiscord-playerto access the player instance; do not instantiate player directly
Do not duplicate queue or player state outside Discord Player; use shared services from@lucky/sharedfor persistent data like track history and session information
UseerrorLoganddebugLogfrom@lucky/shared/utilsfor logging throughout the bot package
Use embed and reply utilities from@lucky/sharedfor consistent message formatting and error sanitization across the bot
Use services from@lucky/shared(DatabaseService, Redis client) for database and cache access; do not instantiate Prisma or Redis directly in the bot package
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
packages/bot/**
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
The
botpackage depends onsharedand contains Discord bot commands and player handlers using Discord.js and Discord Player
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/*.{js,mjs,ts,mts}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Use Node.js version ≥22 with ESM (ECMAScript modules) only; no CommonJS
Files:
packages/bot/src/types/index.tspackages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
packages/bot/src/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/subagent-discord.mdc)
Use
@lucky/sharedfor database, Redis, logging, and embed utilities instead of implementing them locally
Files:
packages/bot/src/types/index.tspackages/bot/src/utils/music/queueManipulation.tspackages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/utils/music/idleDisconnect.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.tspackages/bot/src/types/QueueMetadata.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Test behavior, not implementation. Prefer Testing Library utilities for testing React/React Native components.
For React Native tests: mock native modules and test component interactions and accessibility labels.
Files:
packages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pattern.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Test behavior, not implementation details
Prefer unit tests for core logic; add integration tests at meaningful boundaries
Files:
packages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/testing-quality.mdc)
**/*.{test,spec}.{js,ts,jsx,tsx}: Use Jest + a React testing library for unit and component tests as applicable
Test behavior, not implementation details
Files:
packages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.ts
{packages/*/tests/**/*.test.{js,ts},tests/**/*.test.{js,ts}}
📄 CodeRabbit inference engine (.cursor/rules/lucky-project.mdc)
Add or adjust unit and integration tests when changing behavior; follow existing patterns in
packages/*/testsand roottests/directories
Files:
packages/bot/tests/handlers/player/playerFactory.test.ts
**/*.{spec,test}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
**/*.{spec,test}.{ts,tsx,js,jsx}: Use Jest for unit and integration tests
Test behavior, not implementation details
Run unit, integration tests, and coverage report in CI quality checks
Files:
packages/bot/tests/handlers/player/playerFactory.test.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.ts
packages/bot/src/handlers/player/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/lucky-discord-bot.mdc)
Track handling, errors, and lifecycle must be managed through dedicated handlers in
packages/bot/src/handlers/player/(trackHandlers, errorHandlers, lifecycleHandlers)
Files:
packages/bot/src/handlers/player/trackHandlers.tspackages/bot/src/handlers/player/lifecycleHandlers.tspackages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.tspackages/bot/src/handlers/player/errorHandlers.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.tspackages/bot/src/handlers/player/trackNowPlaying.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/quality.mdc)
Unit tests must use naming convention
*.spec.ts
Files:
packages/bot/src/handlers/player/errorHandlers.spec.tspackages/bot/src/handlers/player/trackHandlers.spec.tspackages/bot/src/handlers/player/playerFactory.bridge.spec.ts
**/[A-Z]*.{ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Components must use PascalCase naming
Files:
packages/bot/src/types/QueueMetadata.ts
🔇 Additional comments (13)
packages/bot/tests/handlers/player/playerFactory.test.ts (1)
37-41: Good provider-health mock baseline for this suite.Defaulting
isAvailabletotruekeeps existingplayerFactorytests stable while accommodating the new cooldown gate path.packages/bot/src/handlers/player/playerFactory.bridge.spec.ts (2)
9-9: Nice test-control injection for provider availability.Using
providerIsAvailableMockmakes availability branching explicit and easy to steer per test.Also applies to: 43-47
373-375: Strong coverage for the new SoundCloud-cooldown branch.This validates both outcomes: expected rejection and zero SoundCloud fallback calls when the provider is unavailable.
Also applies to: 410-421
packages/bot/src/types/index.ts (1)
25-25: Good re-export to centralize the queue metadata type surface.
This keeps consumers aligned on a single metadata contract.packages/bot/src/utils/music/queueManipulation.ts (1)
307-309: Typed metadata fallback is cleaner and safer here.
The updatedQueueMetadata | undefinedhandling preserves behavior while removing local-shape drift.packages/bot/src/handlers/player/lifecycleHandlers.ts (1)
36-41: Nice metadata typing consolidation in session restore path.
No functional regression and better consistency with the shared queue metadata model.packages/bot/src/handlers/player/trackHandlers.ts (1)
142-148: Great improvement: retry failures are no longer silent.
IncludingguildIdin the warning payload will materially help incident triage.packages/bot/src/handlers/player/errorHandlers.spec.ts (1)
347-361: Solid regression test for logger-failure hardening.
This covers the exact guarded-path behavior the PR introduces.packages/bot/src/handlers/player/trackHandlers.spec.ts (1)
305-327: Good behavioral test for failed retry logging path.
Verifying both call count (2) and warning payload makes this change robust against regressions.packages/bot/src/utils/music/idleDisconnect.ts (1)
48-55: Nice consistency update on idle disconnect metadata access.
The sharedQueueMetadatacast plus guardedchannel.sendpath is clean and safe.packages/bot/src/handlers/player/errorHandlers.ts (1)
73-75: Good hardening of logger failure path.Catching logger failures and emitting a fallback debug log removes silent failure behavior and improves operability.
packages/bot/src/handlers/player/trackNowPlaying.ts (2)
43-44: Requester fallback update looks correct.Using shared
QueueMetadatafor queue-level requester fallback keeps this path aligned with the player handlers.
68-69: Nice defensive guard before channel usage.Early return on missing
metadata.channelprevents avoidable runtime failures in now-playing message send/edit flow.
* fix(bot): log player retry failures * fix(bot): share queue metadata type * fix(bot): skip unhealthy soundcloud bridge
This branch was successfully deployed
1 participant
Summary
IQueueMetadatadrift across player/music paths.Verification
npm run lintexited 0.npm run buildexited 0.npm run test:allexited 0 after rerun with local listener permissions: shared 257, backend 678, bot 1877, frontend 476 tests passed.npm run audit:highexited 0 with 0 vulnerabilities.Note: the first sandboxed
npm run test:allrun failed in backend Supertest withlisten EPERM: operation not permitted 0.0.0.0; rerunning with approved permissions passed.Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests