Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

126 add error handling mechanism to the UI #518

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
8 changes: 8 additions & 0 deletions public/locales/en/errorPage.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"message": {
"heading": "Oops,",
"errorText": "something went wrong..."
},
"brandName": "Dataverse",
"backToHomepage": "Back to {{brandName}} Homepage"
}
35 changes: 23 additions & 12 deletions src/router/routes.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand All @@ -12,56 +11,68 @@ 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: <Layout />,
errorElement: <PageNotFound />,
errorElement: <ErrorPage />,
children: [
{
path: Route.HOME,
element: HomepageFactory.create()
element: HomepageFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.COLLECTIONS_BASE,
element: CollectionFactory.create()
element: CollectionFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.COLLECTIONS,
element: CollectionFactory.create()
element: CollectionFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.DATASETS,
element: DatasetFactory.create()
element: DatasetFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.FILES,
element: FileFactory.create()
element: FileFactory.create(),
errorElement: <ErrorPage />
},
// 🔐 Protected routes are only accessible to authenticated users
{
element: <ProtectedRoute />,
errorElement: <ErrorPage />,
children: [
{
path: Route.CREATE_COLLECTION,
element: CreateCollectionFactory.create()
element: CreateCollectionFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.CREATE_DATASET,
element: CreateDatasetFactory.create()
element: CreateDatasetFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.UPLOAD_DATASET_FILES,
element: UploadDatasetFilesFactory.create()
element: UploadDatasetFilesFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.EDIT_DATASET_METADATA,
element: EditDatasetMetadataFactory.create()
element: EditDatasetMetadataFactory.create(),
errorElement: <ErrorPage />
},
{
path: Route.ACCOUNT,
element: AccountFactory.create()
element: AccountFactory.create(),
errorElement: <ErrorPage />
}
]
}
Expand Down
30 changes: 30 additions & 0 deletions src/sections/error-page/ErrorPage.module.scss
Original file line number Diff line number Diff line change
@@ -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: var(--error-min-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;
}
36 changes: 36 additions & 0 deletions src/sections/error-page/ErrorPage.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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()

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)

Copy link
Contributor

@g-saracca g-saracca Oct 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid running natives javascript dom selectors to find elements by their tag name or even by a class name, these might change and might not even be the only ones you find.

Take a look at this, is from my lazy imports PR, see this tsx file and this scss one.

So then you only need to pass that prop to the ErrorPage on the root layout path object.


  {
    path: '/',
    element: <Layout />,
    errorElement: <ErrorPage fullViewport />,
    children: [
    ...rest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll follow the example. Thank you!

return (
<section className={styles['section-wrapper']}>
<div className={styles['middle-search-cta-wrapper']}>
<div className={styles['icon-layout']}>
<ExclamationCircle color={theme.color.dangerColor} size={62} />
<div aria-label="error-page">
<h1>{t('message.heading')}</h1>
<h4>{t('message.errorText')}</h4>
</div>
</div>

<Link to="/" className="btn btn-secondary">
{t('backToHomepage', { brandName: t('brandName') })}
</Link>
</div>
</section>
)
}
21 changes: 21 additions & 0 deletions src/sections/error-page/useErrorLogger.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { useCallback, useEffect } from 'react'

const loggedErrors = new Set<string>()

export function useErrorLogger(error: Error | unknown): (error: Error) => void {
const logErrorOnce = useCallback((err: Error & { data?: unknown }): void => {
const errorString = String(err.data)
if (!loggedErrors.has(errorString)) {
loggedErrors.add(errorString)
console.error('Error:', err)
}
}, [])

useEffect(() => {
if (error) {
logErrorOnce(error as Error)
}
}, [error, logErrorOnce])

return logErrorOnce
}
100 changes: 100 additions & 0 deletions tests/component/sections/error-page/useErrorLogger.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
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) => {
cy.stub(win.console, 'error').as('consoleError')
})
})

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 | RouterError | null }) => useErrorLogger(error),
{ initialProps: { error: testError } }
)
cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', testError)
cy.then(() => {
rerender({ error: testError })
})
cy.get('@consoleError').should('have.been.calledOnce')
cy.then(() => {
rerender({ error: newError })
})
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 | RouterError | 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 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(() => {
return result.current(badRequestError.error)
})
cy.get('@consoleError').should('have.been.calledOnceWith', 'Error:', badRequestError.error)
cy.then(() => {
result.current(badRequestError.error)
})
cy.get('@consoleError').should('have.been.calledOnce')
cy.then(() => {
result.current(newError.error)
})
cy.get('@consoleError').should('have.been.calledWith', 'Error:', newError.error)
})
})
})
Loading