From b5702ada91d49251ab981bcda3e3fac4ff6f7c7b Mon Sep 17 00:00:00 2001 From: Greg Baker Date: Fri, 20 Dec 2024 14:13:46 -0330 Subject: [PATCH] fix(frontend): use CodedError everywhere --- frontend/app/components/app-link.tsx | 4 ++- frontend/app/errors/coded-error.ts | 28 +++++++++++++++++++-- frontend/app/i18n-config.server.ts | 3 ++- frontend/app/routes/auth/callback.tsx | 3 ++- frontend/app/routes/auth/login.tsx | 3 ++- frontend/app/routes/dev/error.tsx | 4 +-- frontend/app/utils/route-utils.ts | 5 ++-- frontend/app/utils/validation-utils.ts | 4 ++- frontend/tests/components/app-link.test.tsx | 3 ++- frontend/tests/utils/route-utils.test.ts | 25 +++++++++++++----- 10 files changed, 64 insertions(+), 18 deletions(-) diff --git a/frontend/app/components/app-link.tsx b/frontend/app/components/app-link.tsx index 777a8ce7..33542192 100644 --- a/frontend/app/components/app-link.tsx +++ b/frontend/app/components/app-link.tsx @@ -4,6 +4,7 @@ import { forwardRef } from 'react'; import type { Params, Path } from 'react-router'; import { generatePath, Link } from 'react-router'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import { useLanguage } from '~/hooks/use-language'; import type { I18nRouteFile } from '~/i18n-routes'; import { i18nRoutes } from '~/i18n-routes'; @@ -27,8 +28,9 @@ export const AppLink = forwardRef, AppLinkProps>( const targetLanguage = lang ?? currentLanguage; if (targetLanguage === undefined) { - throw new Error( + throw new CodedError( 'The `lang` parameter was not provided, and the current language could not be determined from the request', + ErrorCodes.MISSING_LANG_PARAM, ); } diff --git a/frontend/app/errors/coded-error.ts b/frontend/app/errors/coded-error.ts index 2b46ec3a..5afcb07b 100644 --- a/frontend/app/errors/coded-error.ts +++ b/frontend/app/errors/coded-error.ts @@ -1,5 +1,29 @@ import { randomString } from '~/utils/string-utils'; +export type ErrorCode = (typeof ErrorCodes)[keyof typeof ErrorCodes]; + +export const ErrorCodes = { + UNCAUGHT_ERROR: 'UNC-0000', + + // auth error codes + MISCONFIGURED_PROVIDER: 'AUTH-0001', + + // component error codes + MISSING_LANG_PARAM: 'CMP-0001', + + // i18n error codes + NO_LANGUAGE_FOUND: 'I18N-0001', + + // route error codes + ROUTE_NOT_FOUND: 'RTE-0001', + + // validation error codes + INVALID_NUMBER: 'VAL-0001', + + // dev-only error codes + TEST_ERROR_CODE: 'DEV-0001', +} as const; + export type CodedErrorProps = { readonly code: string; readonly correlationId: string; @@ -13,7 +37,7 @@ export type CodedErrorProps = { export class CodedError { public readonly name = 'CodedError'; - public readonly code: string; + public readonly code: ErrorCode; public readonly correlationId: string; public readonly stack?: string; @@ -23,7 +47,7 @@ export class CodedError { // message is supplied to `log.error(message, error)` public readonly msg?: string; - public constructor(msg?: string, code = 'UNC-0000', correlationId = generateCorrelationId()) { + public constructor(msg?: string, code: ErrorCode = ErrorCodes.UNCAUGHT_ERROR, correlationId = generateCorrelationId()) { this.code = code; this.correlationId = correlationId; this.msg = msg; diff --git a/frontend/app/i18n-config.server.ts b/frontend/app/i18n-config.server.ts index c62cbe6d..fa6c2617 100644 --- a/frontend/app/i18n-config.server.ts +++ b/frontend/app/i18n-config.server.ts @@ -4,6 +4,7 @@ import { initReactI18next } from 'react-i18next'; import { serverEnvironment } from '~/.server/environment'; import { i18nResources } from '~/.server/locales'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import { getLanguage } from '~/utils/i18n-utils'; /** @@ -24,7 +25,7 @@ export async function getFixedT( : languageOrRequest; if (language === undefined) { - throw new Error('No language found in request'); + throw new CodedError('No language found in request', ErrorCodes.NO_LANGUAGE_FOUND); } const i18n = await initI18next(language); diff --git a/frontend/app/routes/auth/callback.tsx b/frontend/app/routes/auth/callback.tsx index 999b5a97..5a53450b 100644 --- a/frontend/app/routes/auth/callback.tsx +++ b/frontend/app/routes/auth/callback.tsx @@ -5,6 +5,7 @@ import { Redacted } from 'effect'; import type { Route } from './+types/callback'; import { serverEnvironment } from '~/.server/environment'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import type { AuthenticationStrategy } from '~/utils/auth/authentication-strategy'; import { AzureADAuthenticationStrategy } from '~/utils/auth/azuread-authentication-strategy'; import { LocalAuthenticationStrategy } from '~/utils/auth/local-authentication-strategy'; @@ -32,7 +33,7 @@ export async function loader({ context, params, request }: Route.LoaderArgs) { const AZUREAD_CLIENT_SECRET = Redacted.value(serverEnvironment.AZUREAD_CLIENT_SECRET); if (!AZUREAD_ISSUER_URL || !AZUREAD_CLIENT_ID || !AZUREAD_CLIENT_SECRET) { - throw new Error('The Azure OIDC settings are misconfigured'); + throw new CodedError('The Azure OIDC settings are misconfigured', ErrorCodes.MISCONFIGURED_PROVIDER); } const authStrategy = new AzureADAuthenticationStrategy( diff --git a/frontend/app/routes/auth/login.tsx b/frontend/app/routes/auth/login.tsx index 6d3fdd5e..7dad9ed8 100644 --- a/frontend/app/routes/auth/login.tsx +++ b/frontend/app/routes/auth/login.tsx @@ -6,6 +6,7 @@ import { Redacted } from 'effect'; import type { Route } from './+types/login'; import { serverEnvironment } from '~/.server/environment'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import type { AuthenticationStrategy } from '~/utils/auth/authentication-strategy'; import { AzureADAuthenticationStrategy } from '~/utils/auth/azuread-authentication-strategy'; import { LocalAuthenticationStrategy } from '~/utils/auth/local-authentication-strategy'; @@ -39,7 +40,7 @@ export async function loader({ context, params, request }: Route.LoaderArgs) { const AZUREAD_CLIENT_SECRET = Redacted.value(serverEnvironment.AZUREAD_CLIENT_SECRET); if (!AZUREAD_ISSUER_URL || !AZUREAD_CLIENT_ID || !AZUREAD_CLIENT_SECRET) { - throw new Error('The Azure OIDC settings are misconfigured'); + throw new CodedError('The Azure OIDC settings are misconfigured', ErrorCodes.MISCONFIGURED_PROVIDER); } const authStrategy = new AzureADAuthenticationStrategy( diff --git a/frontend/app/routes/dev/error.tsx b/frontend/app/routes/dev/error.tsx index 1ce036e9..fea1da3c 100644 --- a/frontend/app/routes/dev/error.tsx +++ b/frontend/app/routes/dev/error.tsx @@ -1,8 +1,8 @@ -import { CodedError } from '~/errors/coded-error'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; /** * An error route that can be used to test error boundaries. */ export default function Error() { - throw new CodedError('ERR-0001'); + throw new CodedError('This is a test error', ErrorCodes.TEST_ERROR_CODE); } diff --git a/frontend/app/utils/route-utils.ts b/frontend/app/utils/route-utils.ts index 01b250c9..17a8e015 100644 --- a/frontend/app/utils/route-utils.ts +++ b/frontend/app/utils/route-utils.ts @@ -1,3 +1,4 @@ +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import type { I18nPageRoute, I18nRoute, I18nRouteFile } from '~/i18n-routes'; import { isI18nLayoutRoute, isI18nPageRoute } from '~/i18n-routes'; @@ -59,7 +60,7 @@ export function getRouteByFile(i18nRouteFile: I18nRouteFile, routes: I18nRoute[] const route = findRouteByFile(i18nRouteFile, routes); if (route === undefined) { - throw new Error(`No route found for ${i18nRouteFile} (this should never happen)`); + throw new CodedError(`No route found for ${i18nRouteFile} (this should never happen)`, ErrorCodes.ROUTE_NOT_FOUND); } return route; @@ -77,7 +78,7 @@ export function getRouteByPath(pathname: string, routes: I18nRoute[]): I18nPageR const route = findRouteByPath(pathname, routes); if (route === undefined) { - throw new Error(`No route found for ${pathname} (this should never happen)`); + throw new CodedError(`No route found for ${pathname} (this should never happen)`, ErrorCodes.ROUTE_NOT_FOUND); } return route; diff --git a/frontend/app/utils/validation-utils.ts b/frontend/app/utils/validation-utils.ts index 1d6f6c8b..e07a6d64 100644 --- a/frontend/app/utils/validation-utils.ts +++ b/frontend/app/utils/validation-utils.ts @@ -1,6 +1,8 @@ import { Redacted } from 'effect'; import type { z, ZodEffects, ZodTypeAny } from 'zod'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; + /** * Parses a string to a boolean. */ @@ -16,7 +18,7 @@ export function asNumber(schema: T): ZodEffects const number = Number(val); if (Number.isNaN(number)) { - throw new Error(`Invalid number ${val}`); + throw new CodedError(`Invalid number ${val}`, ErrorCodes.INVALID_NUMBER); } return number; diff --git a/frontend/tests/components/app-link.test.tsx b/frontend/tests/components/app-link.test.tsx index 030ffb61..c5c58251 100644 --- a/frontend/tests/components/app-link.test.tsx +++ b/frontend/tests/components/app-link.test.tsx @@ -4,6 +4,7 @@ import { render } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { AppLink } from '~/components/app-link'; +import type { CodedError } from '~/errors/coded-error'; describe('AppLink', () => { beforeEach(() => { @@ -45,7 +46,7 @@ describe('AppLink', () => { { path: '/', Component: () => This is a test, - ErrorBoundary: () => <>{(useRouteError() as Error).message}, + ErrorBoundary: () => <>{(useRouteError() as CodedError).msg}, }, ]); diff --git a/frontend/tests/utils/route-utils.test.ts b/frontend/tests/utils/route-utils.test.ts index 102dda58..791672b1 100644 --- a/frontend/tests/utils/route-utils.test.ts +++ b/frontend/tests/utils/route-utils.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; +import { CodedError, ErrorCodes } from '~/errors/coded-error'; import type { I18nRouteFile } from '~/i18n-routes'; import { i18nRoutes, isI18nLayoutRoute, isI18nPageRoute } from '~/i18n-routes'; import { findRouteByFile, findRouteByPath, getRouteByFile, getRouteByPath } from '~/utils/route-utils'; @@ -43,9 +44,15 @@ describe('route-utils', () => { }); it('should throw an error if the route is not found', () => { - expect(() => getRouteByFile('routes/💩.tsx' as I18nRouteFile, i18nRoutes)).toThrowError( - 'No route found for routes/💩.tsx (this should never happen)', - ); + try { + getRouteByFile('routes/💩.tsx' as I18nRouteFile, i18nRoutes); + } catch (error) { + expect(error).toBeInstanceOf(CodedError); + + const codedError = error as CodedError; + expect(codedError.msg).toEqual('No route found for routes/💩.tsx (this should never happen)'); + expect(codedError.code).toEqual(ErrorCodes.ROUTE_NOT_FOUND); + } }); }); @@ -59,9 +66,15 @@ describe('route-utils', () => { }); it('should throw an error if the route is not found', () => { - expect(() => getRouteByPath('/en/foobar', i18nRoutes)).toThrowError( - 'No route found for /en/foobar (this should never happen)', - ); + try { + getRouteByPath('/en/foobar', i18nRoutes); + } catch (error) { + expect(error).toBeInstanceOf(CodedError); + + const codedError = error as CodedError; + expect(codedError.msg).toEqual('No route found for /en/foobar (this should never happen)'); + expect(codedError.code).toEqual(ErrorCodes.ROUTE_NOT_FOUND); + } }); });