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
4 changes: 4 additions & 0 deletions packages/backend/src/errors/AppError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ export class AppError extends Error {
static serviceUnavailable(message = 'Service unavailable'): AppError {
return new AppError(503, message)
}

static gatewayTimeout(message = 'Gateway timeout'): AppError {
return new AppError(504, message)
}
}
47 changes: 32 additions & 15 deletions packages/backend/src/routes/twitch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { writeLimiter } from '../middleware/rateLimit'
import { asyncHandler } from '../middleware/asyncHandler'
import { managementSchemas as s } from '../schemas/management'
import { twitchNotificationService } from '@lucky/shared/services'
import { warnLog } from '@lucky/shared/utils'
import { AppError } from '../errors/AppError'
import { z } from 'zod'

Expand Down Expand Up @@ -59,7 +60,14 @@ async function getAppAccessToken(
expiresAt: Date.now() + data.expires_in * 1000,
}
return data.access_token
} catch {
} catch (error) {
if (error instanceof DOMException && error.name === 'TimeoutError') {
warnLog({
message: 'Twitch token exchange timed out',
error,
})
throw error
}
return null
}
}
Expand Down Expand Up @@ -125,22 +133,31 @@ export function setupTwitchRoutes(app: Express): void {
if (typeof login !== 'string' || login.length < 1) {
throw AppError.badRequest('login query parameter required')
}
const { user, configured } = await lookupTwitchUser(
login.toLowerCase(),
)
if (!configured) {
throw AppError.serviceUnavailable(
'Twitch is not configured on this server',
try {
const { user, configured } = await lookupTwitchUser(
login.toLowerCase(),
)
if (!configured) {
throw AppError.serviceUnavailable(
'Twitch is not configured on this server',
)
}
if (!user) {
throw AppError.notFound('Twitch user not found')
}
res.json({
id: user.id,
login: user.login,
displayName: user.display_name,
})
} catch (error) {
if (error instanceof DOMException && error.name === 'TimeoutError') {
throw AppError.gatewayTimeout(
'Twitch API request timed out, please try again',
)
}
throw error
}
if (!user) {
throw AppError.notFound('Twitch user not found')
}
res.json({
id: user.id,
login: user.login,
displayName: user.display_name,
})
}),
)

Expand Down
57 changes: 41 additions & 16 deletions packages/backend/src/services/artistSuggestion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,23 +226,45 @@ export class ArtistSuggestionService {
const seen = new Set<string>()
const timeRanges = ['short_term', 'medium_term', 'long_term'] as const

try {
const promises = timeRanges.map((range) =>
fetch(
`https://api.spotify.com/v1/me/top/artists?limit=50&time_range=${range}`,
{
headers: {
Authorization: `Bearer ${accessToken}`,
},
signal: AbortSignal.timeout(8_000),
const promises = timeRanges.map((range) =>
fetch(
`https://api.spotify.com/v1/me/top/artists?limit=50&time_range=${range}`,
{
headers: {
Authorization: `Bearer ${accessToken}`,
},
),
)
const responses = await Promise.all(promises)
signal: AbortSignal.timeout(8_000),
},
),
)
const results = await Promise.allSettled(promises)

for (let i = 0; i < results.length; i++) {
const result = results[i]
const timeRange = timeRanges[i]

if (result.status === 'rejected') {
if (
result.reason instanceof DOMException &&
result.reason.name === 'TimeoutError'
) {
warnLog({
message: `Spotify top artists fetch timed out for time_range=${timeRange}`,
error: result.reason,
})
} else {
errorLog({
message: `Failed to fetch Spotify top artists for time_range=${timeRange}`,
error: result.reason,
})
}
continue
}

for (const res of responses) {
if (!res.ok) continue
const res = result.value
if (!res.ok) continue

try {
const data = (await res.json()) as {
items?: unknown[]
}
Expand Down Expand Up @@ -270,9 +292,12 @@ export class ArtistSuggestionService {
}
}
}
} catch (error) {
errorLog({
message: `Failed to parse Spotify response for time_range=${timeRange}`,
error,
})
}
} catch (error) {
errorLog({ message: 'Failed to fetch Spotify top artists', error })
}

return collected
Expand Down
29 changes: 29 additions & 0 deletions packages/backend/tests/unit/routes/twitch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,33 @@ describe('GET /api/twitch/users', () => {
const res = await request(app).get('/api/twitch/users?login=testuser')
expect(res.status).toBe(503)
})

test('returns 504 when twitch API request times out', async () => {
process.env.TWITCH_CLIENT_ID = 'test-client-id'
process.env.TWITCH_ACCESS_TOKEN = 'test-token'
const timeoutError = new DOMException('The operation was aborted', 'AbortError')
Object.defineProperty(timeoutError, 'name', { value: 'TimeoutError', writable: false })
;(global.fetch as jest.MockedFunction<typeof fetch>).mockRejectedValue(
timeoutError,
)
const app = createApp()
const res = await request(app).get('/api/twitch/users?login=testuser')
expect(res.status).toBe(504)
expect(res.body.error).toMatch(/timed out/)
})

test('returns 504 when token exchange fetch times out', async () => {
process.env.TWITCH_CLIENT_ID = 'test-client-id'
process.env.TWITCH_CLIENT_SECRET = 'test-secret'
delete process.env.TWITCH_ACCESS_TOKEN
const timeoutError = new DOMException('The operation was aborted', 'AbortError')
Object.defineProperty(timeoutError, 'name', { value: 'TimeoutError', writable: false })
;(global.fetch as jest.MockedFunction<typeof fetch>).mockRejectedValue(
timeoutError,
)
const app = createApp()
const res = await request(app).get('/api/twitch/users?login=testuser')
expect(res.status).toBe(504)
expect(res.body.error).toMatch(/timed out/)
})
})
69 changes: 69 additions & 0 deletions packages/backend/tests/unit/services/artistSuggestion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -539,5 +539,74 @@ describe('ArtistSuggestionService', () => {
global.fetch = originalFetch
}
})

test('returns partial results when one time_range times out', async () => {
const mockArtist = {
id: 'artist_1',
name: 'Test Artist',
images: [{ url: 'https://example.com/image.jpg' }],
popularity: 75,
genres: ['pop'],
}

const originalFetch = global.fetch
const fetch = jest.fn()
const timeoutError = new DOMException('Timeout', 'AbortError')
Object.defineProperty(timeoutError, 'name', {
value: 'TimeoutError',
writable: false,
})

// First call (short_term): success
// Second call (medium_term): timeout
// Third call (long_term): success
let callCount = 0
fetch.mockImplementation(async () => {
callCount++
if (callCount === 1) {
// short_term success
return {
ok: true,
json: async () => ({ items: [mockArtist] }),
}
} else if (callCount === 2) {
// medium_term timeout
throw timeoutError
} else {
// long_term success
return {
ok: true,
json: async () => ({
items: [{ ...mockArtist, id: 'artist_2', name: 'Another Artist' }],
}),
}
}
})
global.fetch = fetch as unknown as typeof global.fetch

try {
;(spotifyLinkService as any).getValidAccessToken = jest.fn().mockResolvedValue('test-token')
const suggestions = await service.getSuggestions('user_123')

// Should have results from short_term and long_term, skipping the timed-out medium_term
expect(suggestions.length).toBe(2)
expect(suggestions).toContainEqual(
expect.objectContaining({ id: 'artist_1' }),
)
expect(suggestions).toContainEqual(
expect.objectContaining({ id: 'artist_2' }),
)

// warnLog should be called for the timeout
const { warnLog } = await import('@lucky/shared/utils')
expect(warnLog).toHaveBeenCalledWith(
expect.objectContaining({
message: expect.stringContaining('timed out'),
}),
)
} finally {
global.fetch = originalFetch
}
})
})
})
Loading