From ee3caaa1e96b227029a19d40e1d33e81b4c824ed Mon Sep 17 00:00:00 2001 From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Date: Fri, 22 Mar 2024 10:21:22 +1100 Subject: [PATCH] Centralise frontend error reporting (and suppress unactionable Sentry errors) (#3850) * Centralise frontend network error reporting * Add shim for tests * Revert unnecessary change to `useSearch` * Remove unnecessary errors plugin shim causing jest.mock to fail for feature flag json See note regarding setup-after-env in this section of Jest documentation: https://jestjs.io/docs/jest-object\#jestmockmodulename-factory-options * Fix jest types to match actual jest version * Add unit tests for errors plugin * Skip 429s and rearrange function for clarity * WIP/ PR does not work due to analytics composable * Consider SSR/CSR in test * Remove stray console.error * Add search route network error handling test * Fix bad merge of pnpm lock --- frontend/nuxt.config.ts | 1 + .../VHeader/VHeaderMobile/VHeaderMobile.vue | 2 +- frontend/src/composables/use-search.ts | 4 +- frontend/src/plugins/errors.ts | 147 ++++++++++++++ frontend/src/stores/media/index.ts | 16 +- frontend/src/stores/media/related-media.ts | 12 +- frontend/src/stores/media/single-result.ts | 12 +- frontend/src/stores/provider.ts | 8 +- frontend/src/types/analytics.ts | 17 ++ frontend/src/utils/errors.ts | 66 +----- frontend/test/playwright/e2e/search.spec.ts | 99 +++++++++ frontend/test/playwright/playwright.config.ts | 7 +- .../test/unit/specs/plugins/errors.spec.ts | 191 ++++++++++++++++++ frontend/test/unit/test-utils/pinia.js | 3 + pnpm-lock.yaml | 8 +- 15 files changed, 500 insertions(+), 93 deletions(-) create mode 100644 frontend/src/plugins/errors.ts create mode 100644 frontend/test/unit/specs/plugins/errors.spec.ts diff --git a/frontend/nuxt.config.ts b/frontend/nuxt.config.ts index 92298bef1a7..a010c68a902 100644 --- a/frontend/nuxt.config.ts +++ b/frontend/nuxt.config.ts @@ -147,6 +147,7 @@ const config: NuxtConfig = { "~/plugins/polyfills.client.ts", "~/plugins/sentry.ts", "~/plugins/analytics.ts", + "~/plugins/errors.ts", ], css: ["~/assets/fonts.css", "~/styles/tailwind.css", "~/styles/accent.css"], head, diff --git a/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue b/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue index a6d9bedb62d..a0e826a475d 100644 --- a/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue +++ b/frontend/src/components/VHeader/VHeaderMobile/VHeaderMobile.vue @@ -119,7 +119,6 @@ import { cyclicShift } from "~/utils/math" import { keycodes } from "~/constants/key-codes" -import { useAnalytics } from "~/composables/use-analytics" import { useDialogControl } from "~/composables/use-dialog-control" import { useSearch } from "~/composables/use-search" @@ -127,6 +126,7 @@ import { useMediaStore } from "~/stores/media" import { useSearchStore } from "~/stores/search" import { useHydrating } from "~/composables/use-hydrating" +import { useAnalytics } from "~/composables/use-analytics" import VLogoButton from "~/components/VHeader/VLogoButton.vue" import VInputModal from "~/components/VModal/VInputModal.vue" diff --git a/frontend/src/composables/use-search.ts b/frontend/src/composables/use-search.ts index 2c8c27aa431..e9f90ca4d11 100644 --- a/frontend/src/composables/use-search.ts +++ b/frontend/src/composables/use-search.ts @@ -6,10 +6,10 @@ import { useMediaStore } from "~/stores/media" import { useI18nResultsCount } from "~/composables/use-i18n-utilities" import { useMatchSearchRoutes } from "~/composables/use-match-routes" -import type { EventName, Events } from "~/types/analytics" +import { useAnalytics } from "~/composables/use-analytics" export const useSearch = ( - sendCustomEvent: (name: T, payload: Events[T]) => void + sendCustomEvent: ReturnType["sendCustomEvent"] ) => { const mediaStore = useMediaStore() const searchStore = useSearchStore() diff --git a/frontend/src/plugins/errors.ts b/frontend/src/plugins/errors.ts new file mode 100644 index 00000000000..5be8fab573d --- /dev/null +++ b/frontend/src/plugins/errors.ts @@ -0,0 +1,147 @@ +import axios from "axios" +import { Plugin, Context } from "@nuxt/types" + +import { ERR_UNKNOWN, ErrorCode, errorCodes } from "~/constants/errors" +import type { FetchingError, RequestKind } from "~/types/fetch-state" +import type { SupportedSearchType } from "~/constants/media" + +const isValidErrorCode = ( + code: string | undefined | null +): code is ErrorCode => { + if (!code) { + return false + } + return (errorCodes as readonly string[]).includes(code) +} + +function isDetailedResponseData(data: unknown): data is { detail: string } { + return !!data && typeof data === "object" && "detail" in data +} + +/** + * Normalize any error occurring during a network call. + * + * @param error - Any error arising during a network call + * @param searchType - The type of search selected when the error occurred + * @param requestKind - The kind of request the error occurred for + * @param details - Any additional details to attach to the error + * @returns Normalized error object + */ +export function normalizeFetchingError( + error: unknown, + searchType: SupportedSearchType, + requestKind: RequestKind, + details?: Record +): FetchingError { + const fetchingError: FetchingError = { + requestKind, + details, + searchType, + code: ERR_UNKNOWN, + } + + if (!axios.isAxiosError(error)) { + fetchingError.message = (error as Error).message + return fetchingError + } + + // Otherwise, it's an AxiosError + if (isValidErrorCode(error.code)) { + fetchingError.code = error.code + } + + if (error.response?.status) { + fetchingError.statusCode = error.response.status + } + + const responseData = error?.response?.data + + // Use the message returned by the API. + if (isDetailedResponseData(responseData)) { + fetchingError.message = responseData.detail as string + } else { + fetchingError.message = error.message + } + + return fetchingError +} + +/** + * Record network errors using the appropriate tool, as needed, + * based on response code, status, and request kind. + * @param error - The error to record + */ +function recordError( + context: Context, + originalError: unknown, + fetchingError: FetchingError +) { + if (fetchingError.statusCode === 429) { + // These are more readily monitored via the Cloudflare dashboard. + return + } + + if ( + fetchingError.requestKind === "single-result" && + fetchingError.statusCode === 404 + ) { + /** + * Do not record 404s for single result requests because: + * 1. Plausible will already record them as resulting in a 404 page view + * 2. The Openverse API 404s on malformed identifiers, so there is no way + * to distinguish between truly not found works and bad requests from + * the client side. + * 3. There isn't much we can do other than monitor for an anomalously high + * number of 404 responses from the frontend server that could indicate a frontend + * implementation or configuration error suddenly causing malformed + * identifiers to be used. Neither Sentry nor Plausible are the right tool + * for that task. If the 404s are caused by an API issue, we'd see that in + * API response code monitoring, where we can more easily trace the cause + */ + return + } + + if (process.client && fetchingError.code === "ERR_NETWORK") { + /** + * Record network errors in Plausible so that we can evaluate potential + * regional or device configuration issues, for which Sentry is not + * as good a tool. Additionally, the number of these events are trivial + * for Plausible, but do actually affect our Sentry quota enough that it + * is worth diverting them. + */ + context.$sendCustomEvent("NETWORK_ERROR", { + requestKind: fetchingError.requestKind, + searchType: fetchingError.searchType, + }) + } else { + context.$sentry.captureException(originalError, { + extra: { fetchingError }, + }) + } +} + +function createProcessFetchingError( + context: Context +): typeof normalizeFetchingError { + function processFetchingError( + ...[originalError, ...args]: Parameters + ) { + const fetchingError = normalizeFetchingError(originalError, ...args) + recordError(context, originalError, fetchingError) + return fetchingError + } + + return processFetchingError +} + +declare module "@nuxt/types" { + interface Context { + $processFetchingError: ReturnType + } +} + +const plugin: Plugin = async (context, inject) => { + inject("processFetchingError", createProcessFetchingError(context)) +} + +export default plugin diff --git a/frontend/src/stores/media/index.ts b/frontend/src/stores/media/index.ts index 5c35ee48e6a..2da62aad110 100644 --- a/frontend/src/stores/media/index.ts +++ b/frontend/src/stores/media/index.ts @@ -2,7 +2,7 @@ import { defineStore } from "pinia" import { warn } from "~/utils/console" import { hash, rand as prng } from "~/utils/prng" -import { isRetriable, parseFetchingError } from "~/utils/errors" +import { isRetriable } from "~/utils/errors" import type { AudioDetail, DetailFromMediaType, @@ -499,15 +499,17 @@ export const useMediaStore = defineStore("media", { }) return mediaCount } catch (error: unknown) { - const errorData = parseFetchingError(error, mediaType, "search", { - searchTerm: queryParams.q ?? "", - }) + const errorData = this.$nuxt.$processFetchingError( + error, + mediaType, + "search", + { + searchTerm: queryParams.q ?? "", + } + ) this._updateFetchState(mediaType, "end", errorData) - this.$nuxt.$sentry.captureException(error, { - extra: { errorData }, - }) return null } }, diff --git a/frontend/src/stores/media/related-media.ts b/frontend/src/stores/media/related-media.ts index eed57e08e7f..ec3964fa417 100644 --- a/frontend/src/stores/media/related-media.ts +++ b/frontend/src/stores/media/related-media.ts @@ -1,6 +1,5 @@ import { defineStore } from "pinia" -import { parseFetchingError } from "~/utils/errors" import { initServices } from "~/stores/media/services" import type { FetchingError, FetchState } from "~/types/fetch-state" @@ -59,9 +58,14 @@ export const useRelatedMediaStore = defineStore("related-media", { return this.media.length } catch (error) { - const errorData = parseFetchingError(error, mediaType, "related", { - id, - }) + const errorData = this.$nuxt.$processFetchingError( + error, + mediaType, + "related", + { + id, + } + ) this._endFetching(errorData) this.$nuxt.$sentry.captureException(error, { extra: { errorData } }) diff --git a/frontend/src/stores/media/single-result.ts b/frontend/src/stores/media/single-result.ts index 0a85aa5cc4d..67aa181a989 100644 --- a/frontend/src/stores/media/single-result.ts +++ b/frontend/src/stores/media/single-result.ts @@ -12,7 +12,6 @@ import type { SupportedMediaType } from "~/constants/media" import { initServices } from "~/stores/media/services" import { useMediaStore } from "~/stores/media/index" import { useProviderStore } from "~/stores/provider" -import { parseFetchingError } from "~/utils/errors" import type { FetchingError, FetchState } from "~/types/fetch-state" @@ -168,11 +167,14 @@ export const useSingleResultStore = defineStore("single-result", { return item as DetailFromMediaType } catch (error) { - const errorData = parseFetchingError(error, type, "single-result", { - id, - }) + const errorData = this.$nuxt.$processFetchingError( + error, + type, + "single-result", + { id } + ) this._updateFetchState("end", errorData) - this.$nuxt.$sentry.captureException(error, { extra: { errorData } }) + return null } }, diff --git a/frontend/src/stores/provider.ts b/frontend/src/stores/provider.ts index f077c45c380..fad0221f39b 100644 --- a/frontend/src/stores/provider.ts +++ b/frontend/src/stores/provider.ts @@ -3,7 +3,6 @@ import { ssrRef } from "@nuxtjs/composition-api" import { capitalCase } from "~/utils/case" import { env } from "~/utils/env" -import { parseFetchingError } from "~/utils/errors" import { AUDIO, IMAGE, @@ -153,12 +152,15 @@ export const useProviderStore = defineStore("provider", { sortedProviders = sortProviders(res) this._updateFetchState(mediaType, "end") } catch (error: unknown) { - const errorData = parseFetchingError(error, mediaType, "provider") + const errorData = this.$nuxt.$processFetchingError( + error, + mediaType, + "provider" + ) // Fallback on existing providers if there was an error sortedProviders = this.providers[mediaType] this._updateFetchState(mediaType, "end", errorData) - this.$nuxt.$sentry.captureException(error, { extra: { errorData } }) } finally { this.providers[mediaType] = sortedProviders this.sourceNames[mediaType] = sortedProviders.map((p) => p.source_name) diff --git a/frontend/src/types/analytics.ts b/frontend/src/types/analytics.ts index eb2ba2f3db2..88f15dbe09d 100644 --- a/frontend/src/types/analytics.ts +++ b/frontend/src/types/analytics.ts @@ -7,6 +7,8 @@ import type { ReportReason } from "~/constants/content-report" import type { FilterCategory } from "~/constants/filters" import { ResultKind } from "~/types/result" +import { RequestKind } from "./fetch-state" + export type AudioInteraction = "play" | "pause" | "seek" export type AudioInteractionData = Exclude< Events["AUDIO_INTERACTION"], @@ -352,6 +354,7 @@ export type Events = { /** whether the switch was turned on or off */ checked: boolean } + /** * Description: The user flips the sidebar toggle to not blur sensitive * content in the search results. @@ -364,6 +367,7 @@ export type Events = { /** whether the switch was turned on or off */ checked: boolean } + /** * Description: The user proceeds to see the sensitive content from the * content safety wall. @@ -379,6 +383,7 @@ export type Events = { /** the reasons for why this result is considered sensitive */ sensitivities: string } + /** * Description: The user opts not to see the sensitive content and to go back * to the search results from the content safety wall. @@ -397,6 +402,7 @@ export type Events = { /** the reasons for why this result is considered sensitive */ sensitivities: string } + /** * Description: The user opts to re-hide the sensitive content that has been * unblurred and presented to them. @@ -423,6 +429,17 @@ export type Events = { /** The state of the tags after the user interaction. */ toState: "expanded" | "collapsed" } + /** + * Description: Recorded when a network error occurs. Recorded in Plausible, + * rather than Sentry, because we never have sufficient information in Sentry + * to identify patterns that could be relevant, like regional issues. + */ + NETWORK_ERROR: { + /** The kind of request the network error occurred during */ + requestKind: RequestKind + /** The search type when the network error occurred */ + searchType: SupportedSearchType + } } /** diff --git a/frontend/src/utils/errors.ts b/frontend/src/utils/errors.ts index 0d4c3fe5f9e..7ff9d8cee83 100644 --- a/frontend/src/utils/errors.ts +++ b/frontend/src/utils/errors.ts @@ -1,58 +1,5 @@ -import axios from "axios" - -import { - clientSideErrorCodes, - ERR_UNKNOWN, - ErrorCode, - errorCodes, - NO_RESULT, -} from "~/constants/errors" -import type { FetchingError, RequestKind } from "~/types/fetch-state" -import type { SupportedSearchType } from "~/constants/media" - -/** - * Parses an error object to standardize error-related details for the frontend. - * @param error - the error object to parse. - * @param searchType - the type of media that request was made for (can be `all content`). - * @param requestKind - the kind of request that was made. - * @param details - additional data to display on the error page, e.g. searchTerm. - */ -export const parseFetchingError = ( - error: unknown, - searchType: SupportedSearchType, - requestKind: RequestKind, - details?: Record -) => { - const fetchingError: FetchingError = { - requestKind, - details, - searchType, - code: ERR_UNKNOWN, - } - - if (axios.isAxiosError(error)) { - if (isValidErrorCode(error.code)) { - fetchingError.code = error.code - } - if (error.response?.status) { - fetchingError.statusCode = error.response.status - } - const responseData = error?.response?.data - // Use the message returned by the API. - if ( - typeof responseData === "object" && - responseData !== null && - "detail" in responseData - ) { - fetchingError.message = responseData?.detail as string - } else { - fetchingError.message = error.message - } - } else { - fetchingError.message = (error as Error).message - } - return fetchingError -} +import { clientSideErrorCodes, NO_RESULT } from "~/constants/errors" +import type { FetchingError } from "~/types/fetch-state" const NON_RETRYABLE_ERROR_CODES = [429, 500, 404] as const const isNonRetryableErrorStatusCode = (statusCode: number | undefined) => { @@ -62,15 +9,6 @@ const isNonRetryableErrorStatusCode = (statusCode: number | undefined) => { ) } -const isValidErrorCode = ( - code: string | undefined | null -): code is ErrorCode => { - if (!code) { - return false - } - return (errorCodes as readonly string[]).includes(code) -} - /** * Returns true if the request should be retried if error occurred on * the server. For 429, 500 or 404 errors, or for NO_RESULT error, diff --git a/frontend/test/playwright/e2e/search.spec.ts b/frontend/test/playwright/e2e/search.spec.ts index ea0ee31bfe2..c99b3cd2efb 100644 --- a/frontend/test/playwright/e2e/search.spec.ts +++ b/frontend/test/playwright/e2e/search.spec.ts @@ -8,6 +8,11 @@ */ import { expect, test } from "@playwright/test" +import { + collectAnalyticsEvents, + expectEventPayloadToMatch, + EventResponse, +} from "~~/test/playwright/utils/analytics" import { mockProviderApis } from "~~/test/playwright/utils/route" import { goToSearchTerm, @@ -40,3 +45,97 @@ test("scroll to top on new search term submitted", async ({ page }) => { expect(scrollY).toBe(0) }) + +test("Send network errors to Plausible", async ({ context, page }) => { + const analyticsEvents = collectAnalyticsEvents(context) + await context.route( + (url) => { + // Only match the search requests, rather than any other API request the search page makes + return Boolean( + url.pathname.match(/v1\/(audio|images)/) && + url.searchParams.has("q", "galah") + ) + }, + async (route) => route.abort("connectionaborted") + ) + + await goToSearchTerm(page, "galah", { mode: "CSR" }) + + const searchErrorEvents = analyticsEvents.filter( + (event) => + event.n === "NETWORK_ERROR" && + "requestKind" in event.p && + event.p.requestKind === "search" + ) as EventResponse<"NETWORK_ERROR">[] + expect(searchErrorEvents).toHaveLength(2) + expectEventPayloadToMatch( + searchErrorEvents.find((e) => e.p.searchType == "image"), + { requestKind: "search", searchType: "image" } + ) + expectEventPayloadToMatch( + searchErrorEvents.find((e) => e.p.searchType == "audio"), + { requestKind: "search", searchType: "audio" } + ) +}) + +test("Do not send network errors to Plausible when SSR", async ({ + context, + page, +}) => { + // Plausible not supported server-side, so skip sending the event + const analyticsEvents = collectAnalyticsEvents(context) + await context.route( + (url) => { + // Only match the search requests, rather than any other API request the search page makes + return Boolean( + url.pathname.match(/v1\/(audio|images)/) && + url.searchParams.has("q", "galah") + ) + }, + async (route) => route.abort("connectionaborted") + ) + + await goToSearchTerm(page, "galah", { mode: "SSR" }) + + const searchErrorEvents = analyticsEvents.filter( + (event) => + event.n === "NETWORK_ERROR" && + "requestKind" in event.p && + event.p.requestKind === "search" + ) as EventResponse<"NETWORK_ERROR">[] + expect(searchErrorEvents).toHaveLength(0) +}) + +for (const renderMode of ["CSR", "SSR"] as const) { + test(`Do not send non-network errors to Plausible - ${renderMode}`, async ({ + context, + page, + }) => { + // This tests a _negative_, which isn't ideal, but seeing as we have tests for the error handling plugin, and the previous test, + // `send errors to Plausible`, proves the plugin is being used (ignoring the possibility of conditionally calling the error + // handling function), so there shouldn't be any network error events. Everything would get sent to Sentry, but we don't have a + // way to assert Sentry requests in Playwright, so this approach will have to do. + const analyticsEvents = collectAnalyticsEvents(context) + await context.route( + (url) => { + // Only match the search requests, rather than any other API request the search page makes + return Boolean( + url.pathname.match(/v1\/(audio|images)/) && + url.searchParams.has("q", "galah") + ) + }, + async (route) => { + await route.fulfill({ + status: 500, + }) + } + ) + + await goToSearchTerm(page, "galah", { mode: renderMode }) + + const searchErrorEvents = analyticsEvents.filter( + (event) => event.n === "NETWORK_ERROR" + ) as EventResponse<"NETWORK_ERROR">[] + expect(searchErrorEvents).toHaveLength(0) + }) +} diff --git a/frontend/test/playwright/playwright.config.ts b/frontend/test/playwright/playwright.config.ts index 2541fd19a29..ca866368f44 100644 --- a/frontend/test/playwright/playwright.config.ts +++ b/frontend/test/playwright/playwright.config.ts @@ -2,6 +2,8 @@ import type { PlaywrightTestConfig } from "@playwright/test" const UPDATE_TAPES = process.env.UPDATE_TAPES || "false" +export const API_URL = "http://localhost:49153/" + /** * Enabling `FASTSTART` allows you to bypass running the nuxt build * when rapidly debugging tests locally within the container. If you @@ -13,8 +15,7 @@ const UPDATE_TAPES = process.env.UPDATE_TAPES || "false" * Note that visual-regression tests can be quite flaky when using * `FASTSTART`. */ -const pwCommand = - process.env.FASTSTART !== "false" ? "start:playwright" : "prod:playwright" +const pwCommand = process.env.FASTSTART !== "false" ? "dev" : "prod:playwright" const config: PlaywrightTestConfig = { forbidOnly: !!process.env.CI, @@ -24,7 +25,7 @@ const config: PlaywrightTestConfig = { port: 8443, reuseExistingServer: !process.env.CI || process.env.PWDEBUG === "1", env: { - API_URL: "http://localhost:49153/", + API_URL, UPDATE_TAPES: UPDATE_TAPES, PW: "true", }, diff --git a/frontend/test/unit/specs/plugins/errors.spec.ts b/frontend/test/unit/specs/plugins/errors.spec.ts new file mode 100644 index 00000000000..fb7397f3b4e --- /dev/null +++ b/frontend/test/unit/specs/plugins/errors.spec.ts @@ -0,0 +1,191 @@ +import { Context } from "@nuxt/types" +import { AxiosError, AxiosHeaders } from "axios" + +import type { RequestKind } from "~/types/fetch-state" + +import errorsPlugin from "~/plugins/errors" + +import { SupportedSearchType, supportedSearchTypes } from "~/constants/media" + +const getNotFoundError = () => + new AxiosError( + "Beep boop, something went wrong :(", + AxiosError.ERR_BAD_REQUEST, + undefined, + undefined, + { + statusText: "", + data: "", + headers: {}, + config: { headers: new AxiosHeaders() }, + status: 404, + } + ) + +const getNetworkError = () => + new AxiosError( + "Wowee, this is really bad!", + AxiosError.ERR_NETWORK, + undefined, + undefined, + undefined + ) + +describe("Errors plugin", () => { + const mockContext = { + $sendCustomEvent: jest.fn(), + $sentry: { + captureException: jest.fn(), + }, + } as unknown as Context + const mockInject = jest.fn() + + const getPluginInstance = () => + mockInject.mock.calls[0][1] as Context["$processFetchingError"] + + beforeEach(() => { + jest.resetAllMocks() + }) + + it("should inject the processFetchingError function", async () => { + await errorsPlugin(mockContext, mockInject) + + expect(mockInject).toHaveBeenCalledWith( + "processFetchingError", + expect.any(Function) + ) + }) + + it("should ignore 404s for single result requests", async () => { + await errorsPlugin(mockContext, mockInject) + + const plugin = getPluginInstance() + + const error = getNotFoundError() + const fetchingError = plugin(error, "all", "single-result", {}) + + expect(fetchingError).toMatchObject({ + message: error.message, + code: error.code, + statusCode: error.response!.status, + }) + + expect(mockContext.$sentry.captureException).not.toHaveBeenCalled() + expect(mockContext.$sendCustomEvent).not.toHaveBeenCalled() + }) + + it.each(["provider", "related", "search"] as RequestKind[])( + "should not ignore 404s for other request types", + async (requestKind) => { + await errorsPlugin(mockContext, mockInject) + + const plugin = getPluginInstance() + + const error = getNotFoundError() + + const fetchingError = plugin(error, "all", requestKind, {}) + + expect(fetchingError).toMatchObject({ + message: error.message, + code: error.code, + statusCode: error.response!.status, + }) + + expect(mockContext.$sentry.captureException).toHaveBeenCalledWith(error, { + extra: { fetchingError }, + }) + expect(mockContext.$sendCustomEvent).not.toHaveBeenCalled() + } + ) + + const requestKinds = [ + "provider", + "related", + "single-result", + "search", + ] as RequestKind[] + const combinations = requestKinds.reduce( + (acc, requestKind) => [ + ...acc, + ...supportedSearchTypes.map( + (searchType) => + [requestKind, searchType] as [RequestKind, SupportedSearchType] + ), + ], + [] as [RequestKind, SupportedSearchType][] + ) + + const processClientForBlock = (value: boolean) => { + let originalValue: boolean + + beforeAll(() => { + originalValue = process.client + process.client = value + }) + + afterAll(() => { + process.client = originalValue + }) + } + + describe("client-side network errors", () => { + processClientForBlock(true) + + it.each(combinations)( + "should send %s %s client-side network errors to plausible instead of sentry", + async (requestKind, searchType) => { + await errorsPlugin(mockContext, mockInject) + + const plugin = getPluginInstance() + + const error = getNetworkError() + + const fetchingError = plugin(error, searchType, requestKind, {}) + + expect(fetchingError).toMatchObject({ + message: error.message, + code: error.code, + }) + + expect(mockContext.$sentry.captureException).not.toHaveBeenCalled() + expect(mockContext.$sendCustomEvent).toHaveBeenCalledWith( + "NETWORK_ERROR", + { + requestKind, + searchType, + } + ) + } + ) + }) + + describe("server-side network errors", () => { + processClientForBlock(false) + + it.each(combinations)( + "should send %s %s client-side network errors to plausible instead of sentry", + async (requestKind, searchType) => { + await errorsPlugin(mockContext, mockInject) + + const plugin = getPluginInstance() + + const error = getNetworkError() + + const fetchingError = plugin(error, searchType, requestKind, {}) + + expect(fetchingError).toMatchObject({ + message: error.message, + code: error.code, + }) + + expect(mockContext.$sendCustomEvent).not.toHaveBeenCalled() + expect(mockContext.$sentry.captureException).toHaveBeenCalledWith( + error, + { + extra: { fetchingError }, + } + ) + } + ) + }) +}) diff --git a/frontend/test/unit/test-utils/pinia.js b/frontend/test/unit/test-utils/pinia.js index 0a50d9fc829..08aeab4157c 100644 --- a/frontend/test/unit/test-utils/pinia.js +++ b/frontend/test/unit/test-utils/pinia.js @@ -1,10 +1,13 @@ // eslint-disable-next-line no-restricted-imports import * as pinia from "pinia" +import { normalizeFetchingError } from "~/plugins/errors" + export const createPinia = () => pinia.createPinia().use(() => ({ $nuxt: { $openverseApiToken: "", + $processFetchingError: jest.fn(normalizeFetchingError), $sentry: { captureException: jest.fn(), captureEvent: jest.fn(), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5a11b24cbfc..8aed9e21725 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -213,7 +213,7 @@ importers: version: 1.0.2 '@types/jest': specifier: ^29.5.4 - version: 29.5.4 + version: 29.5.12 '@types/node': specifier: 18.19.21 version: 18.19.21 @@ -7466,8 +7466,8 @@ packages: dependencies: '@types/istanbul-lib-report': 3.0.0 - /@types/jest@29.5.4: - resolution: {integrity: sha512-PhglGmhWeD46FYOVLt3X7TiWjzwuVGW9wG/4qocPevXMjCmrIc5b6db9WjeGE4QYVpUAWMDv3v0IiBwObY289A==} + /@types/jest@29.5.12: + resolution: {integrity: sha512-eDC8bTvT/QhYdxJAulQikueigY5AsdBRH2yDKW3yveW7svY3+DzN84/2NUgkw10RTiJbWqZrTtoGVdYlvFJdLw==} dependencies: expect: 29.6.4 pretty-format: 29.6.3 @@ -7636,7 +7636,7 @@ packages: /@types/testing-library__jest-dom@5.14.2: resolution: {integrity: sha512-vehbtyHUShPxIa9SioxDwCvgxukDMH//icJG90sXQBUm5lJOHLT5kNeU9tnivhnA/TkOFMzGIXN2cTc4hY8/kg==} dependencies: - '@types/jest': 29.5.4 + '@types/jest': 29.5.12 dev: true /@types/throttle-debounce@5.0.0: