From 964a58a3cb6cc61c38fb3a1e4618d0321b7e1b5e Mon Sep 17 00:00:00 2001 From: JD Francis Date: Thu, 13 Jun 2024 15:09:17 -0500 Subject: [PATCH 1/6] wip /stream recovery logic --- .../common/src/store/cache/tracks/actions.ts | 2 +- .../components/audio/RNVideoAudioPlayer.tsx | 1 - packages/web/src/common/store/player/sagas.ts | 25 ++++++++++++++++--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/packages/common/src/store/cache/tracks/actions.ts b/packages/common/src/store/cache/tracks/actions.ts index 754cc15100f..296271df07b 100644 --- a/packages/common/src/store/cache/tracks/actions.ts +++ b/packages/common/src/store/cache/tracks/actions.ts @@ -49,6 +49,6 @@ export function fetchStreamUrls(trackIds: ID[]) { return { type: FETCH_STREAM_URLS, trackIds } } -export function setStreamUrls(streamUrls: { [trackId: ID]: string }) { +export function setStreamUrls(streamUrls: { [trackId: ID]: string | undefined }) { return { type: SET_STREAM_URLS, streamUrls } } diff --git a/packages/mobile/src/components/audio/RNVideoAudioPlayer.tsx b/packages/mobile/src/components/audio/RNVideoAudioPlayer.tsx index 82ae3eb4121..40418bbce4c 100644 --- a/packages/mobile/src/components/audio/RNVideoAudioPlayer.tsx +++ b/packages/mobile/src/components/audio/RNVideoAudioPlayer.tsx @@ -34,7 +34,6 @@ import { useDispatch, useSelector } from 'react-redux' import { usePrevious } from 'react-use' import { useFeatureFlag } from '~/hooks' import { FeatureFlags } from '~/services' -import { getUserId } from '~/store/account/selectors' import { DEFAULT_IMAGE_URL } from 'app/components/image/TrackImage' import { getImageSourceOptimistic } from 'app/hooks/useContentNodeImage' diff --git a/packages/web/src/common/store/player/sagas.ts b/packages/web/src/common/store/player/sagas.ts index cb60b25b893..aba12f16328 100644 --- a/packages/web/src/common/store/player/sagas.ts +++ b/packages/web/src/common/store/player/sagas.ts @@ -2,6 +2,7 @@ import { Kind } from '@audius/common/models' import { FeatureFlags, QueryParams } from '@audius/common/services' import { accountSelectors, + cacheTracksActions, cacheTracksSelectors, cacheActions, queueActions, @@ -42,7 +43,8 @@ import errorSagas from './errorSagas' const { getUserId } = accountSelectors const { setTrackPosition } = playbackPositionActions const { getTrackPosition } = playbackPositionSelectors -const { getTrackStreamUrl } = cacheTracksSelectors +const { getTrackStreamUrl, getTrack } = cacheTracksSelectors +const { setStreamUrls } = cacheTracksActions const { play, playSucceeded, @@ -63,7 +65,6 @@ const { getTrackId, getUid, getCounter, getPlaying, getPlaybackRate } = const { getPlayerBehavior } = queueSelectors const { recordListen } = tracksSocialActions -const { getTrack } = cacheTracksSelectors const { getNftAccessSignatureMap } = gatedContentSelectors const { getIsReachable } = reachabilitySelectors @@ -140,7 +141,8 @@ export function* watchPlay() { trackDuration = getTrackPreviewDuration(track) } - const streamUrl = yield* select(getTrackStreamUrl, track.track_id) + const streamUrl = + 'http://audius-protocol-discovery-provider-1/v1/tracks/Ke0a62w/stream?no_redirect=true&user_data=Gated%20content%20user%20signature%20at%201718308572390&user_id=6NJPlWl&user_signature=0xe3f0acd571a1e739e0d57fb51b5f72f4411a2fc86bc8cb6ef7bf62aa27212caf79785bd6660150442e5d2f7bd8ec7b7fd476f47836d1205566f9dc27fd9b7e9d1b' // yield* select(getTrackStreamUrl, track.track_id) const mp3Url = apiClient.makeUrl( `/tracks/${encodedTrackId}/stream`, @@ -417,7 +419,24 @@ export function* handleAudioErrors() { while (true) { const { error, data } = yield* take(chan) const trackId = yield* select(getTrackId) + console.log(data) if (trackId) { + const getFeatureEnabled = yield* getContext('getFeatureEnabled') + const usePrefetchStreamUrls = yield* call( + getFeatureEnabled, + FeatureFlags.PREFETCH_STREAM_URLS + ) + const streamUrl = yield* select(getTrackStreamUrl, trackId) + console.log({ streamUrl }) + // Check if we were attempting to use a prefetched url + if (streamUrl && usePrefetchStreamUrls) { + console.log('Swapping back to /stream') + // The pre-fetched stream url failed, so we unset it + yield* put(setStreamUrls({ [trackId]: undefined }) + // Trigger another play action. + // This time with the prefetch url unset will force it to use the discovery node /stream endpoint fallbcak + yield* put(play({ uid: 'error', trackId })) + } yield* put(errorAction({ error, trackId, info: data })) } } From fe99a191d0114a40e80b17aedf35b24c09fced5f Mon Sep 17 00:00:00 2001 From: JD Francis Date: Thu, 13 Jun 2024 16:13:13 -0500 Subject: [PATCH 2/6] Add error handling for web saga --- .../common/src/store/cache/tracks/actions.ts | 5 +++- packages/web/src/common/store/player/sagas.ts | 25 +++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/common/src/store/cache/tracks/actions.ts b/packages/common/src/store/cache/tracks/actions.ts index 296271df07b..f88ed65d676 100644 --- a/packages/common/src/store/cache/tracks/actions.ts +++ b/packages/common/src/store/cache/tracks/actions.ts @@ -49,6 +49,9 @@ export function fetchStreamUrls(trackIds: ID[]) { return { type: FETCH_STREAM_URLS, trackIds } } -export function setStreamUrls(streamUrls: { [trackId: ID]: string | undefined }) { +// The value should only be undefined if the stream URL had an error +export function setStreamUrls(streamUrls: { + [trackId: ID]: string | undefined +}) { return { type: SET_STREAM_URLS, streamUrls } } diff --git a/packages/web/src/common/store/player/sagas.ts b/packages/web/src/common/store/player/sagas.ts index aba12f16328..20bf2f7b280 100644 --- a/packages/web/src/common/store/player/sagas.ts +++ b/packages/web/src/common/store/player/sagas.ts @@ -141,8 +141,7 @@ export function* watchPlay() { trackDuration = getTrackPreviewDuration(track) } - const streamUrl = - 'http://audius-protocol-discovery-provider-1/v1/tracks/Ke0a62w/stream?no_redirect=true&user_data=Gated%20content%20user%20signature%20at%201718308572390&user_id=6NJPlWl&user_signature=0xe3f0acd571a1e739e0d57fb51b5f72f4411a2fc86bc8cb6ef7bf62aa27212caf79785bd6660150442e5d2f7bd8ec7b7fd476f47836d1205566f9dc27fd9b7e9d1b' // yield* select(getTrackStreamUrl, track.track_id) + const streamUrl = yield* select(getTrackStreamUrl, track.track_id) const mp3Url = apiClient.makeUrl( `/tracks/${encodedTrackId}/stream`, @@ -419,7 +418,6 @@ export function* handleAudioErrors() { while (true) { const { error, data } = yield* take(chan) const trackId = yield* select(getTrackId) - console.log(data) if (trackId) { const getFeatureEnabled = yield* getContext('getFeatureEnabled') const usePrefetchStreamUrls = yield* call( @@ -427,17 +425,22 @@ export function* handleAudioErrors() { FeatureFlags.PREFETCH_STREAM_URLS ) const streamUrl = yield* select(getTrackStreamUrl, trackId) - console.log({ streamUrl }) // Check if we were attempting to use a prefetched url + // If so we likely have a recovery option if (streamUrl && usePrefetchStreamUrls) { - console.log('Swapping back to /stream') - // The pre-fetched stream url failed, so we unset it - yield* put(setStreamUrls({ [trackId]: undefined }) - // Trigger another play action. - // This time with the prefetch url unset will force it to use the discovery node /stream endpoint fallbcak - yield* put(play({ uid: 'error', trackId })) + const reportToSentry = yield* getContext('reportToSentry') + reportToSentry({ + error: new Error('Audio prefetch playback saga error'), + additionalInfo: { trackId, streamUrl } + }) + // The pre-fetched stream url failed, so we set the value to undefined + yield* put(setStreamUrls({ [trackId]: undefined })) + // Retrigger play action. + // Now the prefetch url is unset and it will instead play from the discovery node /stream endpoint as a fallback + yield* put(play({ trackId })) + } else { + yield* put(errorAction({ error, trackId, info: data })) } - yield* put(errorAction({ error, trackId, info: data })) } } } From 456312b7ee390f57be10ca11bc94d3b37944b23f Mon Sep 17 00:00:00 2001 From: JD Francis Date: Fri, 14 Jun 2024 15:06:34 -0500 Subject: [PATCH 3/6] add mobile error handling --- .../src/components/audio/AudioPlayer.tsx | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/mobile/src/components/audio/AudioPlayer.tsx b/packages/mobile/src/components/audio/AudioPlayer.tsx index 1786b3a43f6..546b4ec95e8 100644 --- a/packages/mobile/src/components/audio/AudioPlayer.tsx +++ b/packages/mobile/src/components/audio/AudioPlayer.tsx @@ -21,7 +21,8 @@ import { playbackPositionSelectors, gatedContentSelectors, calculatePlayerBehavior, - PlayerBehavior + PlayerBehavior, + cacheTracksActions } from '@audius/common/store' import type { Queueable, CommonState } from '@audius/common/store' import { @@ -74,6 +75,7 @@ import { useSavePodcastProgress } from './useSavePodcastProgress' const { getUserId } = accountSelectors const { getUsers } = cacheUsersSelectors const { getTracks, getTrackStreamUrls } = cacheTracksSelectors +const { setStreamUrls } = cacheTracksActions const { getPlaying, getSeek, @@ -170,7 +172,7 @@ export const AudioPlayer = () => { FeatureFlags.PODCAST_CONTROL_UPDATES_ENABLED, FeatureFlags.PODCAST_CONTROL_UPDATES_ENABLED_FALLBACK ) - const { isEnabled: isPerformanceExperimentEnabled } = useFeatureFlag( + const { isEnabled: usePrefetchedStreamUrls } = useFeatureFlag( FeatureFlags.PREFETCH_STREAM_URLS ) const track = useSelector(getCurrentTrack) @@ -267,6 +269,9 @@ export const AudioPlayer = () => { ) const trackStreamUrls = useSelector(getTrackStreamUrls) + const isUsingPrefetchUrl = + trackStreamUrls[track?.track_id ?? -1] !== undefined && + usePrefetchedStreamUrls const reset = useCallback( () => dispatch(playerActions.reset({ shouldAutoplay: false })), [dispatch] @@ -321,7 +326,10 @@ export const AudioPlayer = () => { ]) const makeTrackData = useCallback( - async ({ track, playerBehavior }: QueueableTrack) => { + async ( + { track, playerBehavior }: QueueableTrack, + noPrefetch?: boolean // option to opt out of prefetched stream urls (see error handling method) + ) => { if (!track) { return unlistedTrackFallbackTrackData } @@ -341,7 +349,7 @@ export const AudioPlayer = () => { if (offlineTrackAvailable && isCollectionMarkedForDownload) { const audioFilePath = getLocalAudioPath(trackId) url = `file://${audioFilePath}` - } else if (trackStreamUrl && isPerformanceExperimentEnabled) { + } else if (trackStreamUrl && usePrefetchedStreamUrls && !noPrefetch) { url = trackStreamUrl } else { let queryParams = trackQueryParams.current[trackId] @@ -399,7 +407,7 @@ export const AudioPlayer = () => { isCollectionMarkedForDownload, isNotReachable, isOfflineModeEnabled, - isPerformanceExperimentEnabled, + usePrefetchedStreamUrls, nftAccessSignatureMap, offlineAvailabilityByTrackId, queueTrackOwnersMap, @@ -431,6 +439,20 @@ export const AudioPlayer = () => { if (event.type === Event.PlaybackError) { console.error(`TrackPlayer Playback Error:`, event) + // Special recoverable case where the player was using a prefetched url but it failed + if (isUsingPrefetchUrl && track) { + // Unset the broken stream url, we don't want to accidentally use it again + dispatch(setStreamUrls({ [track.track_id]: undefined })) + // Get an updated track without a prefetched url + const updatedTrack = await makeTrackData( + { + track, + playerBehavior + }, + true + ) + TrackPlayer.load(updatedTrack) + } } if (event.type === Event.RemotePlay || event.type === Event.RemotePause) { @@ -808,7 +830,7 @@ export const AudioPlayer = () => { useAsync(async () => { if (isAudioSetup && didPlayerBehaviorChange) { - const updatedTrack = await makeTrackData(queueTracks[queueIndex]) + const updatedTrack = await makeTrackData(queueTracks[queueIndex], f) await TrackPlayer.load(updatedTrack) updatePlayerInfo({ previewing: calculatePlayerBehavior( From 5ad2b53ffa8d6d8b7539921a9f7dc48802db8e27 Mon Sep 17 00:00:00 2001 From: JD Francis Date: Fri, 14 Jun 2024 15:59:03 -0500 Subject: [PATCH 4/6] fix type accident --- packages/mobile/src/components/audio/AudioPlayer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/mobile/src/components/audio/AudioPlayer.tsx b/packages/mobile/src/components/audio/AudioPlayer.tsx index 546b4ec95e8..133ce7f5ced 100644 --- a/packages/mobile/src/components/audio/AudioPlayer.tsx +++ b/packages/mobile/src/components/audio/AudioPlayer.tsx @@ -830,7 +830,7 @@ export const AudioPlayer = () => { useAsync(async () => { if (isAudioSetup && didPlayerBehaviorChange) { - const updatedTrack = await makeTrackData(queueTracks[queueIndex], f) + const updatedTrack = await makeTrackData(queueTracks[queueIndex]) await TrackPlayer.load(updatedTrack) updatePlayerInfo({ previewing: calculatePlayerBehavior( From a6575cd812cc069466aa47ab8711240d8e6dc0b6 Mon Sep 17 00:00:00 2001 From: JD Francis Date: Fri, 14 Jun 2024 16:41:15 -0500 Subject: [PATCH 5/6] fix ts --- packages/common/src/store/cache/tracks/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/common/src/store/cache/tracks/types.ts b/packages/common/src/store/cache/tracks/types.ts index e9ea89dbdb0..73b8c8c6673 100644 --- a/packages/common/src/store/cache/tracks/types.ts +++ b/packages/common/src/store/cache/tracks/types.ts @@ -2,5 +2,5 @@ import { Cache, ID, Track } from '../../../models' export interface TracksCacheState extends Cache { permalinks: { [permalink: string]: ID } - streamUrls: { [trackId: ID]: string } + streamUrls: { [trackId: ID]: string | undefined } } From 96680b895b0d0749100136d18dffc1f011097d6c Mon Sep 17 00:00:00 2001 From: JD Francis Date: Mon, 17 Jun 2024 18:12:43 -0500 Subject: [PATCH 6/6] reword flag name --- packages/mobile/src/components/audio/AudioPlayer.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/mobile/src/components/audio/AudioPlayer.tsx b/packages/mobile/src/components/audio/AudioPlayer.tsx index d3a0ae492ca..4dc87a0e998 100644 --- a/packages/mobile/src/components/audio/AudioPlayer.tsx +++ b/packages/mobile/src/components/audio/AudioPlayer.tsx @@ -171,7 +171,7 @@ export const AudioPlayer = () => { FeatureFlags.PODCAST_CONTROL_UPDATES_ENABLED, FeatureFlags.PODCAST_CONTROL_UPDATES_ENABLED_FALLBACK ) - const { isEnabled: usePrefetchedStreamUrls } = useFeatureFlag( + const { isEnabled: isStreamPrefetchEnabled } = useFeatureFlag( FeatureFlags.PREFETCH_STREAM_URLS ) const track = useSelector(getCurrentTrack) @@ -269,7 +269,7 @@ export const AudioPlayer = () => { const trackStreamUrls = useSelector(getTrackStreamUrls) const isUsingPrefetchUrl = trackStreamUrls[track?.track_id ?? -1] !== undefined && - usePrefetchedStreamUrls + isStreamPrefetchEnabled const reset = useCallback( () => dispatch(playerActions.reset({ shouldAutoplay: false })), [dispatch] @@ -347,7 +347,7 @@ export const AudioPlayer = () => { if (offlineTrackAvailable && isCollectionMarkedForDownload) { const audioFilePath = getLocalAudioPath(trackId) url = `file://${audioFilePath}` - } else if (trackStreamUrl && usePrefetchedStreamUrls && !noPrefetch) { + } else if (trackStreamUrl && isStreamPrefetchEnabled && !noPrefetch) { url = trackStreamUrl } else { let queryParams = trackQueryParams.current[trackId] @@ -404,7 +404,7 @@ export const AudioPlayer = () => { currentUserId, isCollectionMarkedForDownload, isNotReachable, - usePrefetchedStreamUrls, + isStreamPrefetchEnabled, nftAccessSignatureMap, offlineAvailabilityByTrackId, queueTrackOwnersMap,