#9198: properly serialize axios errors in report error, and skip non …

…mod logs for idb (#9201)

* properly serialize axios errors in report error, and skip non mod logs for idb

* pr feedback

* fix e2e

end-to-end-tests/tests/runtime/localIntegrations.spec.ts

@@ -74,6 +74,7 @@ test.describe("Local Integrations Page", () => {
       localIntegrationsPage.getByText("controlRoomUrl is a required field"),
     ).toBeVisible();
 
+    await localIntegrationsPage.getByLabel("Username").click();
     await expect(localIntegrationsPage.getByLabel("Username")).toBeFocused();
 
     await page.keyboard.press("Tab");

end-to-end-tests/tests/telemetry/errors.spec.ts

@@ -165,7 +165,7 @@ test("can report a service worker error to telemetry service", async ({
       }),
       extensionVersion: expect.any(String),
       manifestVersion: 3,
-      message: "Request failed with status code 500",
+      message: "Internal Server Error",
       name: "AxiosError",
       origin: "logger",
       pageName: "background",

src/components/errors/NetworkErrorDetail.tsx

@@ -23,13 +23,11 @@ import {
 } from "@fortawesome/free-solid-svg-icons";
 import JsonTree from "@/components/jsonTree/JsonTree";
 import { selectAbsoluteUrl } from "@/utils/urlUtils";
-import {
-  safeGuessStatusText,
-  type SerializableAxiosError,
-} from "@/errors/networkErrorHelpers";
+import { safeGuessStatusText } from "@/errors/networkErrorHelpers";
 import styles from "./ErrorDetail.module.scss";
 import useAsyncState from "@/hooks/useAsyncState";
 import AsyncStateGate from "@/components/AsyncStateGate";
+import { type AxiosError } from "axios";
 
 function tryParse(value: unknown): unknown {
   if (typeof value === "string") {
@@ -45,7 +43,7 @@ function tryParse(value: unknown): unknown {
 }
 
 const NetworkErrorDetail: React.FunctionComponent<{
-  error: SerializableAxiosError;
+  error: AxiosError;
 }> = ({ error }) => {
   const absoluteUrl = selectAbsoluteUrl(error.config);
 

src/data/service/errorService.ts

@@ -128,7 +128,8 @@ export function shouldIgnoreError(error: Error): boolean {
  */
 export async function reportToErrorService(
   error: Error,
-  flatContext: MessageContext,
+  flatContext: MessageContext &
+    Required<Pick<MessageContext, "modComponentId">>,
   message: string,
 ): Promise<void> {
   expectContext(
@@ -141,12 +142,6 @@ export async function reportToErrorService(
     return;
   }
 
-  if (flatContext.modComponentId == null) {
-    // Only report errors that occurred within a user-defined extension/blueprint. Other errors only go to Application error telemetry.
-    // (They're problems with our software.)
-    return;
-  }
-
   if (shouldIgnoreError(error)) {
     return;
   }

src/data/service/requestErrorUtils.ts

@@ -18,19 +18,15 @@
 import { isErrorObject } from "@/errors/errorHelpers";
 import { testMatchPatterns } from "@/bricks/available";
 import { getBaseURL } from "@/data/service/baseService";
-import {
-  isAxiosError,
-  type SerializableAxiosError,
-} from "@/errors/networkErrorHelpers";
+import { isAxiosError } from "@/errors/networkErrorHelpers";
 import { selectAbsoluteUrl, withoutTrailingSlash } from "@/utils/urlUtils";
 import { DEFAULT_SERVICE_URL } from "@/urlConstants";
+import { type AxiosError } from "axios";
 
 /**
  * Return true iff the error corresponds to a request to PixieBrix API.
  */
-export async function isAppRequestError(
-  error: SerializableAxiosError,
-): Promise<boolean> {
+export async function isAppRequestError(error: AxiosError): Promise<boolean> {
   const baseURL = await getBaseURL();
   const requestUrl = selectAbsoluteUrl(error.config);
   const patterns = [baseURL, DEFAULT_SERVICE_URL].map(
@@ -42,9 +38,7 @@ export async function isAppRequestError(
 /**
  * Return the AxiosError associated with an error, or null if error is not associated with an AxiosError
  */
-export function selectAxiosError(
-  error: unknown,
-): SerializableAxiosError | null {
+export function selectAxiosError(error: unknown): AxiosError | null {
   if (isAxiosError(error)) {
     return error;
   }

src/errors/clientRequestErrors.ts

@@ -16,7 +16,7 @@
  */
 
 import { BusinessError } from "@/errors/businessErrors";
-import { type SerializableAxiosError } from "@/errors/networkErrorHelpers";
+import { type AxiosError } from "axios";
 
 /**
  * @file ONLY KEEP ACTUAL ERRORS IN HERE.
@@ -30,9 +30,9 @@ import { type SerializableAxiosError } from "@/errors/networkErrorHelpers";
 export class ClientRequestError extends BusinessError {
   override name = "ClientRequestError";
   // Specialize the cause type
-  override readonly cause: SerializableAxiosError;
+  override readonly cause: AxiosError;
 
-  constructor(message: string, options: { cause: SerializableAxiosError }) {
+  constructor(message: string, options: { cause: AxiosError }) {
     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

src/errors/networkErrorHelpers.ts

@@ -15,20 +15,14 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { type SetRequired, type Except } from "type-fest";
+import { type SetRequired } from "type-fest";
 import { type AxiosError, type AxiosResponse } from "axios";
 import { isEmpty } from "lodash";
 import { getReasonPhrase } from "http-status-codes";
 import { isObject } from "@/utils/objectUtils";
 
-/**
- * Axios offers its own serialization method, but it doesn't include the response.
- * By deleting toJSON, the serialize-error library will use its default serialization
- */
-export type SerializableAxiosError = Except<AxiosError, "toJSON">;
-
-// Copy of axios.isAxiosError, without risking to import the whole untreeshakeable axios library
-export function isAxiosError(error: unknown): error is SerializableAxiosError {
+// Copy of axios.isAxiosError, without risking importing the whole untreeshakeable axios library
+export function isAxiosError(error: unknown): error is AxiosError {
   return (
     isObject(error) &&
     // To deal with original AxiosError as well as a serialized error

src/telemetry/logging.ts

@@ -467,6 +467,11 @@ export async function reportToApplicationErrorTelemetry(
   } satisfies RecordErrorMessage);
 }
 
+export type ContextWithMod = MessageContext &
+  Required<Pick<MessageContext, "modComponentId">>;
+const isContextWithMod = (context: MessageContext): context is ContextWithMod =>
+  context.modComponentId != null;
+
 /** @deprecated Use instead: `import reportError from "@/telemetry/reportError"` */
 export async function recordError(
   this: MessengerMeta, // Enforce usage via Messenger only
@@ -487,19 +492,27 @@ export async function recordError(
     const errorMessage = getErrorMessage(error);
     const flatContext = flattenContext(error, context);
 
+    // Only record entries that occurred within a user-defined extension/blueprint.
+    // Other errors only go to Application error telemetry. (They're problems with our software.)
+    const reportModErrorPromises = isContextWithMod(flatContext)
+      ? [
+          reportToErrorService(error, flatContext, errorMessage),
+          appendEntry({
+            uuid: uuidv4(),
+            timestamp: Date.now().toString(),
+            level: "error",
+            context: flatContext,
+            message: errorMessage,
+            data,
+            // Ensure the object is fully serialized. Required because it will be stored in IDB and flow through the Redux state
+            error: serializedError,
+          }),
+        ]
+      : [];
+
     await Promise.all([
       reportToApplicationErrorTelemetry(error, flatContext, errorMessage),
-      reportToErrorService(error, flatContext, errorMessage),
-      appendEntry({
-        uuid: uuidv4(),
-        timestamp: Date.now().toString(),
-        level: "error",
-        context: flatContext,
-        message: errorMessage,
-        data,
-        // Ensure the object is fully serialized. Required because it will be stored in IDB and flow through the Redux state
-        error: serializedError,
-      }),
+      ...reportModErrorPromises,
     ]);
   } catch (recordErrorError) {
     console.error("An error occurred while recording another error", {
@@ -573,7 +586,7 @@ export async function clearModComponentDebugLogs(
       }
     }
   }, IDB_OPERATION.CLEAR_MOD_COMPONENT_DEBUG_LOGS).catch((_error) => {
-    // Swallow error because we've reported it to application error telemetry and
+    // Swallow error because we've reported it to application error telemetry, and
     // we don't want to interrupt the execution of mod pipeline
   });
 }

src/telemetry/reportError.test.ts

@@ -16,6 +16,7 @@
  */
 
 import { getNotifier } from "webext-messenger";
+import { AxiosError } from "axios";
 
 jest.mock("webext-messenger");
 
@@ -80,6 +81,28 @@ describe("reportError", () => {
     );
   });
 
+  it("should serialize AxiosErrors correctly", () => {
+    const error = new AxiosError("Test error", "code", {
+      validateStatus: () => true,
+    });
+    reportError(error);
+    expect(_recordMock).toHaveBeenCalledWith(
+      {
+        message: "Test error",
+        code: "code",
+        name: "AxiosError",
+        config: {}, // Expect any functions to be stripped
+        stack: expect.any(String),
+      },
+      {
+        connectionType: "unknown",
+        pageName: "extension",
+        referrer: "",
+        url: "http://localhost/",
+      },
+    );
+  });
+
   it("should log error to console if an error occurs while reporting the error", () => {
     const error = new Error("Test error");
     const reportingError = new Error("Reporting error");

src/telemetry/reportError.ts

@@ -21,6 +21,7 @@ import { serializeError } from "serialize-error";
 import { selectError, shouldErrorBeIgnored } from "@/errors/errorHelpers";
 import { expectContext } from "@/utils/expectContext";
 import { getContextName } from "webext-detect";
+import { isAxiosError } from "@/errors/networkErrorHelpers";
 
 expectContext(
   "extension",
@@ -94,10 +95,18 @@ export default function reportError(
   }
 
   try {
-    _record(serializeError(selectError(errorLike)), {
-      ...context,
-      ...getReportErrorAdditionalContext(),
-    });
+    _record(
+      serializeError(selectError(errorLike), {
+        // AxiosError toJSON leaves functions intact which are not serializable, so we specifically
+        // disable useToJSON for AxiosErrors. See: https://github.com/pixiebrix/pixiebrix-extension/issues/9198
+        // We cannot just delete toJSON from the error since it is a prototype method and would be inherited
+        useToJSON: !isAxiosError(errorLike),
+      }),
+      {
+        ...context,
+        ...getReportErrorAdditionalContext(),
+      },
+    );
   } catch (reportingError) {
     // The messenger does not throw async errors on "notifiers" but if this is
     // called in the background the call will be executed directly, and it could

src/utils/enrichAxiosErrors.ts

@@ -63,7 +63,6 @@ async function enrichBusinessRequestError(error: unknown): Promise<never> {
       !url.pathname.startsWith(API_PATHS.PROXY)
     ) {
       // TODO: Maybe handle app errors here too, like we do in `selectServerErrorMessage`
-      // eslint-disable-next-line @typescript-eslint/no-throw-literal -- Duck-typed Error, still works
       throw error;
     }