Skip to content

Commit

Permalink
fix: application crash when timeout encountered (#569)
Browse files Browse the repository at this point in the history
I encountered occasional application crashes and traced them back to timeouts coming from the Nylas API. After reviewing the Nylas-SDK, I found the culprit in the `setTimeout` call used to abort the request. It's bad practice to throw an error from `setTimeout` as its `try...catch` blocks will not intercept the error. Instead, these errors will crash the application.

`node-fetch` will reject when `controller.abort()` is called, so we don't need to throw from the timeout anyway.

Reference: 
https://jaketrent.com/post/catch-error-thrown-settimeout/
https://nodejs.org/dist/latest-v7.x/docs/api/errors.html#errors_node_js_style_callbacks
  • Loading branch information
mmollick authored Jul 9, 2024
1 parent 4b964ed commit e0cd773
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/apiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ export default class APIClient {
const controller: AbortController = new AbortController();
const timeout = setTimeout(() => {
controller.abort();
throw new NylasSdkTimeoutError(req.url, this.timeout);
}, this.timeout);

try {
Expand Down Expand Up @@ -187,6 +186,10 @@ export default class APIClient {

return response;
} catch (error) {
if (error instanceof Error && error.name === 'AbortError') {
throw new NylasSdkTimeoutError(req.url, this.timeout);
}

clearTimeout(timeout);
throw error;
}
Expand Down

0 comments on commit e0cd773

Please sign in to comment.