Skip to content

Commit 3696d3c

Browse files
VeikkosuhonenVeikkosuhonen
committed
Client error handling improvements
1 parent 95901b2 commit 3696d3c

File tree

7 files changed

+58
-151
lines changed

7 files changed

+58
-151
lines changed

‎src/client/App.tsx‎

Lines changed: 34 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ import { fi } from 'date-fns/locale'
88
import { AdapterDateFns } from '@mui/x-date-pickers/AdapterDateFnsV3'
99
import { Box, Button, CssBaseline, Snackbar } from '@mui/material'
1010
import { AppContext } from './util/AppContext'
11-
import { ErrorBoundary } from './components/ErrorBoundary'
1211

13-
import { PUBLIC_URL, inCI, inDevelopment, inProduction, inStaging } from '../config'
14-
import { User } from './types'
12+
import { PUBLIC_URL } from '../config'
13+
import type { User } from './types'
1514
import useTheme from './theme'
1615
import NavBar from './components/NavBar'
1716
import Footer from './components/Footer'
@@ -78,62 +77,47 @@ const App = () => {
7877
const { user, isLoading } = useCurrentUser()
7978

8079
useEffect(() => {
81-
// Add error handling to prevent app crashes
82-
try {
83-
console.log('Environment check:', { inCI, inStaging, inProduction, inDevelopment })
84-
85-
if (!inCI && !inStaging && !inDevelopment) {
86-
console.log('Initializing Shibboleth pinger...')
87-
initShibbolethPinger()
88-
} else {
89-
console.log('Skipping Shibboleth pinger initialization in non-production environments.')
90-
}
91-
} catch (error) {
92-
console.error('Error initializing Shibboleth pinger:', error)
93-
// Don't let this crash the app
94-
}
80+
initShibbolethPinger()
9581
}, [])
9682

9783
const onNoAccessPage = location.pathname.includes('/noaccess')
9884

99-
// if (isLoading && !onNoAccessPage) return null
100-
//
101-
// if (!onNoAccessPage && !hasAccess(user, courseId)) {
102-
// window.location.href = PUBLIC_URL + getRedirect(user)
103-
// return null
104-
// }
85+
if (isLoading && !onNoAccessPage) return null
86+
87+
if (!onNoAccessPage && !hasAccess(user, courseId)) {
88+
window.location.href = PUBLIC_URL + getRedirect(user)
89+
return null
90+
}
10591

10692
if (!user && !onNoAccessPage) return null
10793

10894
return (
109-
<ErrorBoundary>
110-
<ThemeProvider theme={theme}>
111-
<CssBaseline />
112-
<LocalizationProvider dateAdapter={AdapterDateFns} adapterLocale={fi}>
113-
<SnackbarProvider preventDuplicate>
114-
<AppContext.Provider value={appRef}>
115-
<Box
116-
sx={{
117-
display: 'flex',
118-
flexDirection: 'column',
119-
minHeight: '100vh',
120-
height: '100vh',
121-
overflowY: 'auto', // deleting this will break the auto scroll on chats
122-
}}
123-
ref={appRef}
124-
>
125-
<NavBar />
126-
<Box sx={{ flex: 1 }}>
127-
<Outlet />
128-
</Box>
129-
<Footer />
95+
<ThemeProvider theme={theme}>
96+
<CssBaseline />
97+
<LocalizationProvider dateAdapter={AdapterDateFns} adapterLocale={fi}>
98+
<SnackbarProvider preventDuplicate>
99+
<AppContext.Provider value={appRef}>
100+
<Box
101+
sx={{
102+
display: 'flex',
103+
flexDirection: 'column',
104+
minHeight: '100vh',
105+
height: '100vh',
106+
overflowY: 'auto', // deleting this will break the auto scroll on chats
107+
}}
108+
ref={appRef}
109+
>
110+
<NavBar />
111+
<Box sx={{ flex: 1 }}>
112+
<Outlet />
130113
</Box>
131-
<AdminLoggedInAsBanner />
132-
</AppContext.Provider>
133-
</SnackbarProvider>
134-
</LocalizationProvider>
135-
</ThemeProvider>
136-
</ErrorBoundary>
114+
<Footer />
115+
</Box>
116+
<AdminLoggedInAsBanner />
117+
</AppContext.Provider>
118+
</SnackbarProvider>
119+
</LocalizationProvider>
120+
</ThemeProvider>
137121
)
138122
}
139123

‎src/client/Router.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ import { ChatV2 } from './components/ChatV2/ChatV2'
1414
import { RagIndex } from './components/Rag/RagIndex'
1515
import { RagFile } from './components/Rag/RagFile'
1616
import { NotFound } from './components/common/NotFound'
17-
import { ErrorBoundary } from './components/ErrorBoundary'
17+
import { ErrorPage } from './components/ErrorPage'
1818

1919
const router = createBrowserRouter(
2020
createRoutesFromElements(
21-
<Route path="/" element={<App />}>
21+
<Route path="/" element={<App />} ErrorBoundary={ErrorPage}>
2222
<Route index element={<Chat />} />
2323
<Route path="/v2" element={<ChatV2 />} />
2424
<Route path="/v2/:courseId" element={<ChatV2 />} />
@@ -40,10 +40,6 @@ const router = createBrowserRouter(
4040
},
4141
)
4242

43-
const Router = () => (
44-
<ErrorBoundary>
45-
<RouterProvider router={router} />
46-
</ErrorBoundary>
47-
)
43+
const Router = () => <RouterProvider router={router} />
4844

4945
export default Router

‎src/client/components/Courses/Course/index.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ import SystemMessage from '../../Chat/SystemMessage'
1616
import Rag from '../../Rag/Rag'
1717
import { formatDate, getCurTypeLabel } from '../util'
1818
import EditCourseForm from './EditCourseForm'
19-
import MaxTokenUsageStudents from './MaxTokenUsageStudents'
2019
import Prompt from './Prompt'
2120
import Stats from './Stats'
2221
import { RouterTabs } from '../../common/RouterTabs'
2322
import Discussion from './Discussions'
23+
import { ApiErrorView } from '../../common/ApiErrorView'
2424

2525
const Course = () => {
2626
const [showTeachers, setShowTeachers] = useState(false)
@@ -32,13 +32,17 @@ const Course = () => {
3232

3333
const { language } = i18n
3434

35-
const { data: course, isSuccess: isCourseSuccess } = useCourse(id)
3635
const { user, isLoading: userLoading } = useCurrentUser()
3736

38-
const studentLink = `${window.location.origin}${PUBLIC_URL}/${course?.courseId}`
37+
const { data: course, isSuccess: isCourseSuccess, error } = useCourse(id)
38+
if (error) {
39+
return <ApiErrorView error={error} />
40+
}
3941

4042
if (userLoading || !user || !isCourseSuccess) return null
4143

44+
const studentLink = `${window.location.origin}${PUBLIC_URL}/${course.courseId}`
45+
4246
const amongResponsibles = course.responsibilities ? course.responsibilities.some((r) => r.user.id === user.id) : false
4347

4448
if (!user.isAdmin && !amongResponsibles) {

‎src/client/components/ErrorBoundary.tsx‎ renamed to ‎src/client/components/ErrorPage.tsx‎

Lines changed: 3 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,19 @@
11
import React from 'react'
22
import { Box, Typography } from '@mui/material'
33
import { useTranslation } from 'react-i18next'
4-
import { setErrorStateHandler } from '../util/apiClient'
54
import { OutlineButtonBlack } from './ChatV2/generics/Buttons'
65
import { ArrowBack, Replay } from '@mui/icons-material'
7-
import { inDevelopment } from '../../config'
6+
import { useRouteError } from 'react-router-dom'
87

9-
// todo: setup sentry
10-
11-
interface ErrorBoundaryState {
12-
hasError: boolean
13-
error?: Error
14-
}
15-
16-
interface ErrorBoundaryProps {
17-
children: React.ReactNode
18-
}
19-
20-
export class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> {
21-
constructor(props: ErrorBoundaryProps) {
22-
super(props)
23-
this.state = { hasError: false }
24-
25-
setErrorStateHandler((error: Error) => {
26-
this.setState({ hasError: true, error })
27-
})
28-
}
29-
30-
static getDerivedStateFromError(error: Error): ErrorBoundaryState {
31-
return { hasError: true, error }
32-
}
33-
34-
componentDidCatch(error: Error, errorInfo: React.ErrorInfo) {
35-
console.error('ErrorBoundary caught an error:', error, errorInfo)
36-
}
37-
38-
emptyErrorState = () => {
39-
this.setState({ hasError: false, error: undefined })
40-
}
41-
42-
render() {
43-
if (this.state.hasError) {
44-
return <ErrorPage error={this.state.error} setErrorState={this.emptyErrorState} />
45-
}
46-
47-
return this.props.children
48-
}
49-
}
50-
51-
const ErrorPage = ({ error, setErrorState }: { error?: Error; setErrorState: VoidFunction }) => {
8+
export const ErrorPage = () => {
9+
const error = useRouteError() as Error
5210
const { t } = useTranslation()
5311

5412
const handleReload = () => {
55-
setErrorState()
5613
window.location.reload()
5714
}
5815

5916
const handleGoHome = () => {
60-
setErrorState()
6117
window.location.href = '/'
6218
}
6319

‎src/client/components/common/ApiErrorView.tsx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { ApiError } from '../../util/apiClient'
2+
import { NotFound } from './NotFound'
3+
4+
export const ApiErrorView = ({ error }: { error: ApiError }) => {
5+
if (error.status === 404) {
6+
return <NotFound />
7+
}
8+
9+
throw error
10+
}

‎src/client/hooks/useCourse.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const useCourse = (courseId?: string) => {
1919
queryKey,
2020
queryFn,
2121
enabled: !!courseId,
22+
retry: false,
2223
})
2324
}
2425

‎src/client/util/apiClient.ts‎

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -22,44 +22,6 @@ apiClient.interceptors.request.use((config) => {
2222
return newConfig
2323
})
2424

25-
let errorStateHandler: ((error: Error) => void) | null = null
26-
let isHandlingError = false
27-
28-
export const setErrorStateHandler = (handler: (error: Error) => void) => {
29-
errorStateHandler = handler
30-
}
31-
32-
const handleError = (error: AxiosError<{ message: string }>, isStream = false, streamMessage?: string) => {
33-
if (isHandlingError) {
34-
console.error('Error occurred while handling another error:', error)
35-
return
36-
}
37-
38-
const message = streamMessage || error.response?.data?.message || error.message || 'Something went wrong'
39-
const customError = new Error(message)
40-
41-
if (errorStateHandler) {
42-
try {
43-
isHandlingError = true
44-
errorStateHandler(customError)
45-
} catch (handlerError) {
46-
console.error('Error in error handler:', handlerError)
47-
} finally {
48-
isHandlingError = false
49-
}
50-
}
51-
52-
if (isStream) {
53-
throw customError
54-
}
55-
return Promise.reject(customError)
56-
}
57-
58-
apiClient.interceptors.response.use(
59-
(response) => response,
60-
(error: AxiosError<{ message: string }>) => handleError(error),
61-
)
62-
6325
export const postAbortableStream = async (path: string, formData: FormData, externalController?: AbortController) => {
6426
const controller = externalController ?? new AbortController()
6527

@@ -76,12 +38,6 @@ export const postAbortableStream = async (path: string, formData: FormData, exte
7638
signal: controller.signal,
7739
})
7840

79-
if (!response.ok) {
80-
const message = (await response.text()) || 'Something went wrong'
81-
const mockError = { response: { status: response.status } } as AxiosError<{ message: string }>
82-
handleError(mockError, true, message)
83-
}
84-
8541
let tokenUsageAnalysis: {
8642
tokenUsageWarning: boolean
8743
message: string

0 commit comments

Comments
 (0)