Skip to content

Commit

Permalink
Create a generic error handling mechanism (#10865)
Browse files Browse the repository at this point in the history
* Implement a generic error handling mechanism

* Cleanup ToastContainer settings

* Fix query client instance & unit tests

* Add unit tests for react-query callbacks and error boundaries

* Fix render error test to pass CodeQL

* Replace query client instances with query client configurations

* Add TODO comment

* Fixes after cr

* Revert useMemo to useState
  • Loading branch information
mlqn authored Aug 29, 2023
1 parent fc09b42 commit 8220c9e
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 20 deletions.
8 changes: 4 additions & 4 deletions frontend/dashboard/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { initReactI18next } from 'react-i18next';
import nb from '../language/src/nb.json';
import en from '../language/src/en.json';
import { DEFAULT_LANGUAGE } from 'app-shared/constants';
import { QueryClient } from '@tanstack/react-query';
import { QueryClientConfig } from '@tanstack/react-query';
import { ServicesContextProvider } from 'app-shared/contexts/ServicesContext';
import * as queries from 'app-shared/api/queries';
import * as mutations from 'app-shared/api/mutations';
Expand All @@ -26,18 +26,18 @@ i18next.use(initReactI18next).init({
const container = document.getElementById('root');
const root = createRoot(container);

const queryClient = new QueryClient({
const queryClientConfig: QueryClientConfig = {
defaultOptions: {
queries: {
retry: false,
refetchOnWindowFocus: false,
},
},
});
};

root.render(
<BrowserRouter basename={DASHBOARD_BASENAME}>
<ServicesContextProvider client={queryClient} {...queries} {...mutations}>
<ServicesContextProvider clientConfig={queryClientConfig} {...queries} {...mutations}>
<App />
</ServicesContextProvider>
</BrowserRouter>
Expand Down
2 changes: 2 additions & 0 deletions frontend/language/src/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@
"general.disabled": "Disabled",
"general.edit": "Edit",
"general.enabled": "Enabled",
"general.error_message": "An error has occurred! If the problem persists, contact us on <a href=\"https://altinn.slack.com\">Slack</a>.",
"general.error_message_with_colon": "Error message:",
"general.fetch_error_message": "Failed to load required data for this page. Please reload the page to try again",
"general.fetch_error_title": "An error occurred while loading",
Expand Down Expand Up @@ -270,6 +271,7 @@
"general.sign_out": "Sign out",
"general.submit": "Submit",
"general.text": "Text",
"general.try_again": "Try again",
"general.unknown_error": "Unknown error ocurred while loading data.",
"general.validate_changes": "Validate changes",
"general.value": "Value",
Expand Down
2 changes: 2 additions & 0 deletions frontend/language/src/nb.json
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
"general.disabled": "Deaktivert",
"general.edit": "Endre",
"general.enabled": "Aktivert",
"general.error_message": "Det har oppstått en feil! Om problemet vedvarer, ta kontakt med oss på <a href=\"https://altinn.slack.com\">Slack</a>.",
"general.error_message_with_colon": "Feilmelding:",
"general.fetch_error_message": "Kunne ikke laste inn nødvendig data for denne siden. Last siden på nytt for å prøve igjen.",
"general.fetch_error_title": "En feil oppstod ved innlasting av",
Expand Down Expand Up @@ -296,6 +297,7 @@
"general.sign_out": "Logg ut",
"general.submit": "Send inn",
"general.text": "Tekst",
"general.try_again": "Prøv igjen",
"general.unknown_error": "Ukjent feil oppstod ved innlasting av data.",
"general.validate_changes": "Valider endringer",
"general.value": "Verdi",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.container {
padding: var(--fds-spacing-10);
width: auto;
}

.alert > [class*='Alert-module_content'] {
display: flex;
gap: 1rem;
}
29 changes: 29 additions & 0 deletions frontend/packages/shared/src/components/ErrorBoundaryFallback.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React from 'react';
import { useErrorBoundary } from 'react-error-boundary';
import { useTranslation } from 'react-i18next';
import { _useIsProdHack } from 'app-shared/utils/_useIsProdHack';
import { Trans } from 'react-i18next';
import { Alert, Button, ErrorMessage, Link, Paragraph } from '@digdir/design-system-react';
import { Center } from './Center';
import classes from './ErrorBoundaryFallback.module.css';

export type ErrorBoundaryFallbackProps = {
error: Error;
}

export const ErrorBoundaryFallback = ({ error }: ErrorBoundaryFallbackProps) => {
const { t } = useTranslation();
const { resetBoundary } = useErrorBoundary();

return (
<Center className={classes.container}>
<Alert severity='danger' className={classes.alert}>
<Paragraph><Trans i18nKey={'general.error_message'} components={{ a: <Link>Slack</Link> }}/></Paragraph>
{!_useIsProdHack && <ErrorMessage>{error.message}</ErrorMessage>}
<Center>
<Button onClick={resetBoundary} size='small'>{t('general.try_again')}</Button>
</Center>
</Alert>
</Center>
);
};
35 changes: 35 additions & 0 deletions frontend/packages/shared/src/contexts/ServicesContext.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import React from 'react';
import { render, renderHook, screen, waitFor } from '@testing-library/react';
import { ServicesContextProvider } from './ServicesContext';
import { queriesMock } from 'app-shared/mocks/queriesMock';
import { useQuery } from '@tanstack/react-query';
import { textMock } from '../../../../testing/mocks/i18nMock';

const wrapper = ({ children }) => (
<ServicesContextProvider {...queriesMock}>
{children}
</ServicesContextProvider>
);

describe('ServicesContext', () => {
it('displays a default error message if an API call fails', async () => {
const mockConsoleError = jest.spyOn(console, 'error').mockImplementation();
const { result } = renderHook(() => useQuery(['fetchData'],() => Promise.reject(), { retry: false }), { wrapper });

await waitFor(() => result.current.isError);

expect(await screen.findByText(textMock('general.error_message'))).toBeInTheDocument();
expect(mockConsoleError).toHaveBeenCalled();
});

it('displays a default error message if a component throws an error while rendering', () => {
const mockConsoleError = jest.spyOn(console, 'error').mockImplementation();

const ErrorComponent = () => { throw new Error('Intentional render error'); };
render(<ErrorComponent />, { wrapper });

expect(screen.getByText(textMock('general.error_message'))).toBeInTheDocument();
expect(screen.getByText(textMock('general.try_again'))).toBeInTheDocument();
expect(mockConsoleError).toHaveBeenCalled();
});
});
82 changes: 74 additions & 8 deletions frontend/packages/shared/src/contexts/ServicesContext.tsx
Original file line number Diff line number Diff line change
@@ -1,27 +1,93 @@
import React, { createContext, useContext, ReactNode } from 'react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import React, { createContext, useContext, ReactNode, useState } from 'react';
import {
MutationCache,
MutationMeta,
QueryCache,
QueryClient,
QueryClientConfig,
QueryClientProvider,
QueryMeta,
} from '@tanstack/react-query';
import type * as queries from '../api/queries';
import type * as mutations from '../api/mutations';
import { ReactQueryDevtools } from '@tanstack/react-query-devtools';
import { ToastContainer, Slide, toast } from 'react-toastify';
import { ErrorBoundary } from 'react-error-boundary';
import { AxiosError } from 'axios';
import { Trans, useTranslation } from 'react-i18next';
import { Link } from '@digdir/design-system-react';
import { ErrorBoundaryFallback } from '../components/ErrorBoundaryFallback';

import 'react-toastify/dist/ReactToastify.css';
import 'app-shared/styles/toast.css';

export type ServicesContextProps = typeof queries & typeof mutations;
export type ServicesContextProviderProps = ServicesContextProps & {
children?: ReactNode;
client?: QueryClient;
client?: QueryClient; // TODO : #10913 should probably be removed to force the use of QueryCache and MutationCache
clientConfig?: QueryClientConfig;
};

const ServicesContext = createContext<ServicesContextProps>(null);
const queryClient = new QueryClient();

const handleError = (
error: AxiosError,
t: (key: string) => string,
meta: QueryMeta | MutationMeta,
): void => {
// TODO : log axios errors
// TODO : handle messages from API
// TODO : logout user when session is expired

if (
meta?.hideDefaultError === true ||
(meta?.hideDefaultError instanceof Function && meta?.hideDefaultError?.(error))
)
return;

toast.error(() => (
<Trans i18nKey={'general.error_message'} components={{ a: <Link inverted={true}>Slack</Link> }}/>
), { toastId: 'default' });
};

export const ServicesContextProvider = ({
children,
client,
clientConfig,
...queries
}: ServicesContextProviderProps) => {
const { t } = useTranslation();

const [queryClient] = useState(
() => client || new QueryClient({
...clientConfig,
queryCache: new QueryCache({
onError: (error: AxiosError, query) => handleError(error, t, query.options?.meta),
}),
mutationCache: new MutationCache({
onError: (error: AxiosError, variables, context, mutation) => handleError(error, t, mutation.options?.meta),
}),
})
);

return (
<QueryClientProvider client={client || queryClient}>
<ServicesContext.Provider value={{ ...queries }}>{children}</ServicesContext.Provider>
<ReactQueryDevtools initialIsOpen={false} />
</QueryClientProvider>
<ErrorBoundary
FallbackComponent={ErrorBoundaryFallback}
onError={() => {
// TODO : log rendering errors
}}
>
<ToastContainer
position='top-center'
autoClose={5000}
theme='colored'
transition={Slide}
/>
<QueryClientProvider client={queryClient}>
<ServicesContext.Provider value={{ ...queries }}>{children}</ServicesContext.Provider>
<ReactQueryDevtools initialIsOpen={false} />
</QueryClientProvider>
</ErrorBoundary>
);
};

Expand Down
12 changes: 12 additions & 0 deletions frontend/packages/shared/src/styles/toast.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
:root {
--toastify-color-info: var(--fds-semantic-surface-action-primary-default);
--toastify-color-success: var(--fds-semantic-surface-success-default);
--toastify-color-warning: var(--fds-semantic-surface-warning-default);
--toastify-color-error: var(--fds-semantic-surface-danger-default);
--toastify-toast-width: 400px;
}

.Toastify__toast {
line-height: 1.5rem;
padding: var(--fds-spacing-3) var(--fds-spacing-3) var(--fds-spacing-4) var(--fds-spacing-3);
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ describe('TextResourceEdit', () => {
});

it('Does not render anything if edit id is undefined', async () => {
const { renderResult } = await render();
expect(renderResult.container).toBeEmptyDOMElement();
await render();
expect(screen.queryByText(legendText)).not.toBeInTheDocument();
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
expect(screen.queryByRole('button', { name: closeText })).not.toBeInTheDocument();
});

it('Renders correctly when a valid edit id is given', async () => {
Expand Down
8 changes: 4 additions & 4 deletions frontend/resourceadm/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import nb from '../language/src/nb.json';
import en from '../language/src/en.json';
import { DEFAULT_LANGUAGE } from 'app-shared/constants';

import { QueryClient } from '@tanstack/react-query';
import { QueryClientConfig } from '@tanstack/react-query';
import { ServicesContextProvider } from 'app-shared/contexts/ServicesContext';
import * as queries from 'app-shared/api/queries';
import * as mutations from 'app-shared/api/mutations';
Expand All @@ -27,19 +27,19 @@ i18next.use(initReactI18next).init({
const container = document.getElementById('root');
const root = createRoot(container);

const queryClient = new QueryClient({
const queryClientConfig: QueryClientConfig = {
defaultOptions: {
queries: {
retry: false,
staleTime: 10 * 60 * 1000,
refetchOnWindowFocus: false,
},
},
});
};

root.render(
<BrowserRouter basename={RESOURCEADM_BASENAME}>
<ServicesContextProvider client={queryClient} {...queries} {...mutations}>
<ServicesContextProvider clientConfig={queryClientConfig} {...queries} {...mutations}>
<App />
</ServicesContextProvider>
</BrowserRouter>
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
"@tanstack/react-query-devtools": "4.33.0",
"ajv": "8.12.0",
"ajv-formats": "2.1.1",
"react-error-boundary": "^4.0.11",
"react-i18next": "13.1.2",
"react-router-dom": "6.15.0"
"react-router-dom": "6.15.0",
"react-toastify": "^9.1.3"
},
"devDependencies": {
"@emotion/react": "11.11.1",
Expand Down
27 changes: 26 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5126,8 +5126,10 @@ __metadata:
prettier: 2.8.8
react: 18.2.0
react-dom: 18.2.0
react-error-boundary: ^4.0.11
react-i18next: 13.1.2
react-router-dom: 6.15.0
react-toastify: ^9.1.3
redux-mock-store: 1.5.4
redux-saga: 1.2.3
redux-saga-test-plan: 4.0.6
Expand Down Expand Up @@ -6368,7 +6370,7 @@ __metadata:
languageName: node
linkType: hard

"clsx@npm:^1.2.1":
"clsx@npm:^1.1.1, clsx@npm:^1.2.1":
version: 1.2.1
resolution: "clsx@npm:1.2.1"
checksum: 30befca8019b2eb7dbad38cff6266cf543091dae2825c856a62a8ccf2c3ab9c2907c4d12b288b73101196767f66812365400a227581484a05f968b0307cfaf12
Expand Down Expand Up @@ -13153,6 +13155,17 @@ __metadata:
languageName: node
linkType: hard

"react-error-boundary@npm:^4.0.11":
version: 4.0.11
resolution: "react-error-boundary@npm:4.0.11"
dependencies:
"@babel/runtime": ^7.12.5
peerDependencies:
react: ">=16.13.1"
checksum: b3c157fea4e8f78411e9aa0fbf5241f6907b66ede1cd8b7bb22faaeb0339ebeb3dc8e63bf90ef3f740bfa8fd994ca6edf975089cd371b664ad6c2735e7512d38
languageName: node
linkType: hard

"react-i18next@npm:13.1.2":
version: 13.1.2
resolution: "react-i18next@npm:13.1.2"
Expand Down Expand Up @@ -13367,6 +13380,18 @@ __metadata:
languageName: node
linkType: hard

"react-toastify@npm:^9.1.3":
version: 9.1.3
resolution: "react-toastify@npm:9.1.3"
dependencies:
clsx: ^1.1.1
peerDependencies:
react: ">=16"
react-dom: ">=16"
checksum: e8bd92c5cbf831b43a042644ab9bc69abe6ceb3ce91ba71f5cd2d8b6a2c9885ca52770e1f1ba64c5632607f6df962db344a26c7fba57606faf5aa0e7bfc8535f
languageName: node
linkType: hard

"react-transition-group@npm:^4.3.0, react-transition-group@npm:^4.4.5":
version: 4.4.5
resolution: "react-transition-group@npm:4.4.5"
Expand Down

0 comments on commit 8220c9e

Please sign in to comment.