diff --git a/src/components/HistoryTable/HistoryTable.tsx b/src/components/HistoryTable/HistoryTable.tsx index 07941330db..f8bcbbbde8 100644 --- a/src/components/HistoryTable/HistoryTable.tsx +++ b/src/components/HistoryTable/HistoryTable.tsx @@ -2,7 +2,7 @@ import { useEffect, useMemo, useRef } from "react"; import throttle from "lodash.throttle"; import { Virtuoso, VirtuosoHandle } from "react-virtuoso"; import { useDimensions } from "hooks/useDimensions"; -import { leaveBreadcrumb } from "utils/errorReporting"; +import { leaveBreadcrumb, SentryBreadcrumb } from "utils/errorReporting"; import { types } from "."; import { useHistoryTable } from "./HistoryTableContext"; import EndOfHistoryRow from "./HistoryTableRow/EndOfHistoryRow"; @@ -55,7 +55,7 @@ const HistoryTable: React.FC = ({ { selectedCommit, }, - "process" + SentryBreadcrumb.UI ); listRef.current.scrollToIndex(selectedCommit.rowIndex); } @@ -71,7 +71,7 @@ const HistoryTable: React.FC = ({ selectedCommit, processedCommitCount, }, - "process" + SentryBreadcrumb.UI ); loadMoreItems(); } diff --git a/src/context/auth.tsx b/src/context/auth.tsx index 8d704d3637..5f02b0bb71 100644 --- a/src/context/auth.tsx +++ b/src/context/auth.tsx @@ -1,6 +1,6 @@ import { createContext, useContext, useMemo, useReducer } from "react"; import { environmentVariables } from "utils"; -import { leaveBreadcrumb } from "utils/errorReporting"; +import { leaveBreadcrumb, SentryBreadcrumb } from "utils/errorReporting"; const { getLoginDomain, getUiUrl } = environmentVariables; @@ -66,13 +66,13 @@ const AuthProvider: React.FC<{ children: React.ReactNode }> = ({ window.location.href = `${getLoginDomain()}/login`; }) .catch((error) => { - leaveBreadcrumb("Logout failed", { error }, "user"); + leaveBreadcrumb("Logout failed", { error }, SentryBreadcrumb.User); }); }, dispatchAuthenticated: () => { if (!state.isAuthenticated) { dispatch({ type: "authenticated" }); - leaveBreadcrumb("Authenticated", {}, "user"); + leaveBreadcrumb("Authenticated", {}, SentryBreadcrumb.User); } }, }), diff --git a/src/gql/GQLWrapper.tsx b/src/gql/GQLWrapper.tsx index ba3193c943..f0e62bd846 100644 --- a/src/gql/GQLWrapper.tsx +++ b/src/gql/GQLWrapper.tsx @@ -10,7 +10,11 @@ import { RetryLink } from "@apollo/client/link/retry"; import { routes } from "constants/routes"; import { useAuthDispatchContext } from "context/auth"; import { environmentVariables } from "utils"; -import { leaveBreadcrumb, reportError } from "utils/errorReporting"; +import { + leaveBreadcrumb, + reportError, + SentryBreadcrumb, +} from "utils/errorReporting"; const { getGQLUrl } = environmentVariables; @@ -128,7 +132,11 @@ const authLink = (logout: () => void): ApolloLink => networkError.statusCode === 401 && window.location.pathname !== routes.login ) { - leaveBreadcrumb("Not Authenticated", { statusCode: 401 }, "user"); + leaveBreadcrumb( + "Not Authenticated", + { status_code: 401 }, + SentryBreadcrumb.User + ); logout(); } }); @@ -164,7 +172,7 @@ const authenticateIfSuccessfulLink = ( status: !response.errors ? "OK" : "ERROR", errors: response.errors, }, - "request" + SentryBreadcrumb.HTTP ); return response; }) diff --git a/src/pages/taskHistory/index.tsx b/src/pages/taskHistory/index.tsx index 897856330b..0883a788e1 100644 --- a/src/pages/taskHistory/index.tsx +++ b/src/pages/taskHistory/index.tsx @@ -26,7 +26,7 @@ import { import { MAINLINE_COMMITS_FOR_HISTORY } from "gql/queries"; import { usePageTitle } from "hooks"; import { string } from "utils"; -import { leaveBreadcrumb } from "utils/errorReporting"; +import { leaveBreadcrumb, SentryBreadcrumb } from "utils/errorReporting"; import BuildVariantSelector from "./BuildVariantSelector"; import ColumnHeaders from "./ColumnHeaders"; import TaskHistoryRow from "./TaskHistoryRow"; @@ -76,7 +76,7 @@ const TaskHistoryContents: React.FC = () => { taskName, numCommits: mainlineCommits.versions.length, }, - "process" + SentryBreadcrumb.UI ); ingestNewCommits(mainlineCommits); }, @@ -96,7 +96,7 @@ const TaskHistoryContents: React.FC = () => { taskName, skipOrderNumber: data.mainlineCommits?.nextPageOrderNumber, }, - "process" + SentryBreadcrumb.UI ); refetch({ mainlineCommitsOptions: { diff --git a/src/pages/variantHistory/index.tsx b/src/pages/variantHistory/index.tsx index fde2af78ca..e0a373ae05 100644 --- a/src/pages/variantHistory/index.tsx +++ b/src/pages/variantHistory/index.tsx @@ -26,7 +26,7 @@ import { import { MAINLINE_COMMITS_FOR_HISTORY } from "gql/queries"; import { usePageTitle } from "hooks"; import { string } from "utils"; -import { leaveBreadcrumb } from "utils/errorReporting"; +import { leaveBreadcrumb, SentryBreadcrumb } from "utils/errorReporting"; import ColumnHeaders from "./ColumnHeaders"; import TaskSelector from "./TaskSelector"; import VariantHistoryRow from "./VariantHistoryRow"; @@ -74,7 +74,7 @@ const VariantHistoryContents: React.FC = () => { variantName, numCommits: mainlineCommits.versions.length, }, - "process" + SentryBreadcrumb.UI ); ingestNewCommits(mainlineCommits); }, @@ -94,7 +94,7 @@ const VariantHistoryContents: React.FC = () => { variantName, skipOrderNumber: data.mainlineCommits?.nextPageOrderNumber, }, - "process" + SentryBreadcrumb.UI ); refetch({ mainlineCommitsOptions: { diff --git a/src/utils/errorReporting.test.ts b/src/utils/errorReporting.test.ts index c219dbf666..4ed10d3b68 100644 --- a/src/utils/errorReporting.test.ts +++ b/src/utils/errorReporting.test.ts @@ -1,14 +1,16 @@ -import Bugsnag from "@bugsnag/js"; import * as Sentry from "@sentry/react"; import { mockEnvironmentVariables } from "test_utils/utils"; -import { leaveBreadcrumb, reportError } from "utils/errorReporting"; +import { + leaveBreadcrumb, + reportError, + SentryBreadcrumb, +} from "utils/errorReporting"; const { cleanup, mockEnv } = mockEnvironmentVariables(); describe("error reporting", () => { beforeEach(() => { jest.spyOn(console, "error").mockImplementation(() => {}); - jest.spyOn(Bugsnag, "notify"); jest.spyOn(Sentry, "captureException"); }); afterEach(() => { @@ -29,28 +31,23 @@ describe("error reporting", () => { err, severity: "warning", }); - expect(Bugsnag.notify).not.toHaveBeenCalled(); expect(Sentry.captureException).not.toHaveBeenCalled(); }); - it("should report errors to Bugsnag and Sentry when in production", () => { + it("should report errors to Sentry when in production", () => { mockEnv("NODE_ENV", "production"); - jest.spyOn(Bugsnag, "notify").mockImplementation(jest.fn()); jest.spyOn(Sentry, "captureException").mockImplementation(jest.fn()); const err = new Error("test error"); const result = reportError(err); result.severe(); - expect(Bugsnag.notify).toHaveBeenCalledWith(err, expect.any(Function)); expect(Sentry.captureException).toHaveBeenCalledWith(err); result.warning(); - expect(Bugsnag.notify).toHaveBeenLastCalledWith(err, expect.any(Function)); expect(Sentry.captureException).toHaveBeenCalledWith(err); }); it("supports metadata field", () => { mockEnv("NODE_ENV", "production"); - jest.spyOn(Bugsnag, "notify").mockImplementation(jest.fn()); jest.spyOn(Sentry, "captureException").mockImplementation(jest.fn()); const err = { message: "GraphQL Error", @@ -60,10 +57,8 @@ describe("error reporting", () => { const metadata = { customField: "foo" }; const result = reportError(err, metadata); result.severe(); - expect(Bugsnag.notify).toHaveBeenCalledWith(err, expect.any(Function)); expect(Sentry.captureException).toHaveBeenCalledWith(err); result.warning(); - expect(Bugsnag.notify).toHaveBeenLastCalledWith(err, expect.any(Function)); expect(Sentry.captureException).toHaveBeenCalledWith(err); }); }); @@ -71,7 +66,6 @@ describe("error reporting", () => { describe("breadcrumbs", () => { beforeEach(() => { jest.spyOn(console, "debug").mockImplementation(() => {}); - jest.spyOn(Bugsnag, "leaveBreadcrumb"); jest.spyOn(Sentry, "addBreadcrumb"); }); afterEach(() => { @@ -81,7 +75,7 @@ describe("breadcrumbs", () => { it("should log breadcrumbs into console when not in production", () => { const message = "my message"; - const type = "error"; + const type = SentryBreadcrumb.Error; const metadata = { foo: "bar" }; leaveBreadcrumb(message, metadata, type); @@ -90,26 +84,19 @@ describe("breadcrumbs", () => { metadata, type, }); - expect(Bugsnag.leaveBreadcrumb).not.toHaveBeenCalled(); expect(Sentry.addBreadcrumb).not.toHaveBeenCalled(); }); - it("should report breadcrumbs to Bugsnag and Sentry when in production and convert Sentry breadcrumbs", () => { + it("should report breadcrumbs to Sentry when in production", () => { jest.useFakeTimers().setSystemTime(new Date("2020-01-01")); mockEnv("NODE_ENV", "production"); - jest.spyOn(Bugsnag, "leaveBreadcrumb").mockImplementation(jest.fn()); jest.spyOn(Sentry, "addBreadcrumb").mockImplementation(jest.fn()); const message = "my message"; - const type = "log"; - const metadata = { statusCode: 401 }; + const type = SentryBreadcrumb.Info; + const metadata = { status_code: 401 }; leaveBreadcrumb(message, metadata, type); - expect(Bugsnag.leaveBreadcrumb).toHaveBeenCalledWith( - message, - metadata, - type - ); expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({ message, type: "info", @@ -123,11 +110,10 @@ describe("breadcrumbs", () => { jest.useFakeTimers().setSystemTime(new Date("2020-01-01")); jest.spyOn(console, "warn").mockImplementation(() => {}); mockEnv("NODE_ENV", "production"); - jest.spyOn(Bugsnag, "leaveBreadcrumb").mockImplementation(jest.fn()); jest.spyOn(Sentry, "addBreadcrumb").mockImplementation(jest.fn()); const message = "navigation message"; - const type = "navigation"; + const type = SentryBreadcrumb.Navigation; const metadata = {}; leaveBreadcrumb(message, metadata, type); @@ -139,11 +125,6 @@ describe("breadcrumbs", () => { 2, "Navigation breadcrumbs should include a 'to' metadata field." ); - expect(Bugsnag.leaveBreadcrumb).toHaveBeenCalledWith( - message, - metadata, - type - ); expect(Sentry.addBreadcrumb).toHaveBeenCalledWith({ message, type, diff --git a/src/utils/errorReporting.ts b/src/utils/errorReporting.ts index c555c81160..1e9cf63ae8 100644 --- a/src/utils/errorReporting.ts +++ b/src/utils/errorReporting.ts @@ -1,6 +1,4 @@ -import Bugsnag, { BreadcrumbType } from "@bugsnag/js"; import { addBreadcrumb, Breadcrumb } from "@sentry/react"; -import { sendError as bugsnagSendError } from "components/ErrorHandling/Bugsnag"; import { sendError as sentrySendError } from "components/ErrorHandling/Sentry"; import { isProductionBuild } from "./environmentVariables"; @@ -26,55 +24,14 @@ const reportError = ( return { severe: () => { - bugsnagSendError(err, "error", metadata); sentrySendError(err, "error", metadata); }, warning: () => { - bugsnagSendError(err, "warning", metadata); sentrySendError(err, "warning", metadata); }, }; }; -type Metadata = { - [key: string]: any; -}; - -const leaveBreadcrumb = ( - message: string, - metadata: Metadata, - type: BreadcrumbType -) => { - if (!isProductionBuild()) { - console.debug({ message, metadata, type }); - } else { - Bugsnag.leaveBreadcrumb(message, metadata, type); - addBreadcrumb(convertToSentryBreadcrumb(message, metadata, type)); - } -}; - -/** - * Convert a Bugsnag breadcrumb to Sentry's type. - * @param message - a string indicating details about the breadcrumb - * @param metadata - additional fields - * @param type - the type of Bugsnag breadcrumb to be sent - * @returns a Sentry breadcrumb - */ -const convertToSentryBreadcrumb = ( - message: string, - metadata: Metadata, - type: BreadcrumbType -): Breadcrumb => { - const breadcrumbType = convertBreadcrumbType(type); - return { - message, - data: convertMetadata(metadata, breadcrumbType), - // Divide date by 1000 because Sentry wants the timestamp in RFC 3339, or seconds (not milliseconds!) since the Unix epoch. - timestamp: new Date().getTime() / 1000, - type: breadcrumbType, - }; -}; - // The "type" field for Sentry breadcrumbs is just "string", but we can approximate the types listed here: // https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/#breadcrumb-types enum SentryBreadcrumb { @@ -90,68 +47,50 @@ enum SentryBreadcrumb { User = "user", } -/** - * Convert a Bugsnag breadcrumb type to one of our custom Sentry types. - * @param breadcrumbType - the type of Bugsnag breadcrumb to be sent - * @returns a custom Sentry breadcrumb type - */ -const convertBreadcrumbType = ( - breadcrumbType: BreadcrumbType -): SentryBreadcrumb => { - switch (breadcrumbType) { - case "error": - return SentryBreadcrumb.Error; - case "navigation": - return SentryBreadcrumb.Navigation; - case "process": - return SentryBreadcrumb.UI; - case "request": - return SentryBreadcrumb.HTTP; - case "user": - return SentryBreadcrumb.User; - case "log": - return SentryBreadcrumb.Info; - case "manual": - case "state": - default: - return SentryBreadcrumb.Default; +const leaveBreadcrumb = ( + message: string, + metadata: Breadcrumb["data"], + type: SentryBreadcrumb +) => { + if (!isProductionBuild()) { + console.debug({ message, metadata, type }); + } else { + const bc: Breadcrumb = { + message, + data: validateMetadata(metadata, type), + // Divide date by 1000 because Sentry wants the timestamp in RFC 3339, or seconds (not milliseconds!) since the Unix epoch. + timestamp: new Date().getTime() / 1000, + type, + }; + addBreadcrumb(bc); } }; /** - * Convert a Bugsnag metadata field to use Sentry's key names and warn about missing fields. + * Ensure metadata follows Sentry's breadcrumb guidelines. * https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/#breadcrumb-types - * @param bugsnagMetadata - Metadata object using Bugsnag-specific key names + * @param metadata - Metadata object * @param breadcrumbType - Sentry breadcrumb type - * @returns an object with the key names converted to Sentry's names. + * @returns an object adhering to Sentry's metadata rules. */ -const convertMetadata = ( - bugsnagMetadata: Metadata, +const validateMetadata = ( + metadata: Breadcrumb["data"], breadcrumbType: SentryBreadcrumb -): Metadata => { - // Convert statusCode => status_code - if (bugsnagMetadata.statusCode) { - const { statusCode, ...rest } = bugsnagMetadata; - return { - ...rest, - status_code: statusCode, - }; - } - +): Breadcrumb["data"] => { if (breadcrumbType === SentryBreadcrumb.Navigation) { - if (!bugsnagMetadata.from) { + if (!metadata.from) { console.warn( "Navigation breadcrumbs should include a 'from' metadata field." ); } - if (!bugsnagMetadata.to) { + if (!metadata.to) { console.warn( "Navigation breadcrumbs should include a 'to' metadata field." ); } } - return bugsnagMetadata; + return metadata; }; -export { leaveBreadcrumb, reportError }; +export { leaveBreadcrumb, reportError, SentryBreadcrumb };