From 375015c59e63594afb1268ee6ed3a2e95f76ff02 Mon Sep 17 00:00:00 2001 From: Sophie Stadler Date: Thu, 22 Feb 2024 12:54:43 -0500 Subject: [PATCH] DEVPROD-356: Add Sentry grouping (#491) --- src/components/ErrorHandling/Sentry.tsx | 37 +++++++++++++++---- src/gql/GQLProvider.tsx | 13 +++++-- src/gql/generated/types.ts | 1 + src/hooks/useLogDownloader/index.ts | 10 ++--- .../errorReporting/errorReporting.test.ts | 25 +++++++++++-- src/utils/errorReporting/index.ts | 33 ++++++++++++++--- src/utils/matchingLines/index.ts | 10 ++--- 7 files changed, 99 insertions(+), 30 deletions(-) diff --git a/src/components/ErrorHandling/Sentry.tsx b/src/components/ErrorHandling/Sentry.tsx index b71c2c25..50b2ae99 100644 --- a/src/components/ErrorHandling/Sentry.tsx +++ b/src/components/ErrorHandling/Sentry.tsx @@ -3,9 +3,11 @@ import { captureException, getClient, init, + setTags, withScope, } from "@sentry/react"; import type { Scope, SeverityLevel } from "@sentry/react"; +import type { Context, Primitive } from "@sentry/types"; import { getReleaseStage, getSentryDSN, @@ -28,20 +30,41 @@ const initializeSentry = () => { const isInitialized = () => !!getClient(); -const sendError = ( - err: Error, - severity: SeverityLevel, - metadata?: { [key: string]: any }, -) => { +export type ErrorInput = { + err: Error; + fingerprint?: string[]; + context?: Context; + severity: SeverityLevel; + tags?: { [key: string]: Primitive }; +}; + +const sendError = ({ + context, + err, + fingerprint, + severity, + tags, +}: ErrorInput) => { withScope((scope) => { - setScope(scope, { context: metadata, level: severity }); + setScope(scope, { context, level: severity }); + + if (fingerprint) { + // A custom fingerprint allows for more intelligent grouping + scope.setFingerprint(fingerprint); + } + + if (tags) { + // Apply tags, which are a searchable/filterable property + setTags(tags); + } + captureException(err); }); }; type ScopeOptions = { level?: SeverityLevel; - context?: { [key: string]: any }; + context?: Context; }; const setScope = (scope: Scope, { context, level }: ScopeOptions = {}) => { diff --git a/src/gql/GQLProvider.tsx b/src/gql/GQLProvider.tsx index 7ee69db8..fcfd894d 100644 --- a/src/gql/GQLProvider.tsx +++ b/src/gql/GQLProvider.tsx @@ -13,10 +13,17 @@ import { reportError } from "utils/errorReporting"; const logErrorsLink = onError(({ graphQLErrors, operation }) => { if (Array.isArray(graphQLErrors)) { graphQLErrors.forEach((gqlErr) => { + const fingerprint = [operation.operationName]; + if (gqlErr?.path?.length) { + fingerprint.push(...gqlErr.path); + } reportError(new Error(gqlErr.message), { - gqlErr, - operationName: operation.operationName, - variables: operation.variables, + context: { + gqlErr, + variables: operation.variables, + }, + fingerprint, + tags: { operationName: operation.operationName }, }).warning(); }); } diff --git a/src/gql/generated/types.ts b/src/gql/generated/types.ts index c2f68ec0..28c3bca6 100644 --- a/src/gql/generated/types.ts +++ b/src/gql/generated/types.ts @@ -2402,6 +2402,7 @@ export type SpruceConfig = { jira?: Maybe; keys: Array; providers?: Maybe; + secretFields: Array; slack?: Maybe; spawnHost: SpawnHostConfig; ui?: Maybe; diff --git a/src/hooks/useLogDownloader/index.ts b/src/hooks/useLogDownloader/index.ts index ded8b8c3..b3cec20e 100644 --- a/src/hooks/useLogDownloader/index.ts +++ b/src/hooks/useLogDownloader/index.ts @@ -62,12 +62,11 @@ const useLogDownloader = ({ downloadSizeLimit, onIncompleteDownload: (reason, incompleteDownloadError) => { reportError(new Error("Incomplete download"), { - message: reason, - metadata: { + context: { incompleteDownloadError, + message: reason, url, }, - name: "Log download incomplete", }).warning(); dispatchToast.warning(LOG_FILE_DOWNLOAD_TOO_LARGE_WARNING, true, { @@ -99,12 +98,11 @@ const useLogDownloader = ({ reason: "Log line size limit exceeded", }); reportError(new Error("Incomplete download"), { - message: "Log line size limit exceeded", - metadata: { + context: { + message: "Log line size limit exceeded", trimmedLines, url, }, - name: "Log download incomplete", }).warning(); dispatchToast.warning(LOG_LINE_TOO_LARGE_WARNING, true, { shouldTimeout: false, diff --git a/src/utils/errorReporting/errorReporting.test.ts b/src/utils/errorReporting/errorReporting.test.ts index 7a459d87..6000f6d8 100644 --- a/src/utils/errorReporting/errorReporting.test.ts +++ b/src/utils/errorReporting/errorReporting.test.ts @@ -46,7 +46,7 @@ describe("error reporting", () => { expect(Sentry.captureException).toHaveBeenCalledWith(err); }); - it("supports metadata field", () => { + it("supports context field", () => { mockEnv("NODE_ENV", "production"); jest.spyOn(Sentry, "captureException").mockImplementation(jest.fn()); const err = { @@ -54,13 +54,32 @@ describe("error reporting", () => { name: "Error Name", }; - const metadata = { customField: "foo" }; - const result = reportError(err, metadata); + const context = { anything: "foo" }; + const result = reportError(err, { context }); result.severe(); expect(Sentry.captureException).toHaveBeenCalledWith(err); result.warning(); expect(Sentry.captureException).toHaveBeenCalledWith(err); }); + + it("supports tags", () => { + mockEnv("NODE_ENV", "production"); + jest.spyOn(Sentry, "captureException").mockImplementation(jest.fn()); + jest.spyOn(Sentry, "setTags").mockImplementation(jest.fn()); + const err = { + message: "GraphQL Error", + name: "Error Name", + }; + + const tags = { spruce: "true" }; + const result = reportError(err, { tags }); + result.severe(); + expect(Sentry.captureException).toHaveBeenCalledWith(err); + expect(Sentry.setTags).toHaveBeenCalledWith(tags); + result.warning(); + expect(Sentry.captureException).toHaveBeenCalledWith(err); + expect(Sentry.setTags).toHaveBeenCalledWith(tags); + }); }); describe("breadcrumbs", () => { diff --git a/src/utils/errorReporting/index.ts b/src/utils/errorReporting/index.ts index a43442b5..e12acd15 100644 --- a/src/utils/errorReporting/index.ts +++ b/src/utils/errorReporting/index.ts @@ -1,5 +1,8 @@ import { Breadcrumb, addBreadcrumb } from "@sentry/react"; -import { sendError as sentrySendError } from "components/ErrorHandling/Sentry"; +import { + ErrorInput, + sendError as sentrySendError, +} from "components/ErrorHandling/Sentry"; import { isProductionBuild } from "utils/environmentVariables"; interface reportErrorResult { @@ -7,27 +10,45 @@ interface reportErrorResult { warning: () => void; } +type ErrorMetadata = { + fingerprint?: ErrorInput["fingerprint"]; + tags?: ErrorInput["tags"]; + context?: ErrorInput["context"]; +}; + const reportError = ( err: Error, - metadata?: { [key: string]: any }, + { context, fingerprint, tags }: ErrorMetadata = {}, ): reportErrorResult => { if (!isProductionBuild()) { return { severe: () => { - console.error({ err, metadata, severity: "severe" }); + console.error({ context, err, fingerprint, severity: "severe", tags }); }, warning: () => { - console.error({ err, metadata, severity: "warning" }); + console.error({ context, err, fingerprint, severity: "warning", tags }); }, }; } return { severe: () => { - sentrySendError(err, "error", metadata); + sentrySendError({ + context, + err, + fingerprint, + severity: "error", + tags, + }); }, warning: () => { - sentrySendError(err, "warning", metadata); + sentrySendError({ + context, + err, + fingerprint, + severity: "warning", + tags, + }); }, }; }; diff --git a/src/utils/matchingLines/index.ts b/src/utils/matchingLines/index.ts index 313d0422..fb657a9f 100644 --- a/src/utils/matchingLines/index.ts +++ b/src/utils/matchingLines/index.ts @@ -26,11 +26,11 @@ export const constructRegexToMatch = (visibleFilters: Filters) => { }); } catch (e) { // If we get an error here, it means the regex is invalid and got past the validation step. We should report this error. - reportError(new Error("Invalid regex for filter"), { - message: `The regex "${f.expression}" is invalid`, - metadata: e, - name: "Invalid Regex", - }).severe(); + if (e instanceof Error) { + reportError(new Error("Invalid regex for filter"), { + context: { error: e.toString(), expression: f.expression }, + }).severe(); + } } } });