Skip to content

Commit 068449a

Browse files
CompuIvesCompuIves
authored
Make API error handling more robust (codesandbox#3086)
* Make API error handling more robust * Change message * Fix status code checking * Fix type error * refactor managing API errors also throw the errors in the API
1 parent 7e1e9d0 commit 068449a

File tree

6 files changed

+180
-84
lines changed

6 files changed

+180
-84
lines changed

‎packages/app/src/app/overmind/effects/analytics.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import track, {
22
identify,
3+
logError,
34
setAnonymousId,
45
setUserId,
56
} from '@codesandbox/common/lib/utils/analytics';
@@ -16,6 +17,7 @@ export default (() => {
1617
trackedEvents[event] = true;
1718
track(event, data);
1819
},
20+
logError,
1921
identify,
2022
setAnonymousId,
2123
setUserId,

‎packages/app/src/app/overmind/effects/api/apiFactory.ts‎

Lines changed: 31 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1+
import { Module } from '@codesandbox/common/lib/types';
12
/* eslint-disable camelcase */
2-
import axios, { AxiosResponse, AxiosError, AxiosRequestConfig } from 'axios';
3-
import { logError } from '@codesandbox/common/lib/utils/analytics';
4-
import { values } from 'lodash-es';
3+
import axios, { AxiosError, AxiosRequestConfig, AxiosResponse } from 'axios';
54
import { camelizeKeys, decamelizeKeys } from 'humps';
6-
import { Module } from '@codesandbox/common/lib/types';
75

86
export const API_ROOT = '/api/v1';
97

10-
type RecursivePartial<T> = { [P in keyof T]?: RecursivePartial<T[P]> };
8+
export type ApiError = AxiosError<
9+
{ errors: string[] } | { error: string } | any
10+
>;
1111

1212
export type Params = {
1313
[key: string]: string;
@@ -32,36 +32,17 @@ export type ApiConfig = {
3232
[path: string]: Module;
3333
};
3434
getParsedConfigurations: () => any;
35-
onError: (error: string) => void;
35+
onError: (error: ApiError) => void;
3636
};
3737

38-
export default (config: {
39-
provideJwtToken: () => string;
40-
onError: (error: string) => void;
41-
}) => {
38+
export default (config: ApiConfig) => {
4239
const createHeaders = (jwt: string) =>
4340
jwt
4441
? {
4542
Authorization: `Bearer ${jwt}`,
4643
}
4744
: {};
4845

49-
const showError = error => {
50-
config.onError(error.message);
51-
error.apiMessage = error.message; // eslint-disable-line no-param-reassign
52-
};
53-
54-
const handleError = error => {
55-
const newError = convertError(error);
56-
try {
57-
showError(newError);
58-
} catch (e) {
59-
console.error(e);
60-
}
61-
62-
throw newError;
63-
};
64-
6546
const api: Api = {
6647
get(path, params, options) {
6748
return axios
@@ -70,31 +51,43 @@ export default (config: {
7051
headers: createHeaders(config.provideJwtToken()),
7152
})
7253
.then(response => handleResponse(response, options))
73-
.catch(e => handleError(e));
54+
.catch(e => {
55+
config.onError(e);
56+
return Promise.reject(e);
57+
});
7458
},
7559
post(path, body, options) {
7660
return axios
7761
.post(API_ROOT + path, decamelizeKeys(body), {
7862
headers: createHeaders(config.provideJwtToken()),
7963
})
8064
.then(response => handleResponse(response, options))
81-
.catch(e => handleError(e));
65+
.catch(e => {
66+
config.onError(e);
67+
return Promise.reject(e);
68+
});
8269
},
8370
patch(path, body, options) {
8471
return axios
8572
.patch(API_ROOT + path, decamelizeKeys(body), {
8673
headers: createHeaders(config.provideJwtToken()),
8774
})
8875
.then(response => handleResponse(response, options))
89-
.catch(e => handleError(e));
76+
.catch(e => {
77+
config.onError(e);
78+
return Promise.reject(e);
79+
});
9080
},
9181
put(path, body, options) {
9282
return axios
9383
.put(API_ROOT + path, decamelizeKeys(body), {
9484
headers: createHeaders(config.provideJwtToken()),
9585
})
9686
.then(response => handleResponse(response, options))
97-
.catch(e => handleError(e));
87+
.catch(e => {
88+
config.onError(e);
89+
return Promise.reject(e);
90+
});
9891
},
9992
delete(path, params, options) {
10093
return axios
@@ -103,7 +96,10 @@ export default (config: {
10396
headers: createHeaders(config.provideJwtToken()),
10497
})
10598
.then(response => handleResponse(response, options))
106-
.catch(e => handleError(e));
99+
.catch(e => {
100+
config.onError(e);
101+
return Promise.reject(e);
102+
});
107103
},
108104
request(requestConfig, options) {
109105
return axios
@@ -115,49 +111,16 @@ export default (config: {
115111
})
116112
)
117113
.then(response => handleResponse(response, options))
118-
.catch(e => handleError(e));
114+
.catch(e => {
115+
config.onError(e);
116+
return Promise.reject(e);
117+
});
119118
},
120119
};
121120

122121
return api;
123122
};
124123

125-
function convertError(error: AxiosError) {
126-
const { response } = error;
127-
128-
if (!response || response.status >= 500) {
129-
logError(error);
130-
}
131-
132-
if (response && response.data) {
133-
if (response.data.errors) {
134-
const errors = values(response.data.errors)[0];
135-
if (Array.isArray(errors)) {
136-
if (errors[0]) {
137-
error.message = errors[0]; // eslint-disable-line no-param-reassign,prefer-destructuring
138-
}
139-
} else {
140-
error.message = errors; // eslint-disable-line no-param-reassign
141-
}
142-
} else if (response.data.error) {
143-
const { error_code, message, ...data } = response.data.error as {
144-
message: string;
145-
error_code: string;
146-
[k: string]: any;
147-
};
148-
// @ts-ignore
149-
error.error_code = error_code; // eslint-disable-line no-param-reassign
150-
error.message = message; // eslint-disable-line no-param-reassign
151-
// @ts-ignore
152-
error.data = data; // eslint-disable-line no-param-reassign
153-
} else if (response.status === 413) {
154-
return 'File too large, upload limit is 5MB.';
155-
}
156-
}
157-
158-
return error;
159-
}
160-
161124
export function handleResponse(
162125
response: AxiosResponse,
163126
{ shouldCamelize = true } = {}

‎packages/app/src/app/overmind/factories.ts‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { Contributor } from '@codesandbox/common/lib/types';
22
import { IDerive, IState, json } from 'overmind';
3+
import { AxiosError } from 'axios';
34

5+
import { notificationState } from '@codesandbox/common/lib/utils/notifications';
6+
import { NotificationStatus } from '@codesandbox/notifications';
47
import { AsyncAction } from '.';
58

69
export const withLoadApp = <T>(
@@ -38,11 +41,31 @@ export const withLoadApp = <T>(
3841
actions.userNotifications.internal.initialize();
3942
effects.api.preloadTemplates();
4043
} catch (error) {
41-
effects.notificationToast.error(
42-
'Your session seems to be expired, please log in again...'
43-
);
44-
effects.jwt.reset();
45-
effects.analytics.setAnonymousId();
44+
if (error.isAxiosError && (error as AxiosError).response.status === 401) {
45+
// Reset existing sign in info
46+
effects.jwt.reset();
47+
effects.analytics.setAnonymousId();
48+
49+
// Allow user to sign in again in notification
50+
notificationState.addNotification({
51+
message: 'Your session seems to be expired, please log in again...',
52+
status: NotificationStatus.ERROR,
53+
actions: {
54+
primary: [
55+
{
56+
label: 'Sign in',
57+
run: () => {
58+
actions.signInClicked({ useExtraScopes: false });
59+
},
60+
},
61+
],
62+
},
63+
});
64+
} else {
65+
effects.notificationToast.error(
66+
"We weren't able to sign you in, this could be due to a flaky connection or something on our server. Please try again in a minute."
67+
);
68+
}
4669
}
4770
} else {
4871
effects.jwt.reset();

‎packages/app/src/app/overmind/internalActions.ts‎

Lines changed: 117 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,18 @@ import {
77
ServerStatus,
88
TabType,
99
} from '@codesandbox/common/lib/types';
10+
import { patronUrl } from '@codesandbox/common/lib/utils/url-generator';
1011
import { NotificationStatus } from '@codesandbox/notifications';
12+
import values from 'lodash-es/values';
1113

14+
import { ApiError } from './effects/api/apiFactory';
1215
import { createOptimisticModule } from './utils/common';
1316
import getItems from './utils/items';
1417
import { defaultOpenedModule, mainModule } from './utils/main-module';
1518
import { parseConfigurations } from './utils/parse-configurations';
1619
import { Action, AsyncAction } from '.';
1720

18-
export const signIn: AsyncAction<{ useExtraScopes: boolean }> = async (
21+
export const signIn: AsyncAction<{ useExtraScopes?: boolean }> = async (
1922
{ state, effects, actions },
2023
options
2124
) => {
@@ -122,7 +125,7 @@ export const authorize: AsyncAction = async ({ state, effects }) => {
122125
};
123126

124127
export const signInGithub: Action<
125-
{ useExtraScopes: boolean },
128+
{ useExtraScopes?: boolean },
126129
Promise<string>
127130
> = ({ effects }, options) => {
128131
const authPath =
@@ -306,11 +309,7 @@ export const updateCurrentSandbox: AsyncAction<Sandbox> = async (
306309
state.editor.currentSandbox.title = sandbox.title;
307310
};
308311

309-
export const ensurePackageJSON: AsyncAction = async ({
310-
state,
311-
effects,
312-
actions,
313-
}) => {
312+
export const ensurePackageJSON: AsyncAction = async ({ state, effects }) => {
314313
const sandbox = state.editor.currentSandbox;
315314
const existingPackageJson = sandbox.modules.find(
316315
module => module.directoryShortid == null && module.title === 'package.json'
@@ -353,3 +352,114 @@ export const closeTabByIndex: Action<number> = ({ state }, tabIndex) => {
353352

354353
state.editor.tabs.splice(tabIndex, 1);
355354
};
355+
356+
export const onApiError: Action<ApiError> = (
357+
{ state, actions, effects },
358+
error
359+
) => {
360+
const { response } = error;
361+
362+
if (response.status === 401) {
363+
// We need to implement a blocking modal to either sign in or refresh the browser to
364+
// continue in an anonymous state
365+
}
366+
367+
if (!response || response.status >= 500) {
368+
effects.analytics.logError(error);
369+
}
370+
371+
const result = response.data;
372+
373+
if (result) {
374+
if ('errors' in result) {
375+
const errors = values(result.errors)[0];
376+
if (Array.isArray(errors)) {
377+
if (errors[0]) {
378+
error.message = errors[0]; // eslint-disable-line no-param-reassign,prefer-destructuring
379+
}
380+
} else {
381+
error.message = errors; // eslint-disable-line no-param-reassign
382+
}
383+
} else if (result.error) {
384+
error.message = result.error; // eslint-disable-line no-param-reassign
385+
} else if (response.status === 413) {
386+
error.message = 'File too large, upload limit is 5MB.';
387+
}
388+
}
389+
390+
if (error.message.startsWith('You need to sign in to create more than')) {
391+
// Error for "You need to sign in to create more than 10 sandboxes"
392+
effects.analytics.track('Anonymous Sandbox Limit Reached', {
393+
errorMessage: error.message,
394+
});
395+
396+
effects.notificationToast.add({
397+
message: error.message,
398+
status: NotificationStatus.ERROR,
399+
actions: {
400+
primary: [
401+
{
402+
label: 'Sign in',
403+
run: () => {
404+
actions.internal.signIn({});
405+
},
406+
},
407+
],
408+
},
409+
});
410+
} else if (error.message.startsWith('You reached the maximum of')) {
411+
effects.analytics.track('Non-Patron Sandbox Limit Reached', {
412+
errorMessage: error.message,
413+
});
414+
415+
effects.notificationToast.add({
416+
message: error.message,
417+
status: NotificationStatus.ERROR,
418+
actions: {
419+
primary: [
420+
{
421+
label: 'Open Patron Page',
422+
run: () => {
423+
window.open(patronUrl(), '_blank');
424+
},
425+
},
426+
],
427+
},
428+
});
429+
} else if (
430+
error.message.startsWith(
431+
'You reached the limit of server sandboxes, you can create more server sandboxes as a patron.'
432+
)
433+
) {
434+
effects.analytics.track('Non-Patron Server Sandbox Limit Reached', {
435+
errorMessage: error.message,
436+
});
437+
438+
effects.notificationToast.add({
439+
message: error.message,
440+
status: NotificationStatus.ERROR,
441+
actions: {
442+
primary: [
443+
{
444+
label: 'Open Patron Page',
445+
run: () => {
446+
window.open(patronUrl(), '_blank');
447+
},
448+
},
449+
],
450+
},
451+
});
452+
} else {
453+
if (
454+
error.message.startsWith(
455+
'You reached the limit of server sandboxes, we will increase the limit in the future. Please contact [email protected] for more server sandboxes.'
456+
)
457+
) {
458+
effects.analytics.track('Patron Server Sandbox Limit Reached', {
459+
errorMessage: error.message,
460+
});
461+
}
462+
463+
effects.notificationToast.error(error.message);
464+
}
465+
};

‎packages/app/src/app/overmind/namespaces/editor/internalActions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ export const forkSandbox: AsyncAction<{
343343
effects.router.updateSandboxUrl(forkedSandbox);
344344
} catch (error) {
345345
console.error(error);
346-
effects.notificationToast.error('Sorry, unable to fork this sandbox');
346+
effects.notificationToast.error('We were unable to fork the sandbox');
347347
}
348348

349349
state.editor.isForkingSandbox = false;

0 commit comments

Comments
 (0)