Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/bot/src/handlers/player/soundcloudMatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export async function streamViaSoundCloud(
scStream = await playdl.stream(match.url)
} catch (err) {
throw new Error(
`SoundCloud: stream creation failed for "${match.name}" — ${(err as Error).message}`,
`SoundCloud: stream creation failed for "${match.name}" — ${err instanceof Error ? err.message : String(err)}`,
{ cause: err },
)
}
Expand Down
4 changes: 4 additions & 0 deletions packages/bot/src/handlers/player/streamBridge.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ describe('streamViaYtDlp – URL validation', () => {
describe('streamViaYtDlp – process lifecycle', () => {
const validUrl = 'https://www.youtube.com/watch?v=abc123'

afterEach(() => {
jest.useRealTimers()
})

it('resolves with a PassThrough stream when stdout emits first chunk', async () => {
const proc = makeFakeProc()
mockSpawn.mockReturnValue(proc)
Expand Down
35 changes: 35 additions & 0 deletions packages/bot/src/lastfm/lastFmApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
parseArtists,
getTrackMetadata,
__resetMetadataCacheForTests,
LastFmSessionExpiredError,
} from './lastFmApi'

const getSessionKeyMock =
Expand Down Expand Up @@ -233,6 +234,29 @@ describe('lastFmApi', () => {
})
})

describe('LastFmSessionExpiredError', () => {
it('creates error with default message', () => {
const error = new LastFmSessionExpiredError()
expect(error).toBeInstanceOf(Error)
expect(error.message).toBe(
'Last.fm session key has expired (error code 9)',
)
expect(error.name).toBe('LastFmSessionExpiredError')
})

it('creates error with custom message', () => {
const customMsg = 'Custom expiry message'
const error = new LastFmSessionExpiredError(customMsg)
expect(error.message).toBe(customMsg)
expect(error.name).toBe('LastFmSessionExpiredError')
})

it('is instanceof Error', () => {
const error = new LastFmSessionExpiredError()
expect(error instanceof Error).toBe(true)
})
})

describe('getTopTracks', () => {
it('returns mapped tracks on success', async () => {
fetchMock.mockResolvedValueOnce({
Expand Down Expand Up @@ -517,6 +541,17 @@ describe('lastFmApi', () => {

expect(isLastFmInvalidSessionError(error)).toBe(true)
})

it('detects LastFmSessionExpiredError instances regardless of message', () => {
expect(
isLastFmInvalidSessionError(new LastFmSessionExpiredError()),
).toBe(true)
expect(
isLastFmInvalidSessionError(
new LastFmSessionExpiredError('totally different message'),
),
).toBe(true)
})
})

describe('getTagTopTracks', () => {
Expand Down
15 changes: 15 additions & 0 deletions packages/bot/src/lastfm/lastFmApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ import { debugLog } from '@lucky/shared/utils/general/log'

const API_BASE = 'https://ws.audioscrobbler.com/2.0/'

/**
* Error thrown when Last.fm session key has expired (error code 9).
* Caller should re-authenticate the user.
*/
export class LastFmSessionExpiredError extends Error {
constructor(message = 'Last.fm session key has expired (error code 9)') {
super(message)
this.name = 'LastFmSessionExpiredError'
}
}

function getApiConfig(): { apiKey: string; secret: string } | null {
const apiKey = process.env.LASTFM_API_KEY
const secret = process.env.LASTFM_API_SECRET
Expand Down Expand Up @@ -83,6 +94,9 @@ async function signedPost(
message?: string
}
if (data.error) {
if (data.error === 9) {
throw new LastFmSessionExpiredError()
}
Comment thread
LucasSantana-Dev marked this conversation as resolved.
throw new Error(
`Last.fm ${method}: ${data.error} - ${data.message ?? ''}`,
)
Expand Down Expand Up @@ -331,6 +345,7 @@ export async function scrobble(
}

export function isLastFmInvalidSessionError(error: unknown): boolean {
if (error instanceof LastFmSessionExpiredError) return true
if (!(error instanceof Error)) return false

const message = error.message.toLowerCase()
Expand Down
194 changes: 153 additions & 41 deletions packages/bot/src/utils/music/autoplay/artistTagCache.spec.ts
Original file line number Diff line number Diff line change
@@ -1,76 +1,188 @@
import { jest } from '@jest/globals'
import { hasGenreTag, createArtistTagFetcher } from './artistTagCache'
import { describe, expect, it, jest, beforeEach } from '@jest/globals'
import {
createArtistTagFetcher,
hasGenreTag,
type ArtistTagFetcher,
} from './artistTagCache'

jest.mock('../../../lastfm', () => ({
getArtistTopTags: jest.fn(),
}))

describe('hasGenreTag', () => {
it('returns false when tags array is empty', () => {
expect(hasGenreTag([], ['rock', 'pop'])).toBe(false)
})
const { getArtistTopTags } = require('../../../lastfm')

it('returns true when a tag matches a genre variant', () => {
expect(hasGenreTag(['sertanejo', 'country'], ['sertanejo'])).toBe(true)
describe('createArtistTagFetcher', () => {
beforeEach(() => {
jest.clearAllMocks()
})

it('returns false when no tags match', () => {
expect(hasGenreTag(['rock', 'indie'], ['sertanejo', 'forró'])).toBe(false)
it('returns empty array for undefined artist', async () => {
const fetcher = createArtistTagFetcher()
const result = await fetcher(undefined)
expect(result).toEqual([])
})

it('is case-insensitive', () => {
expect(hasGenreTag(['Sertanejo'], ['sertanejo'])).toBe(true)
expect(hasGenreTag(['sertanejo'], ['SERTANEJO'])).toBe(true)
it('returns empty array for empty string artist', async () => {
const fetcher = createArtistTagFetcher()
const result = await fetcher('')
expect(result).toEqual([])
})

it('trims whitespace from tags and variants', () => {
expect(hasGenreTag([' sertanejo '], ['sertanejo'])).toBe(true)
it('returns empty array for whitespace-only artist', async () => {
const fetcher = createArtistTagFetcher()
const result = await fetcher(' ')
expect(result).toEqual([])
})

it('returns false when variants array is empty', () => {
expect(hasGenreTag(['rock'], [])).toBe(false)
it('fetches tags from Last.fm for valid artist', async () => {
const tags = ['rock', 'alternative', 'indie']
getArtistTopTags.mockResolvedValue(tags)

const fetcher = createArtistTagFetcher()
const result = await fetcher('The Beatles')

expect(getArtistTopTags).toHaveBeenCalledWith('The Beatles')
expect(result).toEqual(tags)
})
})

describe('createArtistTagFetcher', () => {
it('returns an empty array for undefined artist', async () => {
it('caches results for same artist (case-insensitive)', async () => {
const tags = ['pop', 'electronic']
getArtistTopTags.mockResolvedValue(tags)

const fetcher = createArtistTagFetcher()
const result = await fetcher(undefined)
expect(result).toEqual([])

const result1 = await fetcher('Daft Punk')
const result2 = await fetcher('daft punk')
const result3 = await fetcher('DAFT PUNK')

expect(getArtistTopTags).toHaveBeenCalledTimes(1)
expect(result1).toEqual(tags)
expect(result2).toEqual(tags)
expect(result3).toEqual(tags)
})

it('returns an empty array for empty string artist', async () => {
it('coalesces concurrent requests for same artist', async () => {
const tags = ['country', 'folk']
getArtistTopTags.mockImplementation(
() =>
new Promise((resolve) => {
setTimeout(() => resolve(tags), 50)
}),
)

const fetcher = createArtistTagFetcher()
const result = await fetcher('')
expect(result).toEqual([])

const [result1, result2, result3] = await Promise.all([
fetcher('Willie Nelson'),
fetcher('Willie Nelson'),
fetcher('Willie Nelson'),
])

expect(getArtistTopTags).toHaveBeenCalledTimes(1)
expect(result1).toEqual(tags)
expect(result2).toEqual(tags)
expect(result3).toEqual(tags)
})

it('returns an empty array for whitespace-only artist', async () => {
it('returns empty array when Last.fm fails', async () => {
getArtistTopTags.mockRejectedValue(new Error('API error'))

const fetcher = createArtistTagFetcher()
const result = await fetcher(' ')
const result = await fetcher('Unknown Artist')

expect(result).toEqual([])
})

it('fetches and caches tags for a known artist', async () => {
const { getArtistTopTags } = require('../../../lastfm')
;(getArtistTopTags as jest.Mock).mockResolvedValue(['rock', 'indie'])
it('uses spotify fallback when Last.fm returns empty and fallback provided', async () => {
getArtistTopTags.mockResolvedValue([])
const spotifyFallback = jest
.fn<(artist: string) => Promise<string[]>>()
.mockResolvedValue(['pop', 'latin'])

const fetcher = createArtistTagFetcher()
const result1 = await fetcher('Radiohead')
const result2 = await fetcher('radiohead')
const fetcher = createArtistTagFetcher(spotifyFallback)
const result = await fetcher('Bad Bunny')

expect(result1).toEqual(['rock', 'indie'])
expect(result2).toEqual(['rock', 'indie'])
expect((getArtistTopTags as jest.Mock).mock.calls.length).toBe(1)
expect(getArtistTopTags).toHaveBeenCalledWith('Bad Bunny')
expect(spotifyFallback).toHaveBeenCalledWith('Bad Bunny')
expect(result).toEqual(['pop', 'latin'])
})

it('returns empty array when Last.fm fetch fails', async () => {
const { getArtistTopTags } = require('../../../lastfm')
;(getArtistTopTags as jest.Mock).mockRejectedValue(new Error('API down'))
it('does not use spotify fallback when Last.fm has tags', async () => {
getArtistTopTags.mockResolvedValue(['rock', 'metal'])
const spotifyFallback = jest
.fn<(artist: string) => Promise<string[]>>()
.mockResolvedValue(['pop'])

const fetcher = createArtistTagFetcher()
const result = await fetcher('Some Artist')
const fetcher = createArtistTagFetcher(spotifyFallback)
const result = await fetcher('Metallica')

expect(spotifyFallback).not.toHaveBeenCalled()
expect(result).toEqual(['rock', 'metal'])
})

it('handles spotify fallback errors gracefully', async () => {
getArtistTopTags.mockResolvedValue([])
const spotifyFallback = jest
.fn<(artist: string) => Promise<string[]>>()
.mockRejectedValue(new Error('Spotify API error'))

const fetcher = createArtistTagFetcher(spotifyFallback)
const result = await fetcher('Unknown')

expect(result).toEqual([])
})

it('converts spotify fallback non-array result to empty array', async () => {
getArtistTopTags.mockResolvedValue([])
const spotifyFallback = jest
.fn<(artist: string) => Promise<string[]>>()
.mockResolvedValue(null as unknown as string[])

const fetcher = createArtistTagFetcher(spotifyFallback)
const result = await fetcher('Artist')

expect(result).toEqual([])
})

it('caches fallback results same as Last.fm results', async () => {
getArtistTopTags.mockResolvedValue([])
const spotifyFallback = jest
.fn<(artist: string) => Promise<string[]>>()
.mockResolvedValue(['genre1', 'genre2'])

const fetcher = createArtistTagFetcher(spotifyFallback)

const result1 = await fetcher('Artist')
const result2 = await fetcher('Artist')

expect(spotifyFallback).toHaveBeenCalledTimes(1)
expect(result1).toEqual(result2)
})
})

describe('hasGenreTag', () => {
it('returns false when tags array is empty', () => {
expect(hasGenreTag([], ['rock', 'pop'])).toBe(false)
})

it('returns true when a tag matches a genre variant', () => {
expect(hasGenreTag(['sertanejo', 'country'], ['sertanejo'])).toBe(true)
})

it('returns false when no tags match', () => {
expect(hasGenreTag(['rock', 'indie'], ['sertanejo', 'forró'])).toBe(false)
})

it('is case-insensitive', () => {
expect(hasGenreTag(['Sertanejo'], ['sertanejo'])).toBe(true)
expect(hasGenreTag(['sertanejo'], ['SERTANEJO'])).toBe(true)
})

it('trims whitespace from tags and variants', () => {
expect(hasGenreTag([' sertanejo '], ['sertanejo'])).toBe(true)
})

it('returns false when variants array is empty', () => {
expect(hasGenreTag(['rock'], [])).toBe(false)
})
})
28 changes: 22 additions & 6 deletions packages/bot/src/utils/music/autoplay/artistTagCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,21 @@ export function hasGenreTag(tags: string[], genreVariants: string[]): boolean {
}

/**
* Per-pass de-duplicating Last.fm artist-tag fetcher. The replenisher creates
* one of these per autoplay pass and threads it through every candidate
* collector so we only ask Last.fm for each artist once even when the same
* artist surfaces in Spotify, Last.fm seeds, and genre searches.
* Per-pass de-duplicating artist-tag fetcher. The replenisher creates one per
* autoplay pass and threads it through every candidate collector, so each
* artist is only looked up once even when it surfaces in multiple sources.
*
* When a `spotifyFallback` is provided and Last.fm returns no tags (i.e. the
* user has not linked Last.fm), the fetcher calls the fallback and caches its
* result. This lets the cross-locale veto fire even for artists whose
* title/author carry no Spanish text markers (e.g. "Marcos Witt", "Alex Zurdo").
*
* Each call returns a Promise so concurrent in-flight requests for the same
* artist coalesce automatically.
*/
export function createArtistTagFetcher(): ArtistTagFetcher {
export function createArtistTagFetcher(
spotifyFallback?: (artist: string) => Promise<string[]>,
): ArtistTagFetcher {
const cache = new Map<string, Promise<string[]>>()

return async (artist) => {
Expand All @@ -26,7 +32,17 @@ export function createArtistTagFetcher(): ArtistTagFetcher {
if (!key) return []
const cached = cache.get(key)
if (cached) return cached
const pending = getArtistTopTags(artist).catch(() => [])
const pending = getArtistTopTags(artist)
.catch(() => [])
.then(async (tags) => {
if (tags.length === 0 && spotifyFallback) {
const result = await Promise.resolve(
spotifyFallback(artist),
).catch(() => [])
return Array.isArray(result) ? result : []
}
return tags
})
cache.set(key, pending)
return pending
}
Expand Down
Loading
Loading