From 8f6ea8abbc658e3e21898d64617f73c87d203411 Mon Sep 17 00:00:00 2001 From: Leonard Techel Date: Wed, 29 Dec 2021 18:18:48 +0100 Subject: [PATCH] feat: Reset game when it is unknown, improve error handling --- app/src/app/App.tsx | 9 ++- app/src/app/SWRProvider.tsx | 31 ++++++++++ app/src/app/pages/CodePage.tsx | 6 +- app/src/common/hooks/api/helper/ApiError.ts | 13 ++++- .../api/helper/errorAwareFetcher.test.ts | 4 +- .../hooks/api/useHandleApiError.test.tsx | 56 ++++++++++++++++--- app/src/common/hooks/api/useHandleApiError.ts | 56 ++++++++++++++----- app/src/mocks/data/code-not-found.json | 2 +- app/src/mocks/data/game-not-found.json | 2 +- backend/dpt_app/trails/urls.py | 2 +- backend/dpt_app/trails/views.py | 4 +- docs/ABC-DPT.v1.yaml | 8 +-- 12 files changed, 150 insertions(+), 43 deletions(-) create mode 100644 app/src/app/SWRProvider.tsx diff --git a/app/src/app/App.tsx b/app/src/app/App.tsx index cc9f1dc..383f78e 100644 --- a/app/src/app/App.tsx +++ b/app/src/app/App.tsx @@ -9,6 +9,7 @@ import theme from "./theme"; import AppRouter from "./AppRouter"; import NotificationProvider from "./NotificationProvider"; import PwaReloadNotification from "./PwaReloadNotification"; +import SWRProvider from "./SWRProvider"; const App = () => { return ( @@ -17,9 +18,11 @@ const App = () => { {/* @ts-expect-error */} - - - + + + + + diff --git a/app/src/app/SWRProvider.tsx b/app/src/app/SWRProvider.tsx new file mode 100644 index 0000000..cb4d5fa --- /dev/null +++ b/app/src/app/SWRProvider.tsx @@ -0,0 +1,31 @@ +import { SWRConfig } from "swr"; +import * as React from "react"; + +import ApiError from "../common/hooks/api/helper/ApiError"; +import useHandleApiError from "../common/hooks/api/useHandleApiError"; + +type SWRProviderProps = { + children: React.ReactNode; +}; + +/** + * A provider that enables global error handling for SWR hooks + */ +const SWRProvider = ({ children }: SWRProviderProps) => { + const handleApiError = useHandleApiError(); + + const onError = React.useCallback( + (e: Error) => { + if (!(e instanceof ApiError)) { + return; + } + + handleApiError(e); + }, + [handleApiError] + ); + + return {children}; +}; + +export default SWRProvider; diff --git a/app/src/app/pages/CodePage.tsx b/app/src/app/pages/CodePage.tsx index 4bb33a5..30d9940 100644 --- a/app/src/app/pages/CodePage.tsx +++ b/app/src/app/pages/CodePage.tsx @@ -16,7 +16,6 @@ import ButtonProgressIndicator from "../../common/components/ButtonProgressIndic import Code from "../../features/code/Code"; import StickyActionButtons from "../../common/components/StickyActionButtons"; import useCode from "../../common/hooks/api/useCode"; -import useHandleApiError from "../../common/hooks/api/useHandleApiError"; import useSubmitCode from "../../common/hooks/api/useSubmitCode"; type CodePageProps = { @@ -24,15 +23,12 @@ type CodePageProps = { }; const CodePage = ({ codeId }: CodePageProps) => { - const { data, error } = useCode(codeId); + const { data } = useCode(codeId); const intl = useIntl(); const [isLoading, submitCode] = useSubmitCode(codeId); const code = data?.data; - const handleError = useHandleApiError(); - React.useEffect(() => handleError(error), [handleError, error]); - const onSubmit = () => { if (!code) { return; diff --git a/app/src/common/hooks/api/helper/ApiError.ts b/app/src/common/hooks/api/helper/ApiError.ts index 5f18c6d..7d81e4a 100644 --- a/app/src/common/hooks/api/helper/ApiError.ts +++ b/app/src/common/hooks/api/helper/ApiError.ts @@ -1,10 +1,17 @@ -interface ApiErrorResponseError { - id: string; +export type ApiErrorResponseTypes = + | "already-used" + | "code-not-found" + | "game-not-found" + | "not-authorised" + | "unknown-error"; + +export interface ApiErrorResponseError { + id: ApiErrorResponseTypes; status: number; title: string; } -interface ApiErrorResponse { +export interface ApiErrorResponse { errors: ApiErrorResponseError[]; } diff --git a/app/src/common/hooks/api/helper/errorAwareFetcher.test.ts b/app/src/common/hooks/api/helper/errorAwareFetcher.test.ts index 245e5ec..0925929 100644 --- a/app/src/common/hooks/api/helper/errorAwareFetcher.test.ts +++ b/app/src/common/hooks/api/helper/errorAwareFetcher.test.ts @@ -1,8 +1,8 @@ -import ApiError from "./ApiError"; +import ApiError, { ApiErrorResponse } from "./ApiError"; import errorAwareFetcher from "./errorAwareFetcher"; it("throws an error when the response is not ok", async () => { - const mockError = { + const mockError: ApiErrorResponse = { errors: [ { id: "unknown-error", diff --git a/app/src/common/hooks/api/useHandleApiError.test.tsx b/app/src/common/hooks/api/useHandleApiError.test.tsx index 46c544a..9f838aa 100644 --- a/app/src/common/hooks/api/useHandleApiError.test.tsx +++ b/app/src/common/hooks/api/useHandleApiError.test.tsx @@ -3,6 +3,8 @@ import * as React from "react"; import render from "../../testing/render"; +import * as useCharacter from "../useCharacter"; +import * as useGameId from "../useGameId"; import ApiError from "./helper/ApiError"; import useHandleApiError from "./useHandleApiError"; @@ -11,10 +13,15 @@ const error = new ApiError("test error"); const errorWithInfo = new ApiError("test error", 400, { errors: [ { - id: "test-error", + id: "unknown-error", status: 400, title: "test error 123", }, + { + id: "unknown-error", + status: 500, + title: "other test error 1234", + }, ], }); @@ -38,15 +45,50 @@ const TestComponent = ({ error }: TestComponentProps) => { it("enqueues an error snack with a title", async () => { render(); - await waitFor(() => screen.getByRole("alert")); - expect(screen.getByRole("alert")).toHaveTextContent("test error 123"); + await waitFor(() => { + const errors = screen.getAllByRole("alert"); + expect(errors[0]).toHaveTextContent("test error 123"); + expect(errors[1]).toHaveTextContent("other test error 1234"); + }); }); it("enqueues an error with a default error message when no info is available", async () => { render(); - await waitFor(() => screen.getByRole("alert")); - expect(screen.getByRole("alert")).toHaveTextContent( - "Ein Fehler ist aufgetreten!" - ); + await waitFor(() => { + expect(screen.getByRole("alert")).toHaveTextContent( + "Ein Fehler ist aufgetreten!" + ); + }); +}); + +it("resets the game when a game-not-found error appears", async () => { + const deleteCharacter = jest.fn(); + jest + .spyOn(useCharacter, "default") + .mockReturnValue(["", () => {}, deleteCharacter]); + + const deleteGameId = jest.fn(); + jest + .spyOn(useGameId, "default") + .mockReturnValue(["", () => {}, deleteGameId]); + + const error = new ApiError("test error", 404, { + errors: [ + { + id: "game-not-found", + status: 404, + title: "Unknown game", + }, + ], + }); + + render(); + + await waitFor(() => { + expect(screen.getByRole("alert")).toHaveTextContent("Unknown game"); + }); + + expect(deleteCharacter).toHaveBeenCalled(); + expect(deleteGameId).toHaveBeenCalled(); }); diff --git a/app/src/common/hooks/api/useHandleApiError.ts b/app/src/common/hooks/api/useHandleApiError.ts index 3afd4b9..c4b05d0 100644 --- a/app/src/common/hooks/api/useHandleApiError.ts +++ b/app/src/common/hooks/api/useHandleApiError.ts @@ -1,8 +1,11 @@ import { useIntl } from "react-intl"; import { useLocation } from "wouter"; import { useSnackbar } from "notistack"; +import * as React from "react"; import ApiError from "./helper/ApiError"; +import useCharacter from "../useCharacter"; +import useGameId from "../useGameId"; /** * A React hook that provides error handling for the ApiError @@ -10,29 +13,54 @@ import ApiError from "./helper/ApiError"; * @returns A method that handles an API error */ const useHandleApiError = () => { + const [, , deleteCharacter] = useCharacter(); + const [, , deleteGameId] = useGameId(); const [, setLocation] = useLocation(); const { enqueueSnackbar } = useSnackbar(); const intl = useIntl(); - return (error?: ApiError) => { - if (!error) { - return; - } + return React.useCallback( + (error?: ApiError) => { + if (!error) { + return; + } - const title = error.info?.errors[0].title; - - enqueueSnackbar( - title - ? title - : intl.formatMessage({ + if (!error.info || error.info.errors.length === 0) { + /* Show general error when no error info is set. This case only + * happens when the request completely failed, e.g. by a + * network error. + */ + enqueueSnackbar( + intl.formatMessage({ defaultMessage: "Ein Fehler ist aufgetreten!", description: "unknown error snack", }), - { variant: "error" } - ); + { variant: "error" } + ); + + setLocation("/"); + + return; + } + + const { errors } = error.info; + for (let i = 0; i < errors.length; i += 1) { + const err = errors[i]; + + /* Reset game when the game ID is unknown and redirect to the + * join game start page. + */ + if (err.id === "game-not-found") { + deleteCharacter(); + deleteGameId(); + } - setLocation("/"); - }; + enqueueSnackbar(err.title, { variant: "error" }); + setLocation("/"); + } + }, + [deleteCharacter, deleteGameId, enqueueSnackbar, intl, setLocation] + ); }; export default useHandleApiError; diff --git a/app/src/mocks/data/code-not-found.json b/app/src/mocks/data/code-not-found.json index ab349bf..a492bf5 100644 --- a/app/src/mocks/data/code-not-found.json +++ b/app/src/mocks/data/code-not-found.json @@ -1,7 +1,7 @@ { "errors": [ { - "id": "not-found", + "id": "code-not-found", "status": 404, "title": "Unbekannter QR-Code." } diff --git a/app/src/mocks/data/game-not-found.json b/app/src/mocks/data/game-not-found.json index 35b6e5b..1f3b388 100644 --- a/app/src/mocks/data/game-not-found.json +++ b/app/src/mocks/data/game-not-found.json @@ -1,7 +1,7 @@ { "errors": [ { - "id": "not-found", + "id": "game-not-found", "status": 404, "title": "Unknown game" } diff --git a/backend/dpt_app/trails/urls.py b/backend/dpt_app/trails/urls.py index 85bc306..fde91bc 100644 --- a/backend/dpt_app/trails/urls.py +++ b/backend/dpt_app/trails/urls.py @@ -4,7 +4,7 @@ urlpatterns = [ path('', views.index, name='index'), - path('games//', views.gameManifest, name='gameManifest'), + path('games/', views.gameManifest, name='gameManifest'), path('games//clock', views.clock, name='clock'), path('games//parameters', views.parameter, name='parameter'), path('games//codes/', views.code, name='code'), diff --git a/backend/dpt_app/trails/views.py b/backend/dpt_app/trails/views.py index aec192c..15ec7b9 100644 --- a/backend/dpt_app/trails/views.py +++ b/backend/dpt_app/trails/views.py @@ -49,7 +49,7 @@ def inner(request, gameId, *args, **kwargs): except Game.DoesNotExist: return JsonResponse({"errors": [ { - "id": "not-found", + "id": "game-not-found", "status": 404, "title": "Unknown Game ID" } @@ -149,7 +149,7 @@ def has_valid_bearer(bearer, game): except Code.DoesNotExist: return JsonResponse({"errors": [ { - "id": "not-found", + "id": "code-not-found", "status": 404, "title": "Unknown QR code ID" } diff --git a/docs/ABC-DPT.v1.yaml b/docs/ABC-DPT.v1.yaml index 76127fa..b5c4be0 100644 --- a/docs/ABC-DPT.v1.yaml +++ b/docs/ABC-DPT.v1.yaml @@ -153,10 +153,10 @@ paths: items: $ref: "#/components/schemas/Error" examples: - not-found: + code-not-found: value: errors: - - id: not-found + - id: code-not-found status: 404 title: Unknown QR code ID description: Get the actions represented by a QR code id. @@ -400,10 +400,10 @@ paths: items: $ref: "#/components/schemas/Error" examples: - not-found: + game-not-found: value: errors: - - id: not-found + - id: game-not-found status: 404 title: Unknown game operationId: get-game