Is it an anti-pattern to use async/await inside of a new Promise() constructor?
Solution 1
You're effectively using promises inside the promise constructor executor function, so this the Promise constructor anti-pattern.
Your code is a good example of the main risk: not propagating all errors safely. Read why there.
In addition, the use of async
/await
can make the same traps even more surprising. Compare:
let p = new Promise(resolve => {
""(); // TypeError
resolve();
});
(async () => {
await p;
})().catch(e => console.log("Caught: " + e)); // Catches it.
with a naive (wrong) async
equivalent:
let p = new Promise(async resolve => {
""(); // TypeError
resolve();
});
(async () => {
await p;
})().catch(e => console.log("Caught: " + e)); // Doesn't catch it!
Look in your browser's web console for the last one.
The first one works because any immediate exception in a Promise constructor executor function conveniently rejects the newly constructed promise (but inside any .then
you're on your own).
The second one doesn't work because any immediate exception in an async
function rejects the implicit promise returned by the async
function itself.
Since the return value of a promise constructor executor function is unused, that's bad news!
Your code
There's no reason you can't define myFunction
as async
:
async function myFunction() {
let array = await getAsyncArray();
return new Promise((resolve, reject) => {
eachLimit(array, 500, (item, callback) => {
// do other things that use native promises.
}, error => {
if (error) return reject(error);
// resolve here passing the next value.
});
});
}
Though why use outdated concurrency control libraries when you have await
?
Solution 2
I agree with the answers given above and still, sometimes it's neater to have async inside your promise, especially if you want to chain several operations returning promises and avoid the then().then()
hell. I would consider using something like this in that situation:
const operation1 = Promise.resolve(5)
const operation2 = Promise.resolve(15)
const publishResult = () => Promise.reject(`Can't publish`)
let p = new Promise((resolve, reject) => {
(async () => {
try {
const op1 = await operation1;
const op2 = await operation2;
if (op2 == null) {
throw new Error('Validation error');
}
const res = op1 + op2;
const result = await publishResult(res);
resolve(result)
} catch (err) {
reject(err)
}
})()
});
(async () => {
await p;
})().catch(e => console.log("Caught: " + e));
- The function passed to
Promise
constructor is not async, so linters don't show errors. - All of the async functions can be called in sequential order using
await
. - Custom errors can be added to validate the results of async operations
- The error is caught nicely eventually.
A drawback though is that you have to remember putting try/catch
and attaching it to reject
.
Solution 3
BELIEVING IN ANTI-PATTERNS IS AN ANTI-PATTERN
Throws within an async promise callback can easily be caught.
(async () => {
try {
await new Promise (async (FULFILL, BREAK) => {
try {
throw null;
}
catch (BALL) {
BREAK (BALL);
}
});
}
catch (BALL) {
console.log ("(A) BALL CAUGHT", BALL);
throw BALL;
}
}) ().
catch (BALL => {
console.log ("(B) BALL CAUGHT", BALL);
});
or even more simply,
(async () => {
await new Promise (async (FULFILL, BREAK) => {
try {
throw null;
}
catch (BALL) {
BREAK (BALL);
}
});
}) ().
catch (BALL => {
console.log ("(B) BALL CAUGHT", BALL);
});
Related videos on Youtube
Comments
-
Alexis Tyler over 2 years
I'm using the
async.eachLimit
function to control the maximum number of operations at a time.const { eachLimit } = require("async"); function myFunction() { return new Promise(async (resolve, reject) => { eachLimit((await getAsyncArray), 500, (item, callback) => { // do other things that use native promises. }, (error) => { if (error) return reject(error); // resolve here passing the next value. }); }); }
As you can see, I can't declare the
myFunction
function as async because I don't have access to the value inside the second callback of theeachLimit
function.-
zerkms about 7 years"As you can see, i can't declare the myFunction as async" --- can you elaborate more?
-
Admin about 7 yearsOh, ok... sorry. I need the constructor because i need the async.eachLimit to avoid more than 500 asynchronous operations at a time. I'm downloading and extracting data from text files and i want avoid to much asynchronous operations, after i extract the data, i must return a Promise with the data, and i wont be able to return it from the callback of the async.eachLimit.
-
slebetman about 7 years1. Why do you need the await? Async is already a control-flow mechanism. 2. If you want to use async.js with promises inside node.js take a look at async-q
-
Admin about 7 yearsTo avoid callback hell, and if something throws, the outer promise would catch.
-
-
lonesomeday about 7 yearsYou don't need
return await
:return new Promise
is sufficient. -
Admin about 7 yearsBecause i need control the maximum number of async operations at a time. Do you know any other way to do it without the eachLimit? @jib
-
Admin about 7 yearsI need this because i'm spawning child process, and if i don't control the number, this will cause OS exceptions. Sometimes i need open 10.000 child process's.
-
Bergi about 7 yearsI officially approve this answer, I'd have said exactly the same :-)
-
Bergi about 7 years@celoxxx Have a look here. You indeed should never use async.js with promises
-
Admin about 7 yearsI don't understand TypeScript @Bergi.
-
Admin about 7 yearsI'm a bit curious... why not use async + promises? Promise.all won't resolve the problem of limiting the number of async operations.
-
Admin about 7 yearsThat's not the first time i see you saying it's wrong use the Promise + Async module. I really would like to understand what problems would case this.
-
Bergi about 7 years@celoxxx Just drop the types and it becomes plain js. You should not use async.js because the different interfaces - node-style callbacks vs promises - cause too much friction and lead to unnecessary complicated and error-prone code.
-
Admin about 7 yearsI agree with you... But this code is old, and i'm refactoring to use events + async.js (to control the limit of async, yet. If you know a better way, please say).
-
Matthew Rideout over 5 yearsI agree with user5487299 - if you have100,000 tasks that need to be run, then you don't necessarily want synchronous execution. But you can't execute them all at once either. A library like p-limit allows you to specify the appropriate amount of concurrency to your desired resource usage and speed.
-
PrestonDocks over 3 yearsSo in your example, will
axios.get(url)
function as though it was called asawait axios.get(url)
? -
CherryDT about 3 yearsNo it won't,
res
will contain a promise and rest of the code will fail sinceres.data
will be undefined. -
GorvGoyl almost 3 yearsit gives linting error:
Promises must be handled appropriately or explicitly marked as ignored with the void operator
github.com/typescript-eslint/typescript-eslint/blob/v4.28.0/… -
noseratio almost 3 yearsWhile this works, you might as well get rid of the wrapping promise, try/catch etc, and do the same with the remaining of your IEFE function: i.stack.imgur.com/S3pU2.png
-
Vladyslav Zavalykhatko almost 3 years@noseratio can't agree more. the op though asked if it's okay to use
async
inside of aPromise
body. -
2Toad over 2 yearsLove this philosophy!