diff --git a/src/background/requests.ts b/src/background/requests.ts index 2209f33e40..dcddf6236a 100644 --- a/src/background/requests.ts +++ b/src/background/requests.ts @@ -72,7 +72,7 @@ export async function serializableAxiosRequest( const { data, status, statusText } = await axios(config); - // Firefox won't send response objects from the background page to the content script. So strip out the + // Firefox won't send response objects from the background page to the content script. Strip out the // potentially sensitive parts of the response (the request, headers, etc.) return JSON.parse(JSON.stringify({ data, status, statusText })); } @@ -271,7 +271,15 @@ export async function proxyService( ); } - return serializableAxiosRequest(requestConfig); + try { + return await serializableAxiosRequest(requestConfig); + } catch (error) { + console.debug("Error making request without service config", { + requestConfig, + error, + }); + throw error; + } } try { diff --git a/src/components/errors/NetworkErrorDetail.tsx b/src/components/errors/NetworkErrorDetail.tsx index 938ae6622b..5477d0fec7 100644 --- a/src/components/errors/NetworkErrorDetail.tsx +++ b/src/components/errors/NetworkErrorDetail.tsx @@ -72,7 +72,7 @@ const NetworkErrorDetail: React.FunctionComponent<{ const cleanResponse = useMemo(() => { if (error.response) { const { request, config, data, ...rest } = error.response; - // Don't include request, since we're showing it the other column + // Don't include request or config, since we're showing it the other column return { ...rest, data: tryParse(data), diff --git a/src/development/errorsBadge.ts b/src/development/errorsBadge.ts index d247e8fd54..fd9655892a 100644 --- a/src/development/errorsBadge.ts +++ b/src/development/errorsBadge.ts @@ -23,7 +23,7 @@ let timer: NodeJS.Timeout; function updateBadge(errorMessage: string | null): void { void chrome.browserAction.setTitle({ - title: errorMessage, + title: errorMessage ?? "Unknown error (no error message provided)", }); void chrome.browserAction.setBadgeText({ text: counter ? String(counter) : undefined, diff --git a/src/errors/businessErrors.test.ts b/src/errors/businessErrors.test.ts new file mode 100644 index 0000000000..2c91a5ea9e --- /dev/null +++ b/src/errors/businessErrors.test.ts @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2022 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { BusinessError } from "@/errors/businessErrors"; +import { serializeError } from "serialize-error"; + +describe("BusinessError", () => { + it("records cause", () => { + const cause = new Error("Pylon Error"); + const error = new BusinessError("You Must Construct Additional Pylons", { + cause, + }); + expect(serializeError(error).cause).toStrictEqual(serializeError(cause)); + }); +}); diff --git a/src/errors/clientRequestErrors.test.ts b/src/errors/clientRequestErrors.test.ts new file mode 100644 index 0000000000..6424ab1b23 --- /dev/null +++ b/src/errors/clientRequestErrors.test.ts @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2022 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { serializeError } from "serialize-error"; +import { + ClientRequestError, + RemoteServiceError, +} from "@/errors/clientRequestErrors"; +import axios, { AxiosError } from "axios"; +import MockAdapter from "axios-mock-adapter"; + +const axiosMock = new MockAdapter(axios); + +afterEach(() => { + axiosMock.reset(); + axiosMock.resetHistory(); +}); + +describe("RemoteServiceError", () => { + it("records cause", () => { + const cause = new AxiosError("The tubes are clogged"); + // `serializeError` uses the name/message/stack property to determine if it's errorLike + // In practice, AxiosErrors actually have stacks: https://github.com/axios/axios/issues/2387 + cause.stack = ""; + const error = new RemoteServiceError( + "You Must Construct Additional Pylons", + { cause } + ); + expect(serializeError(error, { useToJSON: false }).cause).toStrictEqual( + serializeError(cause, { useToJSON: false }) + ); + }); + + it("keeps request and response", async () => { + axiosMock + .onAny() + .reply(400, { detail: "I'm sorry Dave, I'm afraid I can't do that" }); + + let cause: AxiosError; + + try { + await axios({ + url: "https://httpstat.us/400", + method: "get", + }); + } catch (error: unknown) { + cause = error as AxiosError; + } + + const error = new RemoteServiceError( + "You Must Construct Additional Pylons", + { cause } + ); + + const serialized = serializeError(error, { useToJSON: false }); + + expect((serialized.cause as AxiosError).config).toBeTruthy(); + expect((serialized.cause as AxiosError).response).toBeTruthy(); + }); +}); + +describe("ClientRequestError", () => { + it("records cause", () => { + const cause = new AxiosError("The tubes are clogged"); + cause.stack = ""; + // `serializeError` uses the name/message/stack property to determine if it's errorLike + // In practice, AxiosErrors actually have stacks: https://github.com/axios/axios/issues/2387 + const error = new ClientRequestError( + "You Must Construct Additional Pylons", + { cause } + ); + expect(serializeError(error, { useToJSON: false }).cause).toStrictEqual( + serializeError(cause, { useToJSON: false }) + ); + }); +}); diff --git a/src/errors/clientRequestErrors.ts b/src/errors/clientRequestErrors.ts index 6c5d1c2297..839a469944 100644 --- a/src/errors/clientRequestErrors.ts +++ b/src/errors/clientRequestErrors.ts @@ -29,11 +29,14 @@ import { SerializableAxiosError } from "@/errors/networkErrorHelpers"; */ export class ClientRequestError extends BusinessError { override name = "ClientRequestError"; + // Specialize the cause type override readonly cause: SerializableAxiosError; - // eslint-disable-next-line @typescript-eslint/no-useless-constructor -- Required to make the types stricter constructor(message: string, options: { cause: SerializableAxiosError }) { super(message, options); + // This assignment seems to be required in Chrome 102 to ensure the cause is serialized by serialize-error + // https://github.com/pixiebrix/pixiebrix-extension/issues/3613 + this.cause = options.cause; } }