From 709b8fc82e80c1482ecaa90478ddaeec18603335 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Tue, 1 Oct 2024 09:29:10 -0400 Subject: [PATCH 01/10] feat: error handling UI page --- src/router/routes.tsx | 4 +-- src/sections/error-page/ErrorPage.module.scss | 30 ++++++++++++++++++ src/sections/error-page/ErrorPage.tsx | 31 +++++++++++++++++++ src/sections/error-page/useErrorLogger.tsx | 21 +++++++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 src/sections/error-page/ErrorPage.module.scss create mode 100644 src/sections/error-page/ErrorPage.tsx create mode 100644 src/sections/error-page/useErrorLogger.tsx diff --git a/src/router/routes.tsx b/src/router/routes.tsx index b71090f75..1b00a552c 100644 --- a/src/router/routes.tsx +++ b/src/router/routes.tsx @@ -1,7 +1,6 @@ import { RouteObject } from 'react-router-dom' import { Route } from '../sections/Route.enum' import { Layout } from '../sections/layout/Layout' -import { PageNotFound } from '../sections/page-not-found/PageNotFound' import { DatasetFactory } from '../sections/dataset/DatasetFactory' import { CreateDatasetFactory } from '../sections/create-dataset/CreateDatasetFactory' import { FileFactory } from '../sections/file/FileFactory' @@ -12,12 +11,13 @@ import { CreateCollectionFactory } from '../sections/create-collection/CreateCol import { AccountFactory } from '../sections/account/AccountFactory' import { ProtectedRoute } from './ProtectedRoute' import { HomepageFactory } from '../sections/homepage/HomepageFactory' +import { ErrorPage } from '../sections/error-page/ErrorPage' export const routes: RouteObject[] = [ { path: '/', element: , - errorElement: , + errorElement: , children: [ { path: Route.HOME, diff --git a/src/sections/error-page/ErrorPage.module.scss b/src/sections/error-page/ErrorPage.module.scss new file mode 100644 index 000000000..842762fde --- /dev/null +++ b/src/sections/error-page/ErrorPage.module.scss @@ -0,0 +1,30 @@ +@use 'sass:color'; +@import 'node_modules/@iqss/dataverse-design-system/src/lib/assets/styles/design-tokens/colors.module'; +@import 'src/assets/variables'; + +.section-wrapper { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + min-height: $main-container-available-height; +} + +.middle-search-cta-wrapper { + display: flex; + flex-direction: column; + gap: 2rem; + align-items: center; + justify-content: center; + width: 100%; + padding-block: 2rem; +} + +.icon-layout { + display: flex; + gap: 1rem; + align-items: center; + justify-content: center; + width: 100%; + padding-block: 2rem; +} diff --git a/src/sections/error-page/ErrorPage.tsx b/src/sections/error-page/ErrorPage.tsx new file mode 100644 index 000000000..a91731e2c --- /dev/null +++ b/src/sections/error-page/ErrorPage.tsx @@ -0,0 +1,31 @@ +import { useTranslation } from 'react-i18next' +import { useRouteError, Link } from 'react-router-dom' +import styles from './ErrorPage.module.scss' +import { useErrorLogger } from './useErrorLogger' +import { ExclamationCircle } from 'react-bootstrap-icons' +import { useTheme } from '@iqss/dataverse-design-system' + +export function ErrorPage() { + const { t } = useTranslation('ErrorPage') + const error = useRouteError() + useErrorLogger(error) + const theme = useTheme() + + return ( +
+
+
+ +
+

Oops,

+

something went wrong...

+
+
+ + + {t('Back to root collection')} + +
+
+ ) +} diff --git a/src/sections/error-page/useErrorLogger.tsx b/src/sections/error-page/useErrorLogger.tsx new file mode 100644 index 000000000..5ce4bcb36 --- /dev/null +++ b/src/sections/error-page/useErrorLogger.tsx @@ -0,0 +1,21 @@ +import { useCallback, useEffect } from 'react' + +const loggedErrors = new Set() + +export function useErrorLogger(error: Error | unknown): (error: Error) => void { + const logErrorOnce = useCallback((err: Error): void => { + const errorString = String(err) + if (!loggedErrors.has(errorString)) { + loggedErrors.add(errorString) + console.error('Error:', err.message) + } + }, []) + + useEffect(() => { + if (error) { + logErrorOnce(error as Error) + } + }, [error, logErrorOnce]) + + return logErrorOnce +} From 545bb9b58a2390e772db4ba05ae2b4dfb26e04c3 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Wed, 2 Oct 2024 12:12:10 -0400 Subject: [PATCH 02/10] test: useErrorLogger hook test --- .../error-page/useErrorLogger.spec.tsx | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 tests/component/sections/error-page/useErrorLogger.spec.tsx diff --git a/tests/component/sections/error-page/useErrorLogger.spec.tsx b/tests/component/sections/error-page/useErrorLogger.spec.tsx new file mode 100644 index 000000000..e2ee3011a --- /dev/null +++ b/tests/component/sections/error-page/useErrorLogger.spec.tsx @@ -0,0 +1,77 @@ +import { renderHook } from '@testing-library/react' +import { useErrorLogger } from '../../../../src/sections/error-page/useErrorLogger' + +describe('useErrorLogger', () => { + beforeEach(() => { + cy.window().then((win) => { + cy.stub(win.console, 'error').as('consoleError') + }) + }) + + it('should only log the same errors once when multiple occur, and log different errors seperately', () => { + const error = new Error('Test error') + + cy.window().then(() => { + const { rerender } = renderHook( + ({ error }: { error: Error | null }) => useErrorLogger(error), + { initialProps: { error } } + ) + cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Test error') + + cy.then(() => { + rerender({ error }) + }) + cy.get('@consoleError').should('have.been.calledOnce') + + const newError = new Error('Another error') + cy.then(() => { + rerender({ error: newError }) + }) + + cy.get('@consoleError').should('have.been.calledWith', 'Error:', 'Another error') + }) + }) + + it('should not log anything if no error is provided', () => { + cy.window().then(() => { + const { rerender } = renderHook( + ({ error }: { error: Error | null }) => useErrorLogger(error), + { initialProps: { error: null } } + ) + + cy.get('@consoleError').should('not.have.been.called') + cy.then(() => { + rerender({ error: null }) + }) + cy.get('@consoleError').should('not.have.been.called') + }) + }) + + it('should be used to log errors manually only once for the same error, and log different errors seperately', () => { + cy.window().then(() => { + const { result } = renderHook(() => useErrorLogger(null)) + + cy.then(() => { + result.current(new Error('Manually logged error')) + }) + + cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Manually logged error') + + cy.then(() => { + result.current(new Error('Manually logged error')) + }) + + cy.get('@consoleError').should('have.been.calledOnce') + + cy.then(() => { + result.current(new Error('Another manually logged error')) + }) + + cy.get('@consoleError').should( + 'have.been.calledWith', + 'Error:', + 'Another manually logged error' + ) + }) + }) +}) From 318687770b8db5a36a413a703875a65464a6dfc7 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Wed, 2 Oct 2024 12:16:14 -0400 Subject: [PATCH 03/10] chore: useErrorLogger hook test --- .../component/sections/error-page/useErrorLogger.spec.tsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/component/sections/error-page/useErrorLogger.spec.tsx b/tests/component/sections/error-page/useErrorLogger.spec.tsx index e2ee3011a..6fa25ac94 100644 --- a/tests/component/sections/error-page/useErrorLogger.spec.tsx +++ b/tests/component/sections/error-page/useErrorLogger.spec.tsx @@ -17,7 +17,6 @@ describe('useErrorLogger', () => { { initialProps: { error } } ) cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Test error') - cy.then(() => { rerender({ error }) }) @@ -38,7 +37,6 @@ describe('useErrorLogger', () => { ({ error }: { error: Error | null }) => useErrorLogger(error), { initialProps: { error: null } } ) - cy.get('@consoleError').should('not.have.been.called') cy.then(() => { rerender({ error: null }) @@ -50,23 +48,17 @@ describe('useErrorLogger', () => { it('should be used to log errors manually only once for the same error, and log different errors seperately', () => { cy.window().then(() => { const { result } = renderHook(() => useErrorLogger(null)) - cy.then(() => { result.current(new Error('Manually logged error')) }) - cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Manually logged error') - cy.then(() => { result.current(new Error('Manually logged error')) }) - cy.get('@consoleError').should('have.been.calledOnce') - cy.then(() => { result.current(new Error('Another manually logged error')) }) - cy.get('@consoleError').should( 'have.been.calledWith', 'Error:', From de6b9b754f1a598f05e5cebbc2c4e118c1a18ba7 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Thu, 3 Oct 2024 15:16:27 -0400 Subject: [PATCH 04/10] bug: fix some bugs in useErrorLogger hook test --- src/sections/error-page/useErrorLogger.tsx | 6 +- .../error-page/useErrorLogger.spec.tsx | 73 +++++++++++++------ 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/src/sections/error-page/useErrorLogger.tsx b/src/sections/error-page/useErrorLogger.tsx index 5ce4bcb36..fd163b6db 100644 --- a/src/sections/error-page/useErrorLogger.tsx +++ b/src/sections/error-page/useErrorLogger.tsx @@ -3,11 +3,11 @@ import { useCallback, useEffect } from 'react' const loggedErrors = new Set() export function useErrorLogger(error: Error | unknown): (error: Error) => void { - const logErrorOnce = useCallback((err: Error): void => { - const errorString = String(err) + const logErrorOnce = useCallback((err: Error & { data?: unknown }): void => { + const errorString = String(err.data) if (!loggedErrors.has(errorString)) { loggedErrors.add(errorString) - console.error('Error:', err.message) + console.error('Error:', err) } }, []) diff --git a/tests/component/sections/error-page/useErrorLogger.spec.tsx b/tests/component/sections/error-page/useErrorLogger.spec.tsx index 6fa25ac94..d53a8e01f 100644 --- a/tests/component/sections/error-page/useErrorLogger.spec.tsx +++ b/tests/component/sections/error-page/useErrorLogger.spec.tsx @@ -1,6 +1,14 @@ import { renderHook } from '@testing-library/react' import { useErrorLogger } from '../../../../src/sections/error-page/useErrorLogger' +interface RouterError { + status: number + statusText: string + internal: boolean + data: string + error: Error +} + describe('useErrorLogger', () => { beforeEach(() => { cy.window().then((win) => { @@ -8,33 +16,44 @@ describe('useErrorLogger', () => { }) }) - it('should only log the same errors once when multiple occur, and log different errors seperately', () => { - const error = new Error('Test error') + it('should only log the same errors once when multiple occur, and log different errors separately', () => { + const testError = { + status: 404, + statusText: 'Not Found', + internal: true, + data: 'Test Error: Not Found', + error: new Error('Test Error') + } + + const newError = { + status: 500, + statusText: 'Internal Server Error', + internal: true, + data: 'Error: Another error occurred', + error: new Error('Another error occurred') + } cy.window().then(() => { const { rerender } = renderHook( - ({ error }: { error: Error | null }) => useErrorLogger(error), - { initialProps: { error } } + ({ error }: { error: Error | RouterError | null }) => useErrorLogger(error), + { initialProps: { error: testError } } ) - cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Test error') + cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', testError) cy.then(() => { - rerender({ error }) + rerender({ error: testError }) }) cy.get('@consoleError').should('have.been.calledOnce') - - const newError = new Error('Another error') cy.then(() => { rerender({ error: newError }) }) - - cy.get('@consoleError').should('have.been.calledWith', 'Error:', 'Another error') + cy.get('@consoleError').should('have.been.calledWith', 'Error:', newError) }) }) it('should not log anything if no error is provided', () => { cy.window().then(() => { const { rerender } = renderHook( - ({ error }: { error: Error | null }) => useErrorLogger(error), + ({ error }: { error: Error | RouterError | null }) => useErrorLogger(error), { initialProps: { error: null } } ) cy.get('@consoleError').should('not.have.been.called') @@ -45,25 +64,37 @@ describe('useErrorLogger', () => { }) }) - it('should be used to log errors manually only once for the same error, and log different errors seperately', () => { + it('should be used to log errors manually only once for the same error, and log different errors separately', () => { + const badRequestError = { + status: 400, + statusText: 'Bad Request', + internal: true, + data: 'Error: Manually logged error', + error: new Error('Manually logged error') + } + + const newError = { + status: 500, + statusText: 'Internal Server Error', + internal: true, + data: 'Error: Another error occurred', + error: new Error('Another error occurred') + } + cy.window().then(() => { const { result } = renderHook(() => useErrorLogger(null)) cy.then(() => { - result.current(new Error('Manually logged error')) + return result.current(badRequestError.error) }) - cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', 'Manually logged error') + cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', badRequestError.error) cy.then(() => { - result.current(new Error('Manually logged error')) + result.current(badRequestError.error) }) cy.get('@consoleError').should('have.been.calledOnce') cy.then(() => { - result.current(new Error('Another manually logged error')) + result.current(newError.error) }) - cy.get('@consoleError').should( - 'have.been.calledWith', - 'Error:', - 'Another manually logged error' - ) + cy.get('@consoleError').should('have.been.calledWith', 'Error:', newError.error) }) }) }) From b2159f41a9cfa03b023875b950e18e5eda721a63 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Fri, 4 Oct 2024 10:24:16 -0400 Subject: [PATCH 05/10] feat: add errorElement to the children routes for adding header and footer --- src/router/routes.tsx | 31 ++++++++++++++++++--------- src/sections/error-page/ErrorPage.tsx | 2 +- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/router/routes.tsx b/src/router/routes.tsx index 1b00a552c..79fefd038 100644 --- a/src/router/routes.tsx +++ b/src/router/routes.tsx @@ -21,47 +21,58 @@ export const routes: RouteObject[] = [ children: [ { path: Route.HOME, - element: HomepageFactory.create() + element: HomepageFactory.create(), + errorElement: }, { path: Route.COLLECTIONS_BASE, - element: CollectionFactory.create() + element: CollectionFactory.create(), + errorElement: }, { path: Route.COLLECTIONS, - element: CollectionFactory.create() + element: CollectionFactory.create(), + errorElement: }, { path: Route.DATASETS, - element: DatasetFactory.create() + element: DatasetFactory.create(), + errorElement: }, { path: Route.FILES, - element: FileFactory.create() + element: FileFactory.create(), + errorElement: }, // 🔐 Protected routes are only accessible to authenticated users { element: , + errorElement: , children: [ { path: Route.CREATE_COLLECTION, - element: CreateCollectionFactory.create() + element: CreateCollectionFactory.create(), + errorElement: }, { path: Route.CREATE_DATASET, - element: CreateDatasetFactory.create() + element: CreateDatasetFactory.create(), + errorElement: }, { path: Route.UPLOAD_DATASET_FILES, - element: UploadDatasetFilesFactory.create() + element: UploadDatasetFilesFactory.create(), + errorElement: }, { path: Route.EDIT_DATASET_METADATA, - element: EditDatasetMetadataFactory.create() + element: EditDatasetMetadataFactory.create(), + errorElement: }, { path: Route.ACCOUNT, - element: AccountFactory.create() + element: AccountFactory.create(), + errorElement: } ] } diff --git a/src/sections/error-page/ErrorPage.tsx b/src/sections/error-page/ErrorPage.tsx index a91731e2c..fc953a19a 100644 --- a/src/sections/error-page/ErrorPage.tsx +++ b/src/sections/error-page/ErrorPage.tsx @@ -23,7 +23,7 @@ export function ErrorPage() { - {t('Back to root collection')} + {t('Back to Dataverse Homepage')} From dd21944ba5f936aa7b2a92c151e442f12a39da0b Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Fri, 4 Oct 2024 15:50:52 -0400 Subject: [PATCH 06/10] feat: add text variables to the page --- public/locales/en/errorPage.json | 8 ++++++++ src/sections/error-page/ErrorPage.tsx | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) create mode 100644 public/locales/en/errorPage.json diff --git a/public/locales/en/errorPage.json b/public/locales/en/errorPage.json new file mode 100644 index 000000000..f27e69b9e --- /dev/null +++ b/public/locales/en/errorPage.json @@ -0,0 +1,8 @@ +{ + "message": { + "heading": "Oops,", + "errorText": "something went wrong..." + }, + "brandName": "Dataverse", + "backToHomepage": "Back to {{brandName}} Homepage" +} diff --git a/src/sections/error-page/ErrorPage.tsx b/src/sections/error-page/ErrorPage.tsx index fc953a19a..ee70407b7 100644 --- a/src/sections/error-page/ErrorPage.tsx +++ b/src/sections/error-page/ErrorPage.tsx @@ -6,7 +6,7 @@ import { ExclamationCircle } from 'react-bootstrap-icons' import { useTheme } from '@iqss/dataverse-design-system' export function ErrorPage() { - const { t } = useTranslation('ErrorPage') + const { t } = useTranslation('errorPage') const error = useRouteError() useErrorLogger(error) const theme = useTheme() @@ -17,13 +17,13 @@ export function ErrorPage() {
-

Oops,

-

something went wrong...

+

{t('message.heading')}

+

{t('message.errorText')}

- {t('Back to Dataverse Homepage')} + {t('backToHomepage', { brandName: t('brandName') })} From 1bc573e6b1ed912a68abcbc77fe82400f8f99ef3 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Tue, 8 Oct 2024 18:29:00 -0400 Subject: [PATCH 07/10] fix: adjust min-height of css as a dynamic variable --- src/sections/error-page/ErrorPage.module.scss | 2 +- src/sections/error-page/ErrorPage.tsx | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sections/error-page/ErrorPage.module.scss b/src/sections/error-page/ErrorPage.module.scss index 842762fde..127aaa88b 100644 --- a/src/sections/error-page/ErrorPage.module.scss +++ b/src/sections/error-page/ErrorPage.module.scss @@ -7,7 +7,7 @@ flex-direction: column; align-items: center; justify-content: center; - min-height: $main-container-available-height; + min-height: var(--error-min-height); } .middle-search-cta-wrapper { diff --git a/src/sections/error-page/ErrorPage.tsx b/src/sections/error-page/ErrorPage.tsx index ee70407b7..4967e5b7a 100644 --- a/src/sections/error-page/ErrorPage.tsx +++ b/src/sections/error-page/ErrorPage.tsx @@ -11,6 +11,11 @@ export function ErrorPage() { useErrorLogger(error) const theme = useTheme() + const header = document.querySelector('nav') + const footer = document.querySelector('footer') + const newMinHeight = header && footer ? '$main-container-available-height' : '100vh' + document.documentElement.style.setProperty('--error-min-height', newMinHeight) + return (
From b0857354e57fefca99ae6d0299736472ec97a4d8 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Wed, 9 Oct 2024 21:12:08 -0400 Subject: [PATCH 08/10] fix: adjust min-height of css as a dynamic variable --- src/router/routes.tsx | 2 +- src/sections/error-page/ErrorPage.module.scss | 7 +++++-- src/sections/error-page/ErrorPage.tsx | 19 +++++++++++-------- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/router/routes.tsx b/src/router/routes.tsx index 79fefd038..2ee151ab7 100644 --- a/src/router/routes.tsx +++ b/src/router/routes.tsx @@ -17,7 +17,7 @@ export const routes: RouteObject[] = [ { path: '/', element: , - errorElement: , + errorElement: , children: [ { path: Route.HOME, diff --git a/src/sections/error-page/ErrorPage.module.scss b/src/sections/error-page/ErrorPage.module.scss index 127aaa88b..3fae83ab5 100644 --- a/src/sections/error-page/ErrorPage.module.scss +++ b/src/sections/error-page/ErrorPage.module.scss @@ -7,10 +7,13 @@ flex-direction: column; align-items: center; justify-content: center; - min-height: var(--error-min-height); + min-height: $main-container-available-height; + &.full-viewport { + min-height: 100vh; + } } -.middle-search-cta-wrapper { +.middle-errorMessage-wrapper { display: flex; flex-direction: column; gap: 2rem; diff --git a/src/sections/error-page/ErrorPage.tsx b/src/sections/error-page/ErrorPage.tsx index 4967e5b7a..a28eaafea 100644 --- a/src/sections/error-page/ErrorPage.tsx +++ b/src/sections/error-page/ErrorPage.tsx @@ -4,21 +4,24 @@ import styles from './ErrorPage.module.scss' import { useErrorLogger } from './useErrorLogger' import { ExclamationCircle } from 'react-bootstrap-icons' import { useTheme } from '@iqss/dataverse-design-system' +import cn from 'classnames' -export function ErrorPage() { +interface AppLoaderProps { + fullViewport?: boolean +} + +export function ErrorPage({ fullViewport = false }: AppLoaderProps) { const { t } = useTranslation('errorPage') const error = useRouteError() useErrorLogger(error) const theme = useTheme() - const header = document.querySelector('nav') - const footer = document.querySelector('footer') - const newMinHeight = header && footer ? '$main-container-available-height' : '100vh' - document.documentElement.style.setProperty('--error-min-height', newMinHeight) - return ( -
-
+
+
From d36b1729e60a21a138f47ec47ed6d7c5066871e4 Mon Sep 17 00:00:00 2001 From: Cheng Shi Date: Wed, 9 Oct 2024 21:21:03 -0400 Subject: [PATCH 09/10] chore: add an empty line before rule --- src/sections/error-page/ErrorPage.module.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sections/error-page/ErrorPage.module.scss b/src/sections/error-page/ErrorPage.module.scss index 3fae83ab5..2681008fc 100644 --- a/src/sections/error-page/ErrorPage.module.scss +++ b/src/sections/error-page/ErrorPage.module.scss @@ -8,6 +8,7 @@ align-items: center; justify-content: center; min-height: $main-container-available-height; + &.full-viewport { min-height: 100vh; } From a1bbdbec567ec462926d0c8b26de1a23452d8281 Mon Sep 17 00:00:00 2001 From: German Gonzalo Saracca Date: Mon, 14 Oct 2024 13:18:25 +0200 Subject: [PATCH 10/10] Update src/router/routes.tsx --- src/router/routes.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/router/routes.tsx b/src/router/routes.tsx index ce2b4be56..462fe480c 100644 --- a/src/router/routes.tsx +++ b/src/router/routes.tsx @@ -120,7 +120,6 @@ export const routes: RouteObject[] = [ // 🔐 Protected routes are only accessible to authenticated users { element: , - errorElement: , children: [ { path: Route.CREATE_COLLECTION,