Skip to content

Commit

Permalink
refactor(Sentry): improve error monitoring (#584)
Browse files Browse the repository at this point in the history
  • Loading branch information
balzdur authored Nov 6, 2024
1 parent da75382 commit 4477dcf
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 18 deletions.
1 change: 1 addition & 0 deletions packages/app-builder/src/components/Auth/AuthError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { authI18n } from './auth-i18n';

const errorLabels: Record<AuthErrors, ParseKeys<typeof authI18n>> = {
NoAccount: 'auth:errors.no_account',
CSRFError: 'auth:errors.csrf_error',
Unknown: 'common:errors.unknown',
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
AccountExistsWithDifferentCredential,
InvalidLoginCredentials,
NetworkRequestFailed,
PopupBlockedByClient,
useGoogleSignIn,
Expand Down Expand Up @@ -51,7 +52,7 @@ function ClientSignInWithGoogle({
signIn: (authPayload: AuthPayload) => void;
loading?: boolean;
}) {
const { t } = useTranslation(['common']);
const { t } = useTranslation(['common', 'auth']);
const googleSignIn = useGoogleSignIn(
clientServices.authenticationClientService,
);
Expand All @@ -72,6 +73,8 @@ function ClientSignInWithGoogle({
toast.error(<PopupBlockedError />);
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else if (error instanceof InvalidLoginCredentials) {
toast.error(t('auth:sign_in.errors.invalid_login_credentials'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
AccountExistsWithDifferentCredential,
InvalidLoginCredentials,
NetworkRequestFailed,
PopupBlockedByClient,
useMicrosoftSignIn,
Expand Down Expand Up @@ -53,7 +54,7 @@ function ClientSignInWithMicrosoft({
signIn: (authPayload: AuthPayload) => void;
loading?: boolean;
}) {
const { t } = useTranslation(['common']);
const { t } = useTranslation(['common', 'auth']);
const microsoftSignIn = useMicrosoftSignIn(
clientServices.authenticationClientService,
);
Expand All @@ -74,6 +75,8 @@ function ClientSignInWithMicrosoft({
toast.error(<PopupBlockedError />);
} else if (error instanceof NetworkRequestFailed) {
toast.error(t('common:errors.firebase_network_error'));
} else if (error instanceof InvalidLoginCredentials) {
toast.error(t('auth:sign_in.errors.invalid_login_credentials'));
} else {
Sentry.captureException(error);
toast.error(t('common:errors.unknown'));
Expand Down
8 changes: 2 additions & 6 deletions packages/app-builder/src/entry.server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { I18nextProvider } from 'react-i18next';
import { PassThrough } from 'stream';

import { serverServices } from './services/init.server';
import { captureUnexpectedRemixError } from './services/monitoring';
import { getServerEnv } from './utils/environment';

const ABORT_DELAY = 5000;
Expand Down Expand Up @@ -185,10 +186,5 @@ export const handleError: HandleErrorFunction = (error, { request }) => {
if (request.signal.aborted) {
return;
}
if (error instanceof Error) {
void Sentry.captureRemixServerException(error, 'remix.server', request);
} else {
// Optionally capture non-Error objects
Sentry.captureException(error);
}
captureUnexpectedRemixError(error, 'remix.server', request);
};
1 change: 1 addition & 0 deletions packages/app-builder/src/locales/en/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"sign_in.google": "Sign in with Google",
"sign_in.microsoft": "Sign in with Microsoft",
"errors.no_account": "No Marble account found for this address.",
"errors.csrf_error": "A required security token was not found or was invalid.\n Refresh the page and try again.",
"sign_in.email": "Email",
"sign_in.password": "Password",
"sign_in": "Sign in",
Expand Down
1 change: 1 addition & 0 deletions packages/app-builder/src/locales/fr/auth.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"email-verification.resend": "renvoyer l'e-mail de vérification",
"email-verification.wrong_place": "Mauvais endroit ? \nrevenez à <SignIn>{{signIn}}</SignIn>.",
"errors.no_account": "Aucun compte Marble trouvé pour cette adresse.",
"errors.csrf_error": "Un jeton de sécurité requis n'a pas été trouvé ou était invalide. \nActualisez la page et réessayez.",
"great_rules_right_tools": "Les bonnes règles sont construites avec les <RightTools>bons outils</RightTools>.",
"marble_description": "Marble est un moteur de règles en temps réel pour la surveillance de la fraude et de la conformité, conçu pour les sociétés de technologie financière et les institutions financières.",
"reset-password.email_sent": "Un email vous a été envoyé avec un lien pour réinitialiser votre mot de passe.",
Expand Down
7 changes: 6 additions & 1 deletion packages/app-builder/src/models/auth-errors.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import { CSRFError } from 'remix-utils/csrf/server';

import { isMarbleError, isUnauthorizedHttpError } from './http-errors';

export type AuthErrors = 'NoAccount' | 'Unknown';
export type AuthErrors = 'NoAccount' | 'CSRFError' | 'Unknown';

export function adaptAuthErrors(error: unknown): AuthErrors {
if (error instanceof CSRFError) {
return 'CSRFError';
}
if (isUnauthorizedHttpError(error) && isMarbleError(error)) {
const errorCode = error.data.error_code;
if (errorCode === 'unknown_user') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import {
UniqueDataTypes,
} from '@app-builder/models';
import { serverServices } from '@app-builder/services/init.server';
import { captureUnexpectedRemixError } from '@app-builder/services/monitoring';
import { getRoute } from '@app-builder/utils/routes';
import { zodResolver } from '@hookform/resolvers/zod';
import { type ActionFunctionArgs, json } from '@remix-run/node';
import { useFetcher } from '@remix-run/react';
import * as Sentry from '@sentry/remix';
import { type Namespace } from 'i18next';
import { useEffect, useState } from 'react';
import { Form, FormProvider, useForm, useWatch } from 'react-hook-form';
Expand Down Expand Up @@ -119,7 +119,7 @@ export async function action({ request }: ActionFunctionArgs) {
type: 'error',
messageKey: 'common:errors.unknown',
});
Sentry.captureException(error);
captureUnexpectedRemixError(error, 'createField@action', request);
return json(
{
success: false as const,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
UniqueDataTypes,
} from '@app-builder/models';
import { serverServices } from '@app-builder/services/init.server';
import { captureUnexpectedRemixError } from '@app-builder/services/monitoring';
import { getRoute } from '@app-builder/utils/routes';
import { zodResolver } from '@hookform/resolvers/zod';
import { type ActionFunctionArgs, json } from '@remix-run/node';
import { useFetcher } from '@remix-run/react';
import * as Sentry from '@sentry/remix';
import { type Namespace } from 'i18next';
import { useEffect, useMemo, useState } from 'react';
import { Form, FormProvider, useForm, useWatch } from 'react-hook-form';
Expand Down Expand Up @@ -73,7 +73,7 @@ export async function action({ request }: ActionFunctionArgs) {
type: 'error',
messageKey: 'common:errors.unknown',
});
Sentry.captureException(error);
captureUnexpectedRemixError(error, 'editField@action', request);
return json(
{
success: false as const,
Expand Down
4 changes: 4 additions & 0 deletions packages/app-builder/src/services/auth/auth.client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export function useGoogleSignIn({
throw new AccountExistsWithDifferentCredential();
case AuthErrorCodes.POPUP_BLOCKED:
throw new PopupBlockedByClient();
case AuthErrorCodes.INVALID_IDP_RESPONSE:
throw new InvalidLoginCredentials();
case AuthErrorCodes.NETWORK_REQUEST_FAILED:
throw new NetworkRequestFailed();
}
Expand Down Expand Up @@ -77,6 +79,8 @@ export function useMicrosoftSignIn({
throw new AccountExistsWithDifferentCredential();
case AuthErrorCodes.POPUP_BLOCKED:
throw new PopupBlockedByClient();
case AuthErrorCodes.INVALID_IDP_RESPONSE:
throw new InvalidLoginCredentials();
case AuthErrorCodes.NETWORK_REQUEST_FAILED:
throw new NetworkRequestFailed();
}
Expand Down
18 changes: 13 additions & 5 deletions packages/app-builder/src/services/auth/auth.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ import { type WebhookRepository } from '@app-builder/repositories/WebhookReposit
import { getServerEnv } from '@app-builder/utils/environment';
import { parseForm } from '@app-builder/utils/input-validation';
import { json, redirect } from '@remix-run/node';
import * as Sentry from '@sentry/remix';
import { marblecoreApi } from 'marble-api';
import { type CSRF } from 'remix-utils/csrf/server';
import { type CSRF, CSRFError } from 'remix-utils/csrf/server';
import * as z from 'zod';

import { getRoute } from '../../utils/routes';
import { captureUnexpectedRemixError } from '../monitoring';
import { type SessionService } from './session.server';

interface AuthenticatedInfo {
Expand Down Expand Up @@ -156,6 +156,10 @@ interface MakeAuthenticationServerServiceArgs {
csrfService: CSRF;
}

function expectedErrors(error: unknown) {
return error instanceof CSRFError || error instanceof z.ZodError;
}

export function makeAuthenticationServerService({
getMarbleCoreAPIClientWithAuth,
getTransfercheckAPIClientWithAuth,
Expand Down Expand Up @@ -221,10 +225,12 @@ export function makeAuthenticationServerService({
authSession.set('user', user);
redirectUrl = options.successRedirect;
} catch (error) {
Sentry.captureException(error);
authSession.flash('authError', { message: adaptAuthErrors(error) });

redirectUrl = options.failureRedirect;

if (!expectedErrors(error)) {
captureUnexpectedRemixError(error, 'auth.server@authenticate', request);
}
}

throw redirect(redirectUrl, {
Expand Down Expand Up @@ -284,7 +290,9 @@ export function makeAuthenticationServerService({
},
);
} catch (error) {
Sentry.captureException(error);
if (!expectedErrors(error)) {
captureUnexpectedRemixError(error, 'auth.server@refresh', request);
}
throw redirect(options.failureRedirect);
}
}
Expand Down
26 changes: 26 additions & 0 deletions packages/app-builder/src/services/monitoring.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import {
isForbiddenHttpError,
isNotFoundHttpError,
isUnauthorizedHttpError,
} from '@app-builder/models';
import * as Sentry from '@sentry/remix';

export function captureUnexpectedRemixError(
error: unknown,
name: string,
request: Request,
) {
if (
isUnauthorizedHttpError(error) ||
isForbiddenHttpError(error) ||
isNotFoundHttpError(error)
) {
return;
}
if (error instanceof Error) {
void Sentry.captureRemixServerException(error, name, request);
} else {
// Optionally capture non-Error objects
Sentry.captureException(error);
}
}

0 comments on commit 4477dcf

Please sign in to comment.