Skip to content

Commit 67c594f

Browse files
committed
fix(lastfm): Check for singular scrobble response
1 parent 6b938c4 commit 67c594f

File tree

1 file changed

+66
-45
lines changed

1 file changed

+66
-45
lines changed

src/backend/common/vendor/LastfmApiClient.ts

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,11 @@ export default class LastfmApiClient extends AbstractApiClient implements Pagina
253253
return 'second';
254254
}
255255

256-
getPaginatedTimeRangeListens = async (fetchOptions: PaginatedListensTimeRangeOptions<number>, options: {includeNowPlaying?: boolean} = {}): Promise<PaginatedTimeRangeListensResult<number>> => {
256+
getPaginatedTimeRangeListens = async (fetchOptions: PaginatedListensTimeRangeOptions<number>, options: { includeNowPlaying?: boolean } = {}): Promise<PaginatedTimeRangeListensResult<number>> => {
257257

258258
const resp = await this.getRecentTracksWithPagination(fetchOptions);
259259

260-
const {includeNowPlaying = true} = options;
260+
const { includeNowPlaying = true } = options;
261261

262262
const {
263263
recenttracks: {
@@ -270,49 +270,44 @@ export default class LastfmApiClient extends AbstractApiClient implements Pagina
270270
} = {},
271271
} = resp;
272272

273-
let plays = [];
274-
if(Array.isArray(list)) {
275-
plays = list.reduce((acc: any, x: any) => {
276-
try {
277-
const formatted = formatPlayObj(x);
278-
const {
279-
data: {
280-
track,
281-
playDate,
282-
},
283-
meta: {
284-
mbid,
285-
nowPlaying,
286-
}
287-
} = formatted;
288-
if (nowPlaying === true && !includeNowPlaying) {
289-
// if the track is "now playing" it doesn't get a timestamp so we can't determine when it started playing
290-
// and don't want to accidentally count the same track at different timestamps by artificially assigning it 'now' as a timestamp
291-
// so we'll just ignore it in the context of recent tracks since really we only want "tracks that have already finished being played" anyway
292-
this.logger.debug( { track, mbid }, `Ignoring 'now playing' track returned from ${this.upstreamName} client`);
293-
return acc;
294-
} else if (playDate === undefined) {
295-
this.logger.warn({ track, mbid }, `${this.upstreamName} recently scrobbled track did not contain a timestamp, omitting from time frame check`);
296-
return acc;
297-
}
298-
return acc.concat(formatted);
299-
} catch (e) {
300-
this.logger.warn(new Error(`Failed to format ${this.upstreamName} recently scrobbled track, omitting from time frame check`, { cause: e }));
301-
this.logger.debug({data: x}, 'Full api response object:');
302-
return acc;
273+
const plays = list.reduce((acc: any, x: any) => {
274+
try {
275+
const formatted = formatPlayObj(x);
276+
const {
277+
data: {
278+
track,
279+
playDate,
280+
},
281+
meta: {
282+
mbid,
283+
nowPlaying,
303284
}
304-
}, []);
305-
} else {
306-
this.logger.warn({list: list},`Expected tracks to be a list but it was ${typeof list}`);
307-
}
285+
} = formatted;
286+
if (nowPlaying === true && !includeNowPlaying) {
287+
// if the track is "now playing" it doesn't get a timestamp so we can't determine when it started playing
288+
// and don't want to accidentally count the same track at different timestamps by artificially assigning it 'now' as a timestamp
289+
// so we'll just ignore it in the context of recent tracks since really we only want "tracks that have already finished being played" anyway
290+
this.logger.debug({ track, mbid }, `Ignoring 'now playing' track returned from ${this.upstreamName} client`);
291+
return acc;
292+
} else if (playDate === undefined) {
293+
this.logger.warn({ track, mbid }, `${this.upstreamName} recently scrobbled track did not contain a timestamp, omitting from time frame check`);
294+
return acc;
295+
}
296+
return acc.concat(formatted);
297+
} catch (e) {
298+
this.logger.warn(new Error(`Failed to format ${this.upstreamName} recently scrobbled track, omitting from time frame check`, { cause: e }));
299+
this.logger.debug({ data: x }, 'Full api response object:');
300+
return acc;
301+
}
302+
}, []);
308303

309-
return {data: plays, meta: {...fetchOptions, total: parseInt(total, 10), more: fetchOptions.cursor < parseInt(totalPages, 10)}};
304+
return { data: plays, meta: { ...fetchOptions, total: parseInt(total, 10), more: fetchOptions.cursor < parseInt(totalPages, 10) } };
310305

311306
}
312307

313308
getRecentTracks = async (options: TracksFetchOptions = {}): Promise<LastFMUserGetRecentTracksResponse> => {
314309

315-
const { to, from, extended = 1, ...rest} = options;
310+
const { to, from, extended = 1, ...rest } = options;
316311

317312
const requestOpts: Writeable<LastFMUserGetRecentTracksParams> = {
318313
...rest,
@@ -322,23 +317,50 @@ export default class LastfmApiClient extends AbstractApiClient implements Pagina
322317
extended
323318
};
324319

325-
if(to !== undefined) {
320+
if (to !== undefined) {
326321
requestOpts.to = to.toString();
327322
}
328-
if(from !== undefined) {
323+
if (from !== undefined) {
329324
requestOpts.from = from.toString();
330325
}
331326

332-
let resp: LastFMUserGetRecentTracksResponse;
333327
try {
334-
return await this.callApi<LastFMUserGetRecentTracksResponse>(() => this.userApi.getRecentTracks(requestOpts));
328+
const resp = await this.callApi<LastFMUserGetRecentTracksResponse>(() => this.userApi.getRecentTracks(requestOpts));
329+
const {
330+
recenttracks: {
331+
track = [],
332+
...rest
333+
} = {},
334+
...restResp
335+
} = resp;
336+
const correctedResp: Writeable<LastFMUserGetRecentTracksResponse> = {
337+
...restResp,
338+
// @ts-expect-error
339+
recenttracks: {
340+
...rest,
341+
track: [],
342+
}
343+
}
344+
// when the lfm api response only returns one scrobble it formats `track` as a singular scrobble object INSTEAD of an array
345+
// and in all other cases it should return an array of scrobble objects
346+
if (!Array.isArray(track)) {
347+
if (typeof track === 'object' && track !== null) {
348+
correctedResp.recenttracks.track = [track];
349+
this.logger.debug('Converted `track` property to an array');
350+
} else {
351+
this.logger.warn({ track }, `Expected track to be an array or object but it was ${track === null ? 'null' : typeof track}`);
352+
}
353+
} else {
354+
correctedResp.recenttracks.track = track;
355+
}
356+
return correctedResp;
335357
} catch (e) {
336-
if(e.message.includes('Invalid resource specified')) {
358+
if (e.message.includes('Invalid resource specified')) {
337359
// likely the user does not have any scrobbles on their profile yet
338360
// https://github.com/FoxxMD/multi-scrobbler/issues/401#issuecomment-3749489057
339361
// https://github.com/libre-fm/libre-fm/discussions/91#discussioncomment-15456070
340362
// so we log as a warning and return empty array instead
341-
this.logger.warn(new Error('This error occurs when a librefm (and lastfm?) account has no existing scrobbles yet. If you are seeing this warning and this is not the case, please create an issue', {cause: e}));
363+
this.logger.warn(new Error('This error occurs when a librefm (and lastfm?) account has no existing scrobbles yet. If you are seeing this warning and this is not the case, please create an issue', { cause: e }));
342364
return {
343365
recenttracks: {
344366
track: [],
@@ -352,7 +374,6 @@ export default class LastfmApiClient extends AbstractApiClient implements Pagina
352374
}
353375
}
354376
}
355-
this.logger.debug(resp);
356377
throw e;
357378
}
358379
}

0 commit comments

Comments
 (0)