using async/await in a filter operation not waiting to finish after first await call

Question

For a specific function, a list of tags (rfids) are required from a database, but various tables should be checked.

The issue is centered around the problem of a long running operation isn't awaited upon to finish before proceeding to the next.

Problem:

The code called below runs as follows (# numbers correspond to number in code below):

getAllTags() is called, starting the proc
await db.dbRfid.findAll() returns with X amount of valid results (not Promise objects)
start filtering call on valid objects & await finish (await place since some function calls inside are async calls.
call const driverTags = await tag.getDrivers();

At this point, one would expect this function to return the result and proceed to the next function i.e.

// #5
const truckTags = await tag.getTrucks();

What infact happens is for each of the allTags items, the getDrivers() gets called and the filter() exits and the code continues on by executing this next:

// #6
if (pureTags) {
    return filteredTags;
}

Question:

To my understanding, I am awaiting async operations correctly, yet it seems like the filter only accepts/allows one async operation.

I assume I did something wrong, yet I am unable to the cause of the problem. Any advice would be appreciated!

Full code implementation below:

Called with e.g.

const listOfTags = await getAllTags();

Specific tag requirements (all valid, unused, enabled & unrevoked tags)

const listOfTags = await getAllTags(false, true, true, true);

Problem code:

// #1
const getAllTags = async (pureTags = false, unused = true, enabled = true, notRevoked = false) => {
    // #2
    let allTags = await db.dbRfid.findAll()
    // #3
    let filteredTags = await allTags.filter(async tag => {
        // check tag used
        if (unused) {
            // #4
            const driverTags = await tag.getDrivers();
            // #5
            const truckTags = await tag.getTrucks();
            const userTags = await tag.getUsers();
            if (driverTags.length > 0) {
                return false;
            }
            if (truckTags.length > 0) {
                return false;
            }
            if (userTags.length > 0) {
                return false;
            }
        }

        // check tag enabled
        if (enabled && !tag.enabled) {
            return false;
        }

        // check tag revoked or return true
        return notRevoked && !tag.revoked;
    });

    // return tags as is
    // #6
    if (pureTags) {
        return filteredTags;
    }

    return filteredTags.map(tag => {
        return {
            id: tag.id,
            rfid: tag.rfid,
            expiryDate: tag.expiryDate,
            revoked: tag.revoked,
            enabled: tag.enabled
        }
    });
}

Update

I should mention:

no 'errors' of any sort are shown to indicate of some problem.
the .getDrivers() is a getter created by sequelize which returns a promise.

Update 2

After comment by evgeni fotia

I originally had this code, however chose not to include it as it may havve complicated matters. However, this it the original code I attempted with the addition of the await Promise.all().

const getAllTags = async (pureTags = false, unused = true, enabled = true, notRevoked = false) => {
    let allTags = await db.dbRfid.findAll()
    let filteredTags = await Promise.all(
        // await allTags.filter(async tag => {      //tried the await here too - for good measure
        allTags.filter(async tag => {
            // check tag used
            if (unused) {
                const driverTags = await tag.getDrivers();
                const truckTags = await tag.getTrucks();
                const userTags = await tag.getUsers();
                if (driverTags.length > 0) {
                    return false;
                }
                if (truckTags.length > 0) {
                    return false;
                }
                if (userTags.length > 0) {
                    return false;
                }
            }

            // check tag enabled
            if (enabled && !tag.enabled) {
                return false;
            }

            // check tag revoked or return true
            return notRevoked && !tag.revoked;
        })
    );

    // return tags as is
    if (pureTags) {
        return filteredTags;
    }

    return filteredTags.map(tag => {
        return {
            id: tag.id,
            rfid: tag.rfid,
            expiryDate: tag.expiryDate,
            revoked: tag.revoked,
            enabled: tag.enabled
        }
    });
}

Please note, after running the code in this update or in the original question, I see the debugger hitting the getDrivers() method, then the (HTTP) response is sent to the client, and only after 0.5~1s I see the getDrivers() method returning and proceeding to the next method.

the same problem I believe https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop — evgeni fotia, Aug 30 '20 at 13:38
@evgenifotia yup, know about and tried the `Promise.all`. Same result, I opted to show only the filter part to be more concise. Will post the `Promise.all` as an update to address any questions or suggestions made for it. — CybeX, Aug 30 '20 at 13:42
You can't just use an `async` function as a callback to something that won't do anything useful with the promise the `async` function returns. `filter` just checks the return to see if it's a truthy value -- **all** promise instances are truthy values. If you want to do all the async work in parallel, you can use `map` to build an array of promises and then `Promise.all` to wait for them all to settle. If you want to do them one after another, use a `for` or `for-of` loop and `push` to your result array. — T.J. Crowder, Aug 30 '20 at 13:43
FWIW, something along these lines: https://pastebin.com/JvE21Yja — T.J. Crowder, Aug 30 '20 at 13:47
@T.J.Crowder thank you for your advice. I wanted to use `map` however, I felt this will slow down performance due to having to call `getDrivers()`, then `getTrucks()`, etc w/o checking if `drivers.length > 0`. I don't show that here as I modified it to check if I am getting results, but that was the original intention. Seems like I will need to make `3 x X` calls regardless which is what I was hoping to avoid. — CybeX, Aug 30 '20 at 13:52
@T.J.Crowder or your example doing all in the `map` followed by `filter()` is a nifty solution, thank you! — CybeX, Aug 30 '20 at 13:53
@CybeX - You're welcome! You can (and yes, probably should) make the `if`s that check those more proactive: https://pastebin.com/YHJTTPFL Truth be told, I didn't read the code closely at all, just wanted to provide a sketch. :-) — T.J. Crowder, Aug 30 '20 at 14:13
On an unrelated note, doing a `.findAll()` on the database and filtering the entire result set on the client end seems enormously wasteful. Try to figure out a way to let the database filter most of these things. — Tomalak, Aug 31 '20 at 09:42
On a second unrelated note, if your tags have a `revoked` property, it feels kind of weird to have a `notRevoked` function argument. Having `revoked = false` as the argument would feel more natural to me, especially since the other argument isn't called `notDisabled`, either. — Tomalak, Aug 31 '20 at 10:53

Tomalak · Accepted Answer · 2020-09-01T08:30:01.520

Your central problem is, as T.J. Chowder has commented, that you are trying to .filter() on something that is sometimes a boolean, and sometimes a promise.

You should do the mapping and the filtering in different steps. I tend to prefer the .then() syntax, so here's my approach:

const getAllTags = (pureTags = false, unused = true, enabled = true, notRevoked = false) => {
    return db.dbRfid.findAll()
        .then(allTags => allTags.map(tag => Promise.resolve(
            enabled === tag.enabled && notRevoked === !tag.revoked && Promise.all([
                tag.getDrivers(), tag.getTrucks(), tag.getUsers()
            ]).then(results => results.some(r => r.length))
        ).then(ok => ok ? tag : null)))
        .then(pendingTags => Promise.all(pendingTags))
        .then(resolvedTags => resolvedTags.filter(tag => tag))
        .then(filteredTags => filteredTags.map(tag => pureTags ? tag : {
            id: tag.id,
            rfid: tag.rfid,
            expiryDate: tag.expiryDate,
            revoked: tag.revoked,
            enabled: tag.enabled
        }));
};

The logic of this code:

fetch tags from DB
produces Promise<Tag[]>, i.e. a promise for an array of tags
map each tag to a new promise that resolves either to the tag itself, or null, depending on a condition we figure out using Promise.resolve(), to account for the potentially^(*) async nature of that check
produces Promise<Promise<Tag|null>[]>
wait for all those inner promises to resolve using Promise.all()
produces <Promise<(Tag|null)[]>
filter out the null values (i.e. tags we don't want)
produces <Promise<Tag[]>
map the tags to the overall result data structure we want, depending on pureTags
produces <Promise<(Tag|object)[]>

^(*) The Promise.all() is only invoked if the preconditions check out, due to short circuiting. If not, it's simply a Promise.resolve(false), which resolves to false immediately. In the other case it's going to be Promise.resolve(<Promise<Boolean>), which works exactly the same way as Promise.resolve(Boolean). This way we can unify the synchronous and the asynchronous test.

In the subsequent .then() we can decide whether to return the tag - or null, to indicate that this tag failed the filter conditions. The null values are then picked out by resolvedTags.filter(tag => tag).

Note that your code serializes the three async checks (getDrivers, getTrucks, getUsers) because each is awaited before the next is started. Using Promise.all() lets them run in parallel, which is one of the reasons why I dislike using async/await for everything.

I leave rewriting this into async/await style as an exercise.

thanks for the exercise (think I already did it...almost ;) ) — CybeX, Aug 31 '20 at 14:10

using async/await in a filter operation not waiting to finish after first await call

1 Answers1