Skip to content

Commit e8ad51e

Browse files
authored
fix: handle network errors in notebook controller catch blocks (opendatahub-io#6647)
Signed-off-by: Guilherme Caponetto <638737+caponetto@users.noreply.github.com>
1 parent bbc3711 commit e8ad51e

File tree

4 files changed

+21
-7
lines changed

4 files changed

+21
-7
lines changed

frontend/src/pages/notebookController/ValidateNotebookNamespace.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ const ValidateNotebookNamespace: React.FC<ValidateNotebookNamespaceProps> = ({ c
2020
})
2121
.catch((e) => {
2222
const error = new Error(
23-
`Error validating the role binding of your notebookNamespace; ${e.response.data.message}`,
23+
`Error validating the role binding of your notebookNamespace; ${
24+
e.response?.data?.message || e.message
25+
}`,
2426
);
2527
setLoadError(error);
2628
});

frontend/src/pages/notebookController/screens/admin/__tests__/useCheckForAllowedUsers.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,16 @@ describe('useCheckForAllowedUsers', () => {
5151
expect(renderResult).hookToStrictEqual([[], false, new Error('error1')]);
5252
expect(renderResult).hookToHaveUpdateCount(2);
5353
});
54+
55+
it('should handle error without response (network error)', async () => {
56+
getAllowedUsersMock.mockRejectedValue(new Error('Network Error'));
57+
const renderResult = testHook(useCheckForAllowedUsers)();
58+
59+
expect(renderResult).hookToStrictEqual([[], false, undefined]);
60+
expect(renderResult).hookToHaveUpdateCount(1);
61+
62+
await renderResult.waitForNextUpdate();
63+
expect(renderResult).hookToStrictEqual([[], false, new Error('Network Error')]);
64+
expect(renderResult).hookToHaveUpdateCount(2);
65+
});
5466
});

frontend/src/pages/notebookController/screens/admin/useCheckForAllowedUsers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ const useCheckForAllowedUsers = (): [
2020
setLoaded(true);
2121
})
2222
.catch((e) => {
23-
setError(new Error(e.response.data.message));
23+
setError(new Error(e.response?.data?.message || e.message));
2424
setLoaded(false);
2525
});
2626
}, [workbenchNamespace]);

frontend/src/services/notebookService.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ export const getNotebook = (namespace: string, name: string): Promise<Notebook>
88
.get(url)
99
.then((response) => response.data)
1010
.catch((e) => {
11-
throw new Error(e.response.data.message);
11+
throw new Error(e.response?.data?.message || e.message);
1212
});
1313
};
1414

@@ -23,13 +23,13 @@ export const getNotebookAndStatus = (
2323
.get(url)
2424
.then((response) => response.data)
2525
.catch((e) => {
26-
if (e.response.status === 404) {
26+
if (e.response?.status === 404) {
2727
return { notebook, isRunning: false };
2828
}
2929
/* eslint-disable-next-line no-console */
3030
console.error(
3131
'Checking notebook status failed, falling back on notebook check logic',
32-
e.response.data.message,
32+
e.response?.data?.message || e.message,
3333
);
3434
// Notebooks are unreliable to live status on replicas -- but if we have nothing else...
3535
const isRunning = !!(
@@ -48,7 +48,7 @@ export const enableNotebook = async (notebookData: NotebookData): Promise<Notebo
4848
.post(url, notebookData)
4949
.then((response) => response.data)
5050
.catch((e) => {
51-
throw new Error(e.response.data.message);
51+
throw new Error(e.response?.data?.message || e.message);
5252
});
5353
};
5454

@@ -65,6 +65,6 @@ export const stopNotebook = (username?: string): Promise<Notebook> => {
6565
.patch(url, patch)
6666
.then((response) => response.data)
6767
.catch((e) => {
68-
throw new Error(e.response.data.message);
68+
throw new Error(e.response?.data?.message || e.message);
6969
});
7070
};

0 commit comments

Comments
 (0)