refactor(fx-api): keep errorClass on logs only

Sentry already records the exception type, so the tag was
redundant.

Author: GregHilstonMozilla

src/app/api/utils/withFxaUser.test.ts

@@ -219,19 +219,9 @@ describe("withFxaUser", () => {
             outcome: "introspection-error",
             endpoint: ENDPOINT,
             httpStatus,
-            errorClass: "FxaIntrospectError",
           },
           fingerprint: ["firefox-api-fxa-introspect"],
         });
-        expect(logger.error).toHaveBeenCalledWith("firefox_api_error", {
-          outcome: "introspection-error",
-          endpoint: ENDPOINT,
-          httpStatus,
-          errorClass: "FxaIntrospectError",
-          fingerprint: ["firefox-api-fxa-introspect"],
-          errorMessage: `FxA introspect returned ${httpStatus}`,
-          stack: error.stack,
-        });
         expect(handler).not.toHaveBeenCalled();
       },
     );
@@ -256,11 +246,7 @@ describe("withFxaUser", () => {
       expect(Sentry.captureException).toHaveBeenCalledWith(
         new Error("FxA introspect returned a malformed body"),
         {
-          tags: {
-            outcome: "introspection-error",
-            endpoint: ENDPOINT,
-            errorClass: "Error",
-          },
+          tags: { outcome: "introspection-error", endpoint: ENDPOINT },
           fingerprint: ["firefox-api-fxa-introspect"],
         },
       );
@@ -280,11 +266,7 @@ describe("withFxaUser", () => {
       expect(Sentry.captureException).toHaveBeenCalledWith(
         expect.anything(),
         expect.objectContaining({
-          tags: {
-            outcome: "introspection-error",
-            endpoint: ENDPOINT,
-            errorClass: "TimeoutError",
-          },
+          tags: { outcome: "introspection-error", endpoint: ENDPOINT },
         }),
       );
     });
@@ -305,11 +287,7 @@ describe("withFxaUser", () => {
         CLIENT_ID,
       );
       expect(Sentry.captureException).toHaveBeenCalledWith(new Error("boom"), {
-        tags: {
-          endpoint: ENDPOINT,
-          outcome: "lookup-error",
-          errorClass: "Error",
-        },
+        tags: { endpoint: ENDPOINT, outcome: "lookup-error" },
       });
       // Before the handler, so the auth counter has it
       expect(incFirefoxApiHandlerResponse).not.toHaveBeenCalled();
@@ -331,11 +309,7 @@ describe("withFxaUser", () => {
         CLIENT_ID,
       );
       expect(Sentry.captureException).toHaveBeenCalledWith(new Error("boom"), {
-        tags: {
-          endpoint: ENDPOINT,
-          outcome: "handler-error",
-          errorClass: "Error",
-        },
+        tags: { endpoint: ENDPOINT, outcome: "handler-error" },
       });
       expect(incFirefoxApiHandlerResponse).toHaveBeenCalledWith(
         ENDPOINT,
@@ -345,44 +319,20 @@ describe("withFxaUser", () => {
     });
   });
 
-  describe("error log", () => {
-    it("keeps message and stack on the log", async () => {
-      mockIntrospection();
-      vi.mocked(getSubscriberByFxaUid).mockResolvedValue(subscriber);
-      const error = new TypeError("boom");
-      handler.mockRejectedValueOnce(error);
-
-      await gated(authed());
-
-      expect(logger.error).toHaveBeenCalledWith("firefox_api_error", {
-        endpoint: ENDPOINT,
-        outcome: "handler-error",
-        errorClass: "TypeError",
-        fingerprint: undefined,
-        errorMessage: "boom",
-        stack: error.stack,
-      });
-    });
-
-    it("handles a thrown non-Error", async () => {
-      mockIntrospection();
-      vi.mocked(getSubscriberByFxaUid).mockResolvedValue(subscriber);
-      handler.mockRejectedValueOnce("boom");
+  it("logs the error message and stack", async () => {
+    mockIntrospection();
+    vi.mocked(getSubscriberByFxaUid).mockResolvedValue(subscriber);
+    const error = new TypeError("boom");
+    handler.mockRejectedValueOnce(error);
 
-      const res = await gated(authed());
+    await gated(authed());
 
-      expect(res.status).toBe(500);
-      expect(Sentry.captureException).toHaveBeenCalledWith("boom", {
-        tags: {
-          endpoint: ENDPOINT,
-          outcome: "handler-error",
-          errorClass: "string",
-        },
-      });
-      expect(logger.error).toHaveBeenCalledWith(
-        "firefox_api_error",
-        expect.objectContaining({ errorMessage: "boom", stack: undefined }),
-      );
+    expect(logger.error).toHaveBeenCalledExactlyOnceWith("firefox_api_error", {
+      endpoint: ENDPOINT,
+      outcome: "handler-error",
+      errorClass: "TypeError",
+      errorMessage: "boom",
+      stack: error.stack,
     });
   });
 
@@ -407,7 +357,6 @@ describe("withFxaUser", () => {
           reason: "http-error",
           endpoint: ENDPOINT,
           httpStatus: 502,
-          errorClass: "UpstreamUnavailableError",
         },
         fingerprint: ["firefox-api-hibp-unavailable"],
       });
@@ -543,11 +492,7 @@ describe("withFxaUser", () => {
         expect(Sentry.captureException).toHaveBeenCalledWith(
           expect.any(Error),
           {
-            tags: {
-              outcome: "redis-error",
-              endpoint: ENDPOINT,
-              errorClass: "Error",
-            },
+            tags: { outcome: "redis-error", endpoint: ENDPOINT },
             fingerprint: ["firefox-api-rate-limit-redis"],
           },
         );

src/app/api/utils/withFxaUser.ts

@@ -48,18 +48,14 @@ function reportError(
   tags: Record<string, string | number>,
   fingerprint?: string[],
 ) {
-  const errorClass = error instanceof Error ? error.name : typeof error;
-  Sentry.captureException(error, {
-    tags: { ...tags, errorClass },
-    ...(fingerprint ? { fingerprint } : {}),
-  });
+  Sentry.captureException(error, { tags, fingerprint });
+  const err = error instanceof Error ? error : undefined;
   // winston drops non-enumerable Error props
   logger.error("firefox_api_error", {
     ...tags,
-    errorClass,
-    fingerprint,
-    errorMessage: error instanceof Error ? error.message : String(error),
-    stack: error instanceof Error ? error.stack : undefined,
+    errorClass: err?.name ?? typeof error,
+    errorMessage: err?.message ?? String(error),
+    stack: err?.stack,
   });
 }