DEV Community

Cover image for Preventing unhandled promise rejections in async functions
Gajus Kuizinas
Gajus Kuizinas

Posted on

Preventing unhandled promise rejections in async functions

#javascript

I am developing a HTTP proxy service and I have observed presence of an odd error message in my logs:

unhandledRejection RequestError: HTTP request error. at /dev/rayroute/raygun/src/factories/createRequest.js:107:13 at processTicksAndRejections (internal/process/task_queues.js:93:5) { code: 'RAYGUN_REQUEST_ERROR', originalError: Error: test at /dev/rayroute/raygun/src/factories/createRequest.js:73:29 at processTicksAndRejections (internal/process/task_queues.js:93:5)
Enter fullscreen mode Exit fullscreen mode

It is odd because there are plethora of tests to ensure that all errors are handled. It is also odd because I have never never seen unhandled rejection while developing the service (only ever saw it in production logs).

The relevant code looks like this:

const activeRequestHandler = createRequest(requestDefinition); if (incomingMessage.socket) { incomingMessage.socket.on('close', () => { if (responseIsReceived) { log.trace('client disconnected after response'); } else { log.debug('client disconnected'); activeRequestHandler.abort(new Error('CLIENT_DISCONNECTED')); } }); } try { await actions.afterRequestActions( context, requestDefinition, activeRequestHandler ); } catch (error) { log.error({ error: serializeError(error), }, 'afterRequest threw an error'); } try { responseDefinition = await activeRequestHandler.response; } catch (error) { log.warn({ error: serializeError(error), }, 'an error occurred while waiting for a HTTP response'); // [..] }
Enter fullscreen mode Exit fullscreen mode

It is pretty straightforward:

  • createRequest initiates a HTTP request and returns a request handler
  • the request handler can be used to abort the ongoing request (afterRequestActions aborts request after a hard-timeout); and
  • it is used to resolve the response or eventual rejection of the promise

I have written tests to ensure that errors are handled when:

  • request rejected
  • request aborted
  • afterRequestActions throws an error

, but all tests are passing.

🤔

It turns out that the problem was that in all my test cases actions.afterRequestActions was resolving/ being rejected before activeRequestHandler.response is resolved. Meanwhile, in production afterRequestActions contains logic that can take substantially longer to execute. I have also learned that even if you declare a try..catch block for your async function, if it resolves before it is await-ted, then you will get an unhandled rejection, i.e.

This code will not warn about unhandled rejection:

const delay = require('delay'); const main = async () => { const promise = new Promise((resolve, reject) => { setTimeout(() => { reject(new Error('Expected rejection.')); }, 100); }); await delay(90); try { await promise; } catch (error) { console.error(error) } }; main();
Enter fullscreen mode Exit fullscreen mode

But this code will always produce a warning about an unhandled rejection:

const delay = require('delay'); const main = async () => { const promise = new Promise((resolve, reject) => { setTimeout(() => { reject(new Error('Expected rejection.')); }, 100); }); await delay(110); try { await promise; } catch (error) { console.error(error) } }; main();
Enter fullscreen mode Exit fullscreen mode

The best solution is to add an auxiliary catch block, e.g. This is how I refactored my original code:

const activeRequestHandler = createRequest(requestDefinition); // Without this we were getting occasional unhandledRejection errors. // @see https://dev.to/gajus/handling-unhandled-promise-rejections-in-async-functions-5b2b activeRequestHandler.response.catch((error) => { log.warn({ error: serializeError(error), }, 'an error occurred while waiting for a HTTP response (early warning)'); }); if (incomingMessage.socket) { incomingMessage.socket.on('close', () => { if (responseIsReceived) { log.trace('client disconnected after response'); } else { log.debug('client disconnected'); activeRequestHandler.abort(new Error('CLIENT_DISCONNECTED')); } }); } try { await actions.afterRequestActions( context, requestDefinition, activeRequestHandler ); } catch (error) { log.error({ error: serializeError(error), }, 'afterRequest threw an error'); } try { responseDefinition = await activeRequestHandler.response; } catch (error) { log.warn({ error: serializeError(error), }, 'an error occurred while waiting for a HTTP response'); // [..] }
Enter fullscreen mode Exit fullscreen mode

Top comments (3)

Collapse
 
greim profile image
Greg Reimer • • Edited

Wow, it took me a bit to spot the problem before reading the rest of the post! I think there's a deeper problem, which is that createRequest() exposes a dangerous API to users by creating a reference to a promise and storing it implicitly.

const activeRequestHandler = createRequest(requestDefinition); // activeRequestHandler.response is a promise that's // now just hanging around, unhandled.

Anyone else using the API will have to be cognizant and use the same workaround. As an alternative, I like how the fetch API does things.

// creates and immediately handles a promise const resp = await fetch(...); // ...other logic runs here... // creates and immediately handles a promise const content = await resp.text();

Maybe createRequest() should change its API?

const activeRequestHandler = createRequest(requestDefinition); console.log(typeof activeRequestHandler.response); // function // other sync/async logic here... await activeRequestHandler.response();
Collapse
 
gajus profile image
Gajus Kuizinas •

For what it is worth, createRequest is just a light abstraction around got. It is got that uses a cancellable promise to implement cancel (/abort) functionality.

I do like your suggestion, though and I think it would be an improvement to got API. Please raise an issue with got. I am sure Sindre will appreciate the suggestion.

Collapse
 
gajus profile image
Gajus Kuizinas •

The requirement to process interceptors in series is very much intentional.