From 0341c705ef5c3e02f0ad11d33f547f25dc1a3eb8 Mon Sep 17 00:00:00 2001 From: Pierre <63404022+0-Pierre@users.noreply.github.com> Date: Sun, 16 Mar 2025 13:18:20 +0100 Subject: [PATCH] refactor: replace Promise.all with Promise.allSettled to handle external API failures more gracefully --- server/routes/artist.ts | 42 +++++---- server/routes/discover.ts | 13 +-- server/routes/music.ts | 174 +++++++++++++++++++++++--------------- server/routes/person.ts | 30 ++++--- server/routes/search.ts | 29 +++++-- 5 files changed, 181 insertions(+), 107 deletions(-) diff --git a/server/routes/artist.ts b/server/routes/artist.ts index 3af60a56..6db91c7d 100644 --- a/server/routes/artist.ts +++ b/server/routes/artist.ts @@ -83,23 +83,31 @@ artistRoutes.get('/:id', async (req, res, next) => { const mbIds = releaseGroupsToProcess.map((rg) => rg.mbid); - const [artistWikipedia, artistImages, relatedMedia, albumMetadata] = - await Promise.all([ - musicbrainz - .getArtistWikipediaExtract({ - artistMbid: req.params.id, - language: req.locale, - }) - .catch(() => null), - !metadataArtist?.tadbThumb && !metadataArtist?.tadbCover - ? theAudioDb.getArtistImages(req.params.id) - : theAudioDb.getArtistImagesFromCache(req.params.id), - Media.getRelatedMedia(req.user, mbIds), - getRepository(MetadataAlbum).find({ - where: { mbAlbumId: In(mbIds) }, - cache: true, - }), - ]); + const responses = await Promise.allSettled([ + musicbrainz + .getArtistWikipediaExtract({ + artistMbid: req.params.id, + language: req.locale, + }) + .catch(() => null), + !metadataArtist?.tadbThumb && !metadataArtist?.tadbCover + ? theAudioDb.getArtistImages(req.params.id) + : theAudioDb.getArtistImagesFromCache(req.params.id), + Media.getRelatedMedia(req.user, mbIds), + getRepository(MetadataAlbum).find({ + where: { mbAlbumId: In(mbIds) }, + cache: true, + }), + ]); + + const artistWikipedia = + responses[0].status === 'fulfilled' ? responses[0].value : null; + const artistImages = + responses[1].status === 'fulfilled' ? responses[1].value : null; + const relatedMedia = + responses[2].status === 'fulfilled' ? responses[2].value : []; + const albumMetadata = + responses[3].status === 'fulfilled' ? responses[3].value : []; const metadataMap = new Map( albumMetadata.map((metadata) => [metadata.mbAlbumId, metadata]) diff --git a/server/routes/discover.ts b/server/routes/discover.ts index 8ffe7b33..3f96f3e5 100644 --- a/server/routes/discover.ts +++ b/server/routes/discover.ts @@ -1294,17 +1294,18 @@ discoverRoutes.get('/music/artists', async (req, res, next) => { }; } - const [artistImageResults] = await Promise.all([ + const responses = await Promise.allSettled([ artistsNeedingImages.length > 0 - ? (theAudioDb.batchGetArtistImages( - artistsNeedingImages - ) as Promise) - : ({} as ArtistImageResults), + ? theAudioDb.batchGetArtistImages(artistsNeedingImages) + : Promise.resolve({} as ArtistImageResults), artistsForPersonMapping.length > 0 ? personMapper.batchGetMappings(artistsForPersonMapping) - : {}, + : Promise.resolve({}), ]); + const artistImageResults = + responses[0].status === 'fulfilled' ? responses[0].value : {}; + let updatedArtistMetadata = artistMetadata; if (artistsForPersonMapping.length > 0 || artistsNeedingImages.length > 0) { updatedArtistMetadata = await getRepository(MetadataArtist).find({ diff --git a/server/routes/music.ts b/server/routes/music.ts index f8e5db55..4bc47892 100644 --- a/server/routes/music.ts +++ b/server/routes/music.ts @@ -59,7 +59,7 @@ musicRoutes.get('/:id', async (req, res, next) => { metadataArtist, trackArtistMetadata, artistWikipedia, - ] = await Promise.all([ + ] = await Promise.allSettled([ getRepository(MetadataAlbum).findOne({ where: { mbAlbumId: req.params.id }, }), @@ -81,6 +81,17 @@ musicRoutes.get('/:id', async (req, res, next) => { : Promise.resolve(null), ]); + const resolvedMetadataAlbum = + metadataAlbum.status === 'fulfilled' ? metadataAlbum.value : null; + const resolvedMetadataArtist = + metadataArtist.status === 'fulfilled' ? metadataArtist.value : undefined; + const resolvedTrackArtistMetadata = + trackArtistMetadata.status === 'fulfilled' + ? trackArtistMetadata.value + : []; + const resolvedArtistWikipedia = + artistWikipedia.status === 'fulfilled' ? artistWikipedia.value : null; + const trackArtistsToMap = albumDetails.mediums .flatMap((medium) => medium.tracks) .flatMap((track) => @@ -88,7 +99,7 @@ musicRoutes.get('/:id', async (req, res, next) => { .filter((artist) => artist.artist_mbid) .filter( (artist) => - !trackArtistMetadata.some( + !resolvedTrackArtistMetadata.some( (m) => m.mbArtistId === artist.artist_mbid && m.tmdbPersonId ) ) @@ -98,44 +109,54 @@ musicRoutes.get('/:id', async (req, res, next) => { })) ); - const [artistImages, personMappingResult, updatedArtistMetadata] = - await Promise.all([ - artistId && !metadataArtist?.tadbThumb && !metadataArtist?.tadbCover - ? theAudioDb.getArtistImages(artistId) - : Promise.resolve(null), - artistId && isPerson && !metadataArtist?.tmdbPersonId - ? personMapper - .getMapping( - artistId, - albumDetails.release_group_metadata.artist.artists[0].name - ) - .catch(() => null) - : Promise.resolve(null), - trackArtistsToMap.length > 0 - ? personMapper.batchGetMappings(trackArtistsToMap).then(() => - getRepository(MetadataArtist).find({ - where: { mbArtistId: In(trackArtistIds) }, - }) + const responses = await Promise.allSettled([ + artistId && + !resolvedMetadataArtist?.tadbThumb && + !resolvedMetadataArtist?.tadbCover + ? theAudioDb.getArtistImages(artistId) + : Promise.resolve(null), + artistId && isPerson && !resolvedMetadataArtist?.tmdbPersonId + ? personMapper + .getMapping( + artistId, + albumDetails.release_group_metadata.artist.artists[0].name ) - : Promise.resolve(trackArtistMetadata), - ]); + .catch(() => null) + : Promise.resolve(null), + trackArtistsToMap.length > 0 + ? personMapper.batchGetMappings(trackArtistsToMap).then(() => + getRepository(MetadataArtist).find({ + where: { mbArtistId: In(trackArtistIds) }, + }) + ) + : Promise.resolve(resolvedTrackArtistMetadata), + ]); + + const artistImages = + responses[0].status === 'fulfilled' ? responses[0].value : null; + const personMappingResult = + responses[1].status === 'fulfilled' ? responses[1].value : null; + const updatedArtistMetadata = + responses[2].status === 'fulfilled' + ? responses[2].value + : resolvedTrackArtistMetadata; const updatedMetadataArtist = personMappingResult && artistId ? await getRepository(MetadataArtist).findOne({ where: { mbArtistId: artistId }, }) - : metadataArtist; + : resolvedMetadataArtist; const mappedDetails = mapMusicDetails(albumDetails, media, onUserWatchlist); const finalTrackArtistMetadata = - updatedArtistMetadata || trackArtistMetadata; + updatedArtistMetadata || resolvedTrackArtistMetadata; return res.status(200).json({ ...mappedDetails, - posterPath: metadataAlbum?.caaUrl ?? null, - needsCoverArt: !metadataAlbum?.caaUrl, - artistWikipedia, + posterPath: resolvedMetadataAlbum?.caaUrl ?? null, + needsCoverArt: !resolvedMetadataAlbum?.caaUrl, + artistWikipedia: resolvedArtistWikipedia, artistThumb: updatedMetadataArtist?.tmdbThumb ?? updatedMetadataArtist?.tadbThumb ?? @@ -202,15 +223,20 @@ musicRoutes.get('/:id/artist', async (req, res, next) => { }); } - const [artistDetails, cachedTheAudioDb, metadataArtist] = await Promise.all( - [ - listenbrainzApi.getArtist(artistData.artist_mbid), - theAudioDb.getArtistImagesFromCache(artistData.artist_mbid), - metadataArtistRepository.findOne({ - where: { mbArtistId: artistData.artist_mbid }, - }), - ] - ); + const responses = await Promise.allSettled([ + listenbrainzApi.getArtist(artistData.artist_mbid), + theAudioDb.getArtistImagesFromCache(artistData.artist_mbid), + metadataArtistRepository.findOne({ + where: { mbArtistId: artistData.artist_mbid }, + }), + ]); + + const artistDetails = + responses[0].status === 'fulfilled' ? responses[0].value : null; + const cachedTheAudioDb = + responses[1].status === 'fulfilled' ? responses[1].value : null; + const metadataArtist = + responses[2].status === 'fulfilled' ? responses[2].value : null; if (!artistDetails) { return res.status(404).json({ status: 404, message: 'Artist not found' }); @@ -229,18 +255,24 @@ musicRoutes.get('/:id/artist', async (req, res, next) => { const similarArtistIds = artistDetails.similarArtists?.artists?.map((a) => a.artist_mbid) ?? []; - const [relatedMedia, albumMetadata, similarArtistMetadata] = - await Promise.all([ - Media.getRelatedMedia(req.user, releaseGroupIds), - metadataAlbumRepository.find({ - where: { mbAlbumId: In(releaseGroupIds) }, - }), - similarArtistIds.length > 0 - ? metadataArtistRepository.find({ - where: { mbArtistId: In(similarArtistIds) }, - }) - : Promise.resolve([]), - ]); + const mediaResponses = await Promise.allSettled([ + Media.getRelatedMedia(req.user, releaseGroupIds), + metadataAlbumRepository.find({ + where: { mbAlbumId: In(releaseGroupIds) }, + }), + similarArtistIds.length > 0 + ? metadataArtistRepository.find({ + where: { mbArtistId: In(similarArtistIds) }, + }) + : Promise.resolve([]), + ]); + + const relatedMedia = + mediaResponses[0].status === 'fulfilled' ? mediaResponses[0].value : []; + const albumMetadata = + mediaResponses[1].status === 'fulfilled' ? mediaResponses[1].value : []; + const similarArtistMetadata = + mediaResponses[2].status === 'fulfilled' ? mediaResponses[2].value : []; const albumMetadataMap = new Map( albumMetadata.map((metadata) => [metadata.mbAlbumId, metadata]) @@ -272,24 +304,34 @@ musicRoutes.get('/:id/artist', async (req, res, next) => { { artistThumb: string | null; artistBackground: string | null } >; - const [artistImageResults, updatedArtistMetadata, artistImagesResult] = - await Promise.all([ - artistsNeedingImages.length > 0 - ? theAudioDb.batchGetArtistImages(artistsNeedingImages) - : ({} as ArtistImageResults), - personArtists.length > 0 - ? personMapper.batchGetMappings(personArtists).then(() => - metadataArtistRepository.find({ - where: { mbArtistId: In(similarArtistIds) }, - }) - ) - : Promise.resolve(similarArtistMetadata), - !cachedTheAudioDb && - !metadataArtist?.tadbThumb && - !metadataArtist?.tadbCover - ? theAudioDb.getArtistImages(artistData.artist_mbid) - : Promise.resolve(null), - ]); + const artistResponses = await Promise.allSettled([ + artistsNeedingImages.length > 0 + ? theAudioDb.batchGetArtistImages(artistsNeedingImages) + : ({} as ArtistImageResults), + personArtists.length > 0 + ? personMapper.batchGetMappings(personArtists).then(() => + metadataArtistRepository.find({ + where: { mbArtistId: In(similarArtistIds) }, + }) + ) + : Promise.resolve(similarArtistMetadata), + !cachedTheAudioDb && + !metadataArtist?.tadbThumb && + !metadataArtist?.tadbCover + ? theAudioDb.getArtistImages(artistData.artist_mbid) + : Promise.resolve(null), + ]); + + const artistImageResults = + artistResponses[0].status === 'fulfilled' ? artistResponses[0].value : {}; + const updatedArtistMetadata = + artistResponses[1].status === 'fulfilled' + ? artistResponses[1].value + : similarArtistMetadata; + const artistImagesResult = + artistResponses[2].status === 'fulfilled' + ? artistResponses[2].value + : null; const relatedMediaMap = new Map( relatedMedia.map((media) => [media.mbId, media]) diff --git a/server/routes/person.ts b/server/routes/person.ts index ebf4eca1..b62869e3 100644 --- a/server/routes/person.ts +++ b/server/routes/person.ts @@ -96,18 +96,24 @@ personRoutes.get('/:id', async (req, res, next) => { const allReleaseGroupIds = releaseGroupsToProcess.map((rg) => rg.mbid); - const [artistImagesPromise, relatedMedia, albumMetadata] = - await Promise.all([ - !existingMetadata.tadbThumb && !existingMetadata.tadbCover - ? theAudioDb.getArtistImages(existingMetadata.mbArtistId) - : Promise.resolve(null), - Media.getRelatedMedia(req.user, allReleaseGroupIds), - getRepository(MetadataAlbum).find({ - where: { mbAlbumId: In(allReleaseGroupIds) }, - select: ['mbAlbumId', 'caaUrl'], - cache: true, - }), - ]); + const responses = await Promise.allSettled([ + !existingMetadata.tadbThumb && !existingMetadata.tadbCover + ? theAudioDb.getArtistImages(existingMetadata.mbArtistId) + : Promise.resolve(null), + Media.getRelatedMedia(req.user, allReleaseGroupIds), + getRepository(MetadataAlbum).find({ + where: { mbAlbumId: In(allReleaseGroupIds) }, + select: ['mbAlbumId', 'caaUrl'], + cache: true, + }), + ]); + + const artistImagesPromise = + responses[0].status === 'fulfilled' ? responses[0].value : null; + const relatedMedia = + responses[1].status === 'fulfilled' ? responses[1].value : []; + const albumMetadata = + responses[2].status === 'fulfilled' ? responses[2].value : []; if (artistImagesPromise) { existingMetadata.tadbThumb = artistImagesPromise.artistThumb; diff --git a/server/routes/search.ts b/server/routes/search.ts index 638cb774..0a44b3de 100644 --- a/server/routes/search.ts +++ b/server/routes/search.ts @@ -17,7 +17,6 @@ import { In } from 'typeorm'; const searchRoutes = Router(); -const ITEMS_PER_PAGE = 20; searchRoutes.get('/', async (req, res, next) => { const queryString = req.query.query as string; const page = Number(req.query.page) || 1; @@ -42,7 +41,7 @@ searchRoutes.get('/', async (req, res, next) => { const theAudioDb = TheAudioDb.getInstance(); const personMapper = TmdbPersonMapper.getInstance(); - const [tmdbResults, albumResults, artistResults] = await Promise.all([ + const responses = await Promise.allSettled([ tmdb.searchMulti({ query: queryString, page, @@ -50,14 +49,23 @@ searchRoutes.get('/', async (req, res, next) => { }), musicbrainz.searchAlbum({ query: queryString, - limit: ITEMS_PER_PAGE, + limit: 20, }), musicbrainz.searchArtist({ query: queryString, - limit: ITEMS_PER_PAGE, + limit: 20, }), ]); + const tmdbResults = + responses[0].status === 'fulfilled' + ? responses[0].value + : { page: 1, results: [], total_pages: 1, total_results: 0 }; + const albumResults = + responses[1].status === 'fulfilled' ? responses[1].value : []; + const artistResults = + responses[2].status === 'fulfilled' ? responses[2].value : []; + const personIds = tmdbResults.results .filter( (result) => result.media_type === 'person' && !result.profile_path @@ -162,7 +170,7 @@ searchRoutes.get('/', async (req, res, next) => { { artistThumb: string | null; artistBackground: string | null } >; - const [personMappingResults, artistImageResults] = await Promise.all([ + const externalApiResponses = await Promise.allSettled([ artistsNeedingMapping.length > 0 ? personMapper.batchGetMappings(artistsNeedingMapping) : ({} as PersonMappingResult), @@ -171,6 +179,15 @@ searchRoutes.get('/', async (req, res, next) => { : ({} as ArtistImageResult), ]); + const personMappingResults = + externalApiResponses[0].status === 'fulfilled' + ? externalApiResponses[0].value + : ({} as PersonMappingResult); + const artistImageResults = + externalApiResponses[1].status === 'fulfilled' + ? externalApiResponses[1].value + : ({} as ArtistImageResult); + let updatedArtistsMetadataMap = artistsMetadataMap; if ( (artistsNeedingMapping.length > 0 || artistsNeedingImages.length > 0) && @@ -244,7 +261,7 @@ searchRoutes.get('/', async (req, res, next) => { const totalItems = tmdbResults.total_results + musicResults.length; const totalPages = Math.max( tmdbResults.total_pages, - Math.ceil(totalItems / ITEMS_PER_PAGE) + Math.ceil(totalItems / 20) ); const combinedResults =