From 2d5312276cfdf1d57b48996961ea900b3857df75 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Thu, 26 Jun 2025 17:54:12 +0200 Subject: [PATCH] Improve FE error handling (#12864) This PR aims at improving readability in sentry and user experience with runtime errors. **GraphQL errors (and ApolloError)** 1. In sentry we have a lot of "Object captured as exception with keys: extensions, message" errors (2k over the last 90d), on which we have zero information. This is because in apollo-factory we were passing on GraphQL errors to sentry directly why sentry expects the structure of a JS Error. We are now changing that, rebuilding an Error object and attempting to help grouping by creating a fingerPrint based on error code and truncated operationName (same as we do in the back for 500 graphql errors). 2. In sentry we have a lot of ApolloError, who actually correspond to errors that should not be logged in sentry (Forbidden errors such as "Email is not verified"), or errors that are already tracked by back-end (Postgres errors such as "column xxx does not exist"). This is because ApolloErrors become unhandled rejections errors if they are not caught and automatically sent to sentry through the basic config. To change that we are now filtering out ApolloErrors created from GraphQL Errors before sending error to sentry: image **Runtime errors** 4. Runtime errors were all caught by sentry with the name "Error", making them not easy to differentiate on sentry (they were not grouped together but all appeared in the list as "Error"). We are replacing the "Error" name with the error message, or the error code if present. We are introducing a CustomError class that allows errors whose message contain dynamic text (an id for instance) to be identified on sentry with a common code. _(TODO: if this approach is validated then I have yet to replace Error with dynamic error messages with CustomError)_ 5. Runtime error messages contain technical details that do not mean anything to users (for instance, "Invalid folder ID: ${droppableId}", "ObjectMetadataItem not found", etc.). Let's replace them with "Please refresh the page." to users and keep the message error for sentry and our dev experience (they will still show in the console as uncaught errors). --- .../utils/getActivityTargetObjectRecords.ts | 2 +- .../modules/apollo/services/apollo.factory.ts | 38 +++++++++++++++++-- .../components/CommandMenuRecordPage.tsx | 4 +- .../src/modules/error-handler/CustomError.ts | 9 +++++ .../components/AppErrorBoundary.tsx | 14 ++++++- .../components/AppRootErrorFallback.tsx | 8 ++-- .../components/PromiseRejectionEffect.tsx | 31 ++++++++++++++- .../components/SentryInitEffect.tsx | 18 +++++++-- .../components/internal/AppErrorDisplay.tsx | 8 ++-- .../utils/validateAndExtractFolderId.ts | 18 +++++++-- .../hooks/useFieldMetadataItemById.ts | 9 ++++- .../hooks/useObjectMetadataItemById.ts | 6 ++- .../hooks/useAttachRelatedRecordFromRecord.ts | 9 ++++- .../utils/getAggregateOperationLabel.ts | 6 ++- .../utils/getAggregateOperationShortLabel.ts | 6 ++- .../input/components/MultiItemFieldInput.tsx | 6 ++- .../utils/computeDraftValueFromString.ts | 6 ++- .../utils/computeEmptyDraftValue.ts | 6 ++- ...computeEmptyGqlOperationFilterForEmails.ts | 6 ++- .../computeEmptyGqlOperationFilterForLinks.ts | 6 ++- .../computeGqlOperationFilterForEmails.ts | 9 ++++- .../computeGqlOperationFilterForLinks.ts | 6 ++- .../utils/getEmptyRecordGqlOperationFilter.ts | 11 +++++- .../accounts/hooks/useTriggerApiOAuth.ts | 8 +++- .../ConfigVariableDatabaseInput.tsx | 3 +- .../utils/useSourceContent.ts | 3 +- .../utils/editDatabaseConnection.ts | 11 +++++- .../components/PlaygroundSetupForm.tsx | 6 ++- .../utils/shouldDisplayFormField.ts | 6 ++- .../testing/decorators/getFieldDecorator.tsx | 6 ++- packages/twenty-front/src/utils/date-utils.ts | 16 ++++++-- .../drivers/sentry.driver.ts | 19 +++++----- .../graphql/utils/graphql-errors.util.ts | 2 +- packages/twenty-shared/src/utils/index.ts | 2 + .../utils/sentry/getGenericOperationName.ts | 4 ++ .../sentry/getHumanReadableNameFromCode.ts | 6 +++ 36 files changed, 272 insertions(+), 62 deletions(-) create mode 100644 packages/twenty-front/src/modules/error-handler/CustomError.ts create mode 100644 packages/twenty-shared/src/utils/sentry/getGenericOperationName.ts create mode 100644 packages/twenty-shared/src/utils/sentry/getHumanReadableNameFromCode.ts diff --git a/packages/twenty-front/src/modules/activities/utils/getActivityTargetObjectRecords.ts b/packages/twenty-front/src/modules/activities/utils/getActivityTargetObjectRecords.ts index b3c8131b9..42d230a40 100644 --- a/packages/twenty-front/src/modules/activities/utils/getActivityTargetObjectRecords.ts +++ b/packages/twenty-front/src/modules/activities/utils/getActivityTargetObjectRecords.ts @@ -38,7 +38,7 @@ export const getActivityTargetObjectRecords = ({ const activityTargetObjectRecords = targets .map((activityTarget) => { if (!isDefined(activityTarget)) { - throw new Error(`Cannot find activity target`); + throw new Error('Cannot find activity target'); } const correspondingObjectMetadataItem = objectMetadataItems.find( diff --git a/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts b/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts index 1b351979e..c2f7a9cf8 100644 --- a/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts +++ b/packages/twenty-front/src/modules/apollo/services/apollo.factory.ts @@ -19,7 +19,8 @@ import { logDebug } from '~/utils/logDebug'; import { i18n } from '@lingui/core'; import { GraphQLFormattedError } from 'graphql'; -import { isDefined } from 'twenty-shared/utils'; +import isEmpty from 'lodash.isempty'; +import { getGenericOperationName, isDefined } from 'twenty-shared/utils'; import { cookieStorage } from '~/utils/cookie-storage'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; import { ApolloManager } from '../types/apolloManager.interface'; @@ -160,8 +161,39 @@ export class ApolloFactory implements ApolloManager { ); } import('@sentry/react') - .then(({ captureException }) => { - captureException(graphQLError); + .then(({ captureException, withScope }) => { + withScope((scope) => { + const error = new Error(graphQLError.message); + + error.name = graphQLError.message; + + const fingerPrint: string[] = []; + if (isDefined(graphQLError.extensions)) { + scope.setExtra('extensions', graphQLError.extensions); + if (isDefined(graphQLError.extensions.code)) { + fingerPrint.push( + graphQLError.extensions.code as string, + ); + } + } + + if (isDefined(operation.operationName)) { + scope.setExtra('operation', operation.operationName); + const genericOperationName = getGenericOperationName( + operation.operationName, + ); + + if (isDefined(genericOperationName)) { + fingerPrint.push(genericOperationName); + } + } + + if (!isEmpty(fingerPrint)) { + scope.setFingerprint(fingerPrint); + } + + captureException(error); // Sentry expects a JS error + }); }) .catch((sentryError) => { // eslint-disable-next-line no-console diff --git a/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuRecordPage.tsx b/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuRecordPage.tsx index 2409b0f45..23396c698 100644 --- a/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuRecordPage.tsx +++ b/packages/twenty-front/src/modules/command-menu/pages/record-page/components/CommandMenuRecordPage.tsx @@ -32,11 +32,11 @@ export const CommandMenuRecordPage = () => { ); if (!viewableRecordNameSingular) { - throw new Error(`Object name is not defined`); + throw new Error('Object name is not defined'); } if (!viewableRecordId) { - throw new Error(`Record id is not defined`); + throw new Error('Record id is not defined'); } const { objectNameSingular, objectRecordId } = useRecordShowPage( diff --git a/packages/twenty-front/src/modules/error-handler/CustomError.ts b/packages/twenty-front/src/modules/error-handler/CustomError.ts new file mode 100644 index 000000000..32c9ea569 --- /dev/null +++ b/packages/twenty-front/src/modules/error-handler/CustomError.ts @@ -0,0 +1,9 @@ +// This class is used to group different error messages under the same code for sentry. +export class CustomError extends Error { + public code?: string; + + constructor(message: string, code?: string) { + super(message); + this.code = code; + } +} diff --git a/packages/twenty-front/src/modules/error-handler/components/AppErrorBoundary.tsx b/packages/twenty-front/src/modules/error-handler/components/AppErrorBoundary.tsx index 86eb81834..d2d95b7e6 100644 --- a/packages/twenty-front/src/modules/error-handler/components/AppErrorBoundary.tsx +++ b/packages/twenty-front/src/modules/error-handler/components/AppErrorBoundary.tsx @@ -1,6 +1,8 @@ import { AppErrorBoundaryEffect } from '@/error-handler/components/internal/AppErrorBoundaryEffect'; +import { CustomError } from '@/error-handler/CustomError'; import { ErrorInfo, ReactNode } from 'react'; import { ErrorBoundary, FallbackProps } from 'react-error-boundary'; +import { isDefined } from 'twenty-shared/utils'; type AppErrorBoundaryProps = { children: ReactNode; @@ -8,16 +10,26 @@ type AppErrorBoundaryProps = { resetOnLocationChange?: boolean; }; +const hasErrorCode = ( + error: Error | CustomError, +): error is CustomError & { code: string } => { + return 'code' in error && isDefined(error.code); +}; + export const AppErrorBoundary = ({ children, FallbackComponent, resetOnLocationChange = true, }: AppErrorBoundaryProps) => { - const handleError = async (error: Error, info: ErrorInfo) => { + const handleError = async (error: Error | CustomError, info: ErrorInfo) => { try { const { captureException } = await import('@sentry/react'); captureException(error, (scope) => { scope.setExtras({ info }); + + const fingerprint = hasErrorCode(error) ? error.code : error.message; + scope.setFingerprint([fingerprint]); + error.name = error.message; return scope; }); } catch (sentryError) { diff --git a/packages/twenty-front/src/modules/error-handler/components/AppRootErrorFallback.tsx b/packages/twenty-front/src/modules/error-handler/components/AppRootErrorFallback.tsx index 316e1abdf..eac4365bd 100644 --- a/packages/twenty-front/src/modules/error-handler/components/AppRootErrorFallback.tsx +++ b/packages/twenty-front/src/modules/error-handler/components/AppRootErrorFallback.tsx @@ -1,5 +1,6 @@ import { AppErrorDisplayProps } from '@/error-handler/types/AppErrorDisplayProps'; import styled from '@emotion/styled'; +import { t } from '@lingui/core/macro'; import { motion } from 'framer-motion'; import { IconReload } from 'twenty-ui/display'; import { GRAY_SCALE, THEME_DARK } from 'twenty-ui/theme'; @@ -98,7 +99,6 @@ const StyledIcon = styled(IconReload)` `; export const AppRootErrorFallback = ({ - error, resetErrorBoundary, title = 'Sorry, something went wrong', }: AppRootErrorFallbackProps) => { @@ -117,8 +117,10 @@ export const AppRootErrorFallback = ({ /> - {title} - {error.message} + {t`${title}`} + + {t`Please refresh the page.`} + diff --git a/packages/twenty-front/src/modules/error-handler/components/PromiseRejectionEffect.tsx b/packages/twenty-front/src/modules/error-handler/components/PromiseRejectionEffect.tsx index a385b1fb0..dd172ccf7 100644 --- a/packages/twenty-front/src/modules/error-handler/components/PromiseRejectionEffect.tsx +++ b/packages/twenty-front/src/modules/error-handler/components/PromiseRejectionEffect.tsx @@ -1,17 +1,25 @@ import { useCallback, useEffect } from 'react'; +import { CustomError } from '@/error-handler/CustomError'; import { ObjectMetadataItemNotFoundError } from '@/object-metadata/errors/ObjectMetadataNotFoundError'; import { SnackBarVariant } from '@/ui/feedback/snack-bar-manager/components/SnackBar'; import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; +import isEmpty from 'lodash.isempty'; +import { isDefined } from 'twenty-shared/utils'; + +const hasErrorCode = ( + error: CustomError | any, +): error is CustomError & { code: string } => { + return 'code' in error && isDefined(error.code); +}; export const PromiseRejectionEffect = () => { const { enqueueSnackBar } = useSnackBar(); const handlePromiseRejection = useCallback( - (event: PromiseRejectionEvent) => { + async (event: PromiseRejectionEvent) => { const error = event.reason; - // TODO: connect Sentry here if (error instanceof ObjectMetadataItemNotFoundError) { enqueueSnackBar( `Error with custom object that cannot be found : ${event.reason}`, @@ -24,6 +32,25 @@ export const PromiseRejectionEffect = () => { variant: SnackBarVariant.Error, }); } + + if (error.name === 'ApolloError' && !isEmpty(error.graphQLErrors)) { + return; // already handled by apolloLink + } + + try { + const { captureException } = await import('@sentry/react'); + captureException(error, (scope) => { + scope.setExtras({ mechanism: 'onUnhandle' }); + + const fingerprint = hasErrorCode(error) ? error.code : error.message; + scope.setFingerprint([fingerprint]); + error.name = error.message; + return scope; + }); + } catch (sentryError) { + // eslint-disable-next-line no-console + console.error('Failed to capture exception with Sentry:', sentryError); + } }, [enqueueSnackBar], ); diff --git a/packages/twenty-front/src/modules/error-handler/components/SentryInitEffect.tsx b/packages/twenty-front/src/modules/error-handler/components/SentryInitEffect.tsx index 7b3beab52..e68b5e518 100644 --- a/packages/twenty-front/src/modules/error-handler/components/SentryInitEffect.tsx +++ b/packages/twenty-front/src/modules/error-handler/components/SentryInitEffect.tsx @@ -1,4 +1,3 @@ -import { isNonEmptyString } from '@sniptt/guards'; import { useEffect, useState } from 'react'; import { useRecoilValue } from 'recoil'; @@ -6,6 +5,7 @@ import { currentUserState } from '@/auth/states/currentUserState'; import { currentWorkspaceMemberState } from '@/auth/states/currentWorkspaceMemberState'; import { currentWorkspaceState } from '@/auth/states/currentWorkspaceState'; import { sentryConfigState } from '@/client-config/states/sentryConfigState'; +import { isNonEmptyString } from '@sniptt/guards'; import { isDefined } from 'twenty-shared/utils'; import { REACT_APP_SERVER_BASE_URL } from '~/config'; @@ -30,14 +30,24 @@ export const SentryInitEffect = () => { setIsSentryInitializing(true); try { - const { init, browserTracingIntegration, replayIntegration } = - await import('@sentry/react'); + const { + init, + browserTracingIntegration, + replayIntegration, + globalHandlersIntegration, + } = await import('@sentry/react'); init({ environment: sentryConfig?.environment ?? undefined, release: sentryConfig?.release ?? undefined, dsn: sentryConfig?.dsn, - integrations: [browserTracingIntegration({}), replayIntegration()], + integrations: [ + browserTracingIntegration({}), + replayIntegration(), + globalHandlersIntegration({ + onunhandledrejection: false, // handled in PromiseRejectionEffect + }), + ], tracePropagationTargets: [ 'localhost:3001', REACT_APP_SERVER_BASE_URL, diff --git a/packages/twenty-front/src/modules/error-handler/components/internal/AppErrorDisplay.tsx b/packages/twenty-front/src/modules/error-handler/components/internal/AppErrorDisplay.tsx index a1237739e..810618bed 100644 --- a/packages/twenty-front/src/modules/error-handler/components/internal/AppErrorDisplay.tsx +++ b/packages/twenty-front/src/modules/error-handler/components/internal/AppErrorDisplay.tsx @@ -1,4 +1,7 @@ import { AppErrorDisplayProps } from '@/error-handler/types/AppErrorDisplayProps'; +import { t } from '@lingui/core/macro'; +import { IconRefresh } from 'twenty-ui/display'; +import { Button } from 'twenty-ui/input'; import { AnimatedPlaceholder, AnimatedPlaceholderEmptyContainer, @@ -6,11 +9,8 @@ import { AnimatedPlaceholderEmptyTextContainer, AnimatedPlaceholderEmptyTitle, } from 'twenty-ui/layout'; -import { Button } from 'twenty-ui/input'; -import { IconRefresh } from 'twenty-ui/display'; export const AppErrorDisplay = ({ - error, resetErrorBoundary, title = 'Sorry, something went wrong', }: AppErrorDisplayProps) => { @@ -20,7 +20,7 @@ export const AppErrorDisplay = ({ {title} - {error.message} + {t`Please refresh the page.`}