Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ export default tseslint.config(
'sonarjs/no-ignored-return': 'warn',
'sonarjs/no-ignored-exceptions': 'warn',
'sonarjs/prefer-read-only-props': 'warn',
// React class render() legitimately returns JSX | ReactNode on different paths
'sonarjs/function-return-type': 'warn',
},
},
{
Expand Down
19 changes: 11 additions & 8 deletions packages/web/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { BrowserRouter, Routes, Route, Link } from 'react-router-dom';
import { AuthProvider, useAuth } from './AuthContext';
import ErrorBoundary from './ErrorBoundary';
import Home from './pages/Home';
import MovieDetail from './pages/MovieDetail';
import SeatSelection from './pages/SeatSelection';
Expand Down Expand Up @@ -38,14 +39,16 @@ function App() {
<AuthProvider>
<Header />
<div style={{ padding: '20px' }}>
<Routes>
<Route path="/" element={<Home />} />
<Route path="/movie/:id" element={<MovieDetail />} />
<Route path="/seats/:showtimeId" element={<SeatSelection />} />
<Route path="/booking-confirmation/:bookingId" element={<BookingConfirmation />} />
<Route path="/bookings" element={<MyBookings />} />
<Route path="/login" element={<Login />} />
</Routes>
<ErrorBoundary>
<Routes>
<Route path="/" element={<Home />} />
<Route path="/movie/:id" element={<MovieDetail />} />
<Route path="/seats/:showtimeId" element={<SeatSelection />} />
<Route path="/booking-confirmation/:bookingId" element={<BookingConfirmation />} />
<Route path="/bookings" element={<MyBookings />} />
<Route path="/login" element={<Login />} />
</Routes>
</ErrorBoundary>
</div>
</AuthProvider>
</BrowserRouter>
Expand Down
75 changes: 75 additions & 0 deletions packages/web/src/ErrorBoundary.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import { render, screen } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import ErrorBoundary from './ErrorBoundary'

function Bomb(): never {
throw new Error('Test explosion')
}

function Safe() {
return <div>All good</div>
}

// Suppress the expected console.error noise from ErrorBoundary.componentDidCatch
beforeEach(() => {
vi.spyOn(console, 'error').mockImplementation(() => undefined)
})

afterEach(() => {
vi.restoreAllMocks()
})

describe('ErrorBoundary', () => {
it('renders children when no error occurs', () => {
render(
<ErrorBoundary>
<Safe />
</ErrorBoundary>
)
expect(screen.getByText('All good')).toBeInTheDocument()
})

it('renders the fallback UI when a child throws', () => {
render(
<ErrorBoundary>
<Bomb />
</ErrorBoundary>
)
expect(screen.getByText('Something went wrong.')).toBeInTheDocument()
expect(screen.getByText(/unexpected error occurred/)).toBeInTheDocument()
expect(screen.getByRole('button', { name: 'Try again' })).toBeInTheDocument()
})

it('does not re-throw the error to the parent', () => {
expect(() =>
render(
<ErrorBoundary>
<Bomb />
</ErrorBoundary>
)
).not.toThrow()
})

it('resets error state and re-renders children when "Try again" is clicked', async () => {
let shouldThrow = true

function MaybeThrow() {
if (shouldThrow) throw new Error('Conditional error')
return <div>Recovered</div>
}

render(
<ErrorBoundary>
<MaybeThrow />
</ErrorBoundary>
)

expect(screen.getByText('Something went wrong.')).toBeInTheDocument()

shouldThrow = false
await userEvent.click(screen.getByRole('button', { name: 'Try again' }))

expect(screen.getByText('Recovered')).toBeInTheDocument()
})
})
47 changes: 47 additions & 0 deletions packages/web/src/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React, { Component, ErrorInfo, ReactNode } from 'react';

interface Props {
children: ReactNode;
}

interface State {
hasError: boolean;
}

class ErrorBoundary extends Component<Props, State> {
constructor(props: Props) {
super(props);
this.state = { hasError: false };
}

static getDerivedStateFromError(): State {
return { hasError: true };
}

Comment on lines +18 to +20
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

While getDerivedStateFromError is great for updating state to render a fallback UI, it's also crucial to log the error for debugging and monitoring purposes. You should implement componentDidCatch to handle side-effects like logging the error to the console or an external service. Without this, errors caught by this boundary will not be reported, making it difficult to track issues in production.

  static getDerivedStateFromError(error: Error): State {
    return { hasError: true, error };
  }

  componentDidCatch(error: Error, errorInfo: React.ErrorInfo) {
    // TODO: In a real app, you'd send this to an error reporting service
    console.error("ErrorBoundary caught an error:", error, errorInfo);
  }

componentDidCatch(error: Error, info: ErrorInfo): void {
console.error('ErrorBoundary caught an error:', error, info.componentStack);
}

render(): ReactNode {
if (this.state.hasError) {
return (
<div style={{ textAlign: 'center', padding: '60px 20px' }}>
<h2>Something went wrong.</h2>
<p style={{ color: '#666', marginTop: '8px' }}>
An unexpected error occurred. Please refresh the page to try again.
</p>
<button
style={{ marginTop: '20px' }}
onClick={() => this.setState({ hasError: false })}
>
Comment on lines +25 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

For better maintainability and separation of concerns, it's recommended to avoid inline styles. Consider moving these styles to a dedicated CSS or CSS-in-JS file and using class names instead. This makes styles more reusable and easier to manage, especially as the application grows.

For example, you could create an ErrorBoundary.css file and import it:

ErrorBoundary.css

.error-boundary-fallback {
  text-align: center;
  padding: 60px 20px;
}

.error-message {
  color: #666;
  margin-top: 8px;
}

.retry-button {
  margin-top: 20px;
}

ErrorBoundary.tsx

import './ErrorBoundary.css';

// ...
<div className="error-boundary-fallback">
  <h2>Something went wrong.</h2>
  <p className="error-message">
    {this.state.error?.message ?? 'An unexpected error occurred.'}
  </p>
  <button
    onClick={() => this.setState({ hasError: false, error: null })}
    className="retry-button"
  >
    Try again
  </button>
</div>

Try again
</button>
</div>
);
}

return this.props.children;
}
}

export default ErrorBoundary;
Loading