diff --git a/src/stores/media/index.ts b/src/stores/media/index.ts index ed33f8804b..94975a6f7c 100644 --- a/src/stores/media/index.ts +++ b/src/stores/media/index.ts @@ -369,6 +369,8 @@ export const useMediaStore = defineStore('media', { } }, + // Handles errors for media fetches and sets the fetch state accordingly + // Reported errors are logged to Sentry async handleMediaError({ mediaType, error, @@ -378,16 +380,31 @@ export const useMediaStore = defineStore('media', { }) { let errorMessage if (axios.isAxiosError(error)) { - errorMessage = - error.response?.status === 500 - ? 'There was a problem with our servers' - : `Request failed with status ${ - error.response?.status ?? 'unknown' - }` + // If the error is an axios error: + // If the error has a response property, and recieved a response that is not in the 2xx range, + // then the error is logged to Sentry + if (error.response) { + errorMessage = `Error fetching ${mediaType} from API. Request failed with status code: ${error.response.status}` + } else if (error.request) { + // If the error has a request property, but no response, then we capture the event in Sentry + errorMessage = `Error fetching ${mediaType} from API. No response received from the server` + } else { + // Something happened in setting up the request that triggered an Error + errorMessage = `Error fetching ${mediaType} from API. Unknown Axios error` + } } else { - errorMessage = - error instanceof Error ? error.message : 'Oops! Something went wrong' + // If the error is not an axios error, then we capture the event in Sentry + errorMessage = `Error fetching ${mediaType} from API. Unknown error` } + + this.$nuxt.$sentry.captureEvent({ + message: errorMessage, + extra: { + mediaType, + error, + }, + }) + this._updateFetchState(mediaType, 'end', errorMessage) if (!axios.isAxiosError(error)) { throw new Error(errorMessage) diff --git a/test/unit/setup-after-env.js b/test/unit/setup-after-env.js index c44951a680..91e484b822 100644 --- a/test/unit/setup-after-env.js +++ b/test/unit/setup-after-env.js @@ -1 +1,11 @@ +import Vue from 'vue' import '@testing-library/jest-dom' + +Vue.prototype.$nuxt = { + context: { + $sentry: { + captureException: jest.fn(), + captureEvent: jest.fn(), + }, + }, +} diff --git a/test/unit/specs/stores/media-store.spec.js b/test/unit/specs/stores/media-store.spec.js index 2ec202e0b2..ca8806b65e 100644 --- a/test/unit/specs/stores/media-store.spec.js +++ b/test/unit/specs/stores/media-store.spec.js @@ -1,3 +1,5 @@ +import { AxiosError } from 'axios' + import { setActivePinia, createPinia } from '~~/test/unit/test-utils/pinia' import { deepClone } from '~/utils/clone' @@ -7,11 +9,6 @@ import { useSearchStore } from '~/stores/search' import { ALL_MEDIA, AUDIO, IMAGE, supportedMediaTypes } from '~/constants/media' import { initialFetchState } from '~/composables/use-fetch-state' -jest.mock('axios', () => ({ - ...jest.requireActual('axios'), - isAxiosError: jest.fn((obj) => 'response' in obj), -})) - const uuids = [ '0dea3af1-27a4-4635-bab6-4b9fb76a59f5', '32c22b5b-f2f9-47db-b64f-6b86c2431942', @@ -368,31 +365,87 @@ describe('Media Store', () => { it('handleMediaError handles 500 error', () => { const mediaType = AUDIO - const error = { response: { status: 500, message: 'Server error' } } + const error = new AxiosError( + '500 server error', + 'ERR_BAD_RESPONSE', + undefined, + { path: '/foo' }, + { status: 500 } + ) const mediaStore = useMediaStore() mediaStore.handleMediaError({ mediaType, error }) - expect(mediaStore.mediaFetchState[mediaType].fetchingError).toEqual( - 'There was a problem with our servers' - ) + expect(mediaStore.$nuxt.$sentry.captureEvent).toHaveBeenCalledWith({ + message: + 'Error fetching audio from API. Request failed with status code: 500', + extra: { mediaType, error }, + }) }) it('handleMediaError handles a 403 error', () => { const mediaType = AUDIO - const error = { response: { status: 403 } } + const error = new AxiosError( + '403 error', + 'ERR_BAD_REQUEST', + undefined, + { path: '/foo' }, + { status: 403 } + ) const mediaStore = useMediaStore() mediaStore.handleMediaError({ mediaType, error }) - expect(mediaStore.mediaFetchState[mediaType].fetchingError).toEqual( - 'Request failed with status 403' + expect(mediaStore.$nuxt.$sentry.captureEvent).toHaveBeenCalledWith({ + message: + 'Error fetching audio from API. Request failed with status code: 403', + extra: { mediaType, error }, + }) + expect(mediaStore.mediaFetchState.audio.fetchingError).toEqual( + 'Error fetching audio from API. Request failed with status code: 403' ) + expect(mediaStore.mediaFetchState.audio.isFetching).toEqual(false) }) - it('handleMediaError throws a new error on error when server did not respond', async () => { + it('handleMediaError handles an error when server did not respond', async () => { const mediaStore = useMediaStore() + const mediaType = AUDIO + const noResponseAxiosError = new AxiosError( + 'Unknown error', + 'ETIMEDOUT', + undefined, + { + path: '/foo', + } + ) + await mediaStore.handleMediaError({ + mediaType, + error: noResponseAxiosError, + }) + + const expectedErrorMessage = + 'Error fetching audio from API. No response received from the server' + expect(mediaStore.mediaFetchState.audio.fetchingError).toEqual( + expectedErrorMessage + ) + expect(mediaStore.mediaFetchState.audio.isFetching).toEqual(false) + expect(mediaStore.$nuxt.$sentry.captureEvent).toHaveBeenCalledWith({ + message: expectedErrorMessage, + extra: { mediaType, error: noResponseAxiosError }, + }) + }) + + it('handleMediaError re-throws a non-Axios error', async () => { + const mediaStore = useMediaStore() + const nonAxiosError = new Error('non-Axios error') - const error = new Error('Server did not respond') + const expectedErrorMessage = + 'Error fetching audio from API. Unknown error' await expect( - mediaStore.handleMediaError({ mediaType: AUDIO, error }) - ).rejects.toThrow(error.message) + mediaStore.handleMediaError({ + mediaType: AUDIO, + error: nonAxiosError, + }) + ).rejects.toThrow(expectedErrorMessage) + expect(mediaStore.mediaFetchState.audio.fetchingError).toEqual( + expectedErrorMessage + ) }) describe('setMediaProperties', () => { diff --git a/test/unit/test-utils/pinia.js b/test/unit/test-utils/pinia.js index 7a21c51d03..0f46d3abf0 100644 --- a/test/unit/test-utils/pinia.js +++ b/test/unit/test-utils/pinia.js @@ -5,6 +5,10 @@ export const createPinia = () => pinia.createPinia().use(() => ({ $nuxt: { $openverseApiToken: '', + $sentry: { + captureException: jest.fn(), + captureEvent: jest.fn(), + }, }, }))