Skip to content

Commit d4be568

Browse files
SjurSutterudSagenSjurSutterudSagen
authored
Feature/simplify showing correct error messages (#916)
* Revert "Feaeture/px web2 865 make correct error show for different errors (#908)" This reverts commit ec07444. So we can do a simpler version of it. * Add StartPage error translation * Regenerate language types * Add errormessage to startpage for no tables found * Code cleanup in StartPage * Add the simpler error handling * Remove unused fallback prop * Cleanup ErrorLayout test * Remove console.error from Selection * Fix missing new before Error
1 parent ec07444 commit d4be568

File tree

11 files changed

+75
-623
lines changed

11 files changed

+75
-623
lines changed

‎packages/pxweb2/src/app/components/ErrorBoundary/ErrorBoundary.spec.tsx‎

Lines changed: 13 additions & 185 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,21 @@
1-
import React from 'react';
2-
import { vi } from 'vitest';
31
import { render, screen } from '@testing-library/react';
42
import '@testing-library/jest-dom/vitest';
3+
import { vi } from 'vitest';
54

65
import ErrorBoundary from './ErrorBoundary';
7-
import { ApiError, Problem } from '@pxweb2/pxweb2-api-client';
8-
import { ApiProblemError } from '../../util/ApiProblemError';
96

10-
// Mock only the UI components, not the business logic
7+
// Mock the error components
118
vi.mock('../Errors/GenericError/GenericError', () => ({
129
GenericError: () => (
1310
<div data-testid="generic-error">Generic Error Component</div>
1411
),
1512
}));
1613

1714
vi.mock('../Errors/NotFound/NotFound', () => ({
18-
NotFound: () => <div data-testid="not-found-error">Not Found Component</div>,
15+
NotFound: () => <div data-testid="not-found">Not Found Component</div>,
1916
}));
2017

2118
describe('ErrorBoundary', () => {
22-
// Helper to create a real ApiError instance
23-
const createMockApiError = (
24-
status: number,
25-
problem?: Partial<Problem>,
26-
): ApiError => {
27-
const apiError = Object.create(ApiError.prototype);
28-
apiError.status = status;
29-
apiError.url = 'http://test.com';
30-
apiError.statusText = 'Error';
31-
apiError.body = {
32-
status: problem?.status ?? status,
33-
title: problem?.title ?? 'Test Error',
34-
type: problem?.type ?? 'test-error-type',
35-
...problem,
36-
} as Problem;
37-
apiError.request = { method: 'GET', url: 'http://test.com' };
38-
apiError.name = 'ApiError';
39-
apiError.message = 'Test error';
40-
41-
return apiError;
42-
};
43-
4419
// Setup console mocks before all tests
4520
let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
4621
let consoleLogSpy: ReturnType<typeof vi.spyOn>;
@@ -73,9 +48,9 @@ describe('ErrorBoundary', () => {
7348
expect(screen.getByText('Child Component')).toBeInTheDocument();
7449
});
7550

76-
it('renders GenericError when an error occurs', () => {
51+
it('renders NotFound when a 404 error occurs', () => {
7752
const ErrorComponent = () => {
78-
throw new Error('Test error');
53+
throw new Error('404 Not Found');
7954
};
8055

8156
render(
@@ -84,38 +59,17 @@ describe('ErrorBoundary', () => {
8459
</ErrorBoundary>,
8560
);
8661

87-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
88-
expect(screen.getByText('Generic Error Component')).toBeInTheDocument();
89-
});
90-
91-
it('renders NotFound component when ApiProblemError with 404 status occurs', () => {
92-
const mockApiError = createMockApiError(404, {
93-
title: 'Not Found',
94-
type: 'about:blank',
95-
});
96-
const ErrorComponent = () => {
97-
throw new ApiProblemError(mockApiError, 'test-table-id');
98-
};
99-
100-
render(
101-
<ErrorBoundary>
102-
<ErrorComponent />
103-
</ErrorBoundary>,
104-
);
105-
106-
// Assert that the NotFound component is rendered instead of GenericError
107-
expect(screen.getByTestId('not-found-error')).toBeInTheDocument();
62+
expect(screen.getByTestId('not-found')).toBeInTheDocument();
10863
expect(screen.getByText('Not Found Component')).toBeInTheDocument();
10964
expect(screen.queryByTestId('generic-error')).not.toBeInTheDocument();
65+
expect(
66+
screen.queryByText('Generic Error Component'),
67+
).not.toBeInTheDocument();
11068
});
11169

112-
it('renders GenericError when ApiProblemError with non-404 status occurs', () => {
113-
const mockApiError = createMockApiError(500, {
114-
title: 'Internal Server Error',
115-
type: 'about:blank',
116-
});
70+
it('renders GenericError when a non-404 error occurs', () => {
11771
const ErrorComponent = () => {
118-
throw new ApiProblemError(mockApiError, 'test-table-id');
72+
throw new Error('Test error');
11973
};
12074

12175
render(
@@ -124,135 +78,9 @@ describe('ErrorBoundary', () => {
12478
</ErrorBoundary>,
12579
);
12680

127-
// Assert that the GenericError component is rendered, not NotFound
12881
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
12982
expect(screen.getByText('Generic Error Component')).toBeInTheDocument();
130-
expect(screen.queryByTestId('not-found-error')).not.toBeInTheDocument();
131-
});
132-
133-
it('renders NotFound when error message starts with 404 status code', () => {
134-
const ErrorComponent = () => {
135-
throw new Error(
136-
'404\n TableId: TAB60065 \n Not Found\n - https://...',
137-
);
138-
};
139-
140-
render(
141-
<ErrorBoundary>
142-
<ErrorComponent />
143-
</ErrorBoundary>,
144-
);
145-
146-
expect(screen.getByTestId('not-found-error')).toBeInTheDocument();
147-
});
148-
149-
it('renders NotFound when error message contains 404 pattern', () => {
150-
const ErrorComponent = () => {
151-
throw new Error('Request failed with status code 404');
152-
};
153-
154-
render(
155-
<ErrorBoundary>
156-
<ErrorComponent />
157-
</ErrorBoundary>,
158-
);
159-
160-
expect(screen.getByTestId('not-found-error')).toBeInTheDocument();
161-
});
162-
163-
it('renders NotFound when error message contains "not found" keyword', () => {
164-
const ErrorComponent = () => {
165-
throw new Error('Table not found in the database');
166-
};
167-
168-
render(
169-
<ErrorBoundary>
170-
<ErrorComponent />
171-
</ErrorBoundary>,
172-
);
173-
174-
expect(screen.getByTestId('not-found-error')).toBeInTheDocument();
175-
});
176-
177-
it('renders GenericError when error message contains 500 keyword', () => {
178-
const ErrorComponent = () => {
179-
throw new Error('Server error occurred: 500');
180-
};
181-
182-
render(
183-
<ErrorBoundary>
184-
<ErrorComponent />
185-
</ErrorBoundary>,
186-
);
187-
188-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
189-
});
190-
191-
it('renders GenericError when error message contains "server error" keyword', () => {
192-
const ErrorComponent = () => {
193-
throw new Error('Internal server error occurred');
194-
};
195-
196-
render(
197-
<ErrorBoundary>
198-
<ErrorComponent />
199-
</ErrorBoundary>,
200-
);
201-
202-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
203-
});
204-
205-
it('renders GenericError when status code in message is out of valid HTTP range', () => {
206-
const ErrorComponent = () => {
207-
throw new Error('Invalid status code 999 occurred');
208-
};
209-
210-
render(
211-
<ErrorBoundary>
212-
<ErrorComponent />
213-
</ErrorBoundary>,
214-
);
215-
216-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
217-
});
218-
219-
it('renders GenericError when error has null state', () => {
220-
const ErrorComponent = () => {
221-
throw new Error('Generic error without status');
222-
};
223-
224-
render(
225-
<ErrorBoundary>
226-
<ErrorComponent />
227-
</ErrorBoundary>,
228-
);
229-
230-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
231-
});
232-
233-
it('renders GenericError when error state is null', () => {
234-
// This tests the edge case where state.error is null but hasError is true
235-
// We can simulate this by manually setting the state after component mounts
236-
const TestWrapper = () => {
237-
const boundaryRef = React.useRef<ErrorBoundary>(null);
238-
239-
React.useEffect(() => {
240-
if (boundaryRef.current) {
241-
// Force the component into an error state with null error
242-
boundaryRef.current.setState({ hasError: true, error: null });
243-
}
244-
}, []);
245-
246-
return (
247-
<ErrorBoundary ref={boundaryRef}>
248-
<div>Child Component</div>
249-
</ErrorBoundary>
250-
);
251-
};
252-
253-
render(<TestWrapper />);
254-
255-
// After the effect runs, it should show the generic error
256-
expect(screen.getByTestId('generic-error')).toBeInTheDocument();
83+
expect(screen.queryByTestId('not-found')).not.toBeInTheDocument();
84+
expect(screen.queryByText('Not Found Component')).not.toBeInTheDocument();
25785
});
25886
});

‎packages/pxweb2/src/app/components/ErrorBoundary/ErrorBoundary.tsx‎

Lines changed: 9 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { Component, ReactNode } from 'react';
22

33
import { GenericError } from '../Errors/GenericError/GenericError';
44
import { NotFound } from '../Errors/NotFound/NotFound';
5-
import { ApiProblemError } from '../../util/ApiProblemError';
65

76
interface ErrorBoundaryProps {
87
children: ReactNode;
@@ -16,77 +15,29 @@ interface ErrorBoundaryState {
1615
class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
1716
state: ErrorBoundaryState = { hasError: false, error: null };
1817

19-
/**
20-
* Enhanced error status detection that handles multiple error formats
21-
*/
22-
private getErrorStatus(error: Error): number | null {
23-
// 1. Check if it's already an ApiProblemError (preferred format)
24-
if (error instanceof ApiProblemError) {
25-
return error.status;
26-
}
27-
28-
// 2. Parse HTTP status code from error message beginning (problemMessage format)
29-
// Example: "404\n TableId: TAB60065 \n Not Found\n - https://..."
30-
const statusMatch = error.message.match(/^(\d{3})/);
31-
if (statusMatch) {
32-
const status = Number.parseInt(statusMatch[1], 10);
33-
34-
return status;
35-
}
36-
37-
// 3. Check for status patterns within the message
38-
const statusInMessage = error.message.match(/(\d{3})/);
39-
if (statusInMessage) {
40-
const status = Number.parseInt(statusInMessage[1], 10);
41-
42-
// Validate it's a real HTTP status code
43-
if (status >= 100 && status < 600) {
44-
return status;
45-
}
46-
}
47-
48-
// 4. Check for common error keywords
49-
if (
50-
error.message.includes('404') ||
51-
error.message.toLowerCase().includes('not found') ||
52-
error.message.toLowerCase().includes('table not found')
53-
) {
54-
return 404;
55-
}
56-
57-
if (
58-
error.message.toLowerCase().includes('server error') ||
59-
error.message.includes('500')
60-
) {
61-
return 500;
62-
}
63-
64-
return null;
65-
}
66-
6718
static getDerivedStateFromError(error: Error): ErrorBoundaryState {
6819
return { hasError: true, error };
6920
}
7021

71-
// NOSONAR: Add optional logging here if needed
22+
// NOSONAR: Add error logging here later if wanted
7223
// componentDidCatch(error: Error, info: ErrorInfo) {
73-
//
24+
// console.log(error, info);
7425
// }
7526

7627
render() {
7728
if (this.state.hasError) {
78-
// If error object is available, use it for enhanced detection
79-
if (this.state.error) {
80-
// Use enhanced error detection
81-
const detectedStatus = this.getErrorStatus(this.state.error);
29+
// Parse status code from error message if possible
30+
const statusInMessage = this.state.error?.message.match(/(\d{3})/);
31+
if (statusInMessage) {
32+
const status = Number.parseInt(statusInMessage[1], 10);
8233

83-
// Check for 404 errors using enhanced detection
84-
if (detectedStatus === 404) {
34+
// Check for 404 code in error message
35+
if (status === 404) {
8536
return <NotFound />;
8637
}
8738
}
8839

89-
// Default error UI for all other errors
40+
// Default error component for all other errors
9041
return <GenericError />;
9142
}
9243

0 commit comments

Comments
 (0)