0

Here is my code in typescript:

import _ from "lodash";

const waitWithPromise = (index, ms): Promise<void> =>
    new Promise((resolve) => {
        console.log(`${index}: starting ${Date.now()}`);

        setTimeout(() => {
            console.log(`${index}: ending ${Date.now()}`);
            resolve();
        }, ms);
    });

const waitWithAsync = (index: number, ms: number) => async (): Promise<void> => {
    await waitWithPromise(index, ms);
};

// eslint-disable-next-line @typescript-eslint/require-await
void (async (): Promise<void> => {
    const functionToPromiseArray = _.range(0, 9).map((index) => {
        return waitWithAsync(index, Math.random() * 1000);
    });

    let i = 0;
    const groupSize = 3;
    for (const functionToPromiseGroup of _.chunk(
        functionToPromiseArray,
        groupSize
    )) {
        console.log(`***********************
GROUP ${i + 1} / ${Math.ceil(functionToPromiseArray.length / groupSize)}
SIZE ${functionToPromiseGroup.length}
***********************`);

         await Promise.all(
             functionToPromiseGroup.map((functionToPromise) =>
                 functionToPromise()
             )
         );

        
        i++;
    }

    console.log(`finished! ${Date.now()}`);
})();

Output:

***********************
GROUP 1 / 3
SIZE 3
***********************
0: starting 1635274115798
1: starting 1635274115799
2: starting 1635274115799
0: ending 1635274116358
1: ending 1635274116375
2: ending 1635274116512
***********************
GROUP 2 / 3
SIZE 3
***********************
3: starting 1635274116512
4: starting 1635274116512
5: starting 1635274116512
4: ending 1635274116899
5: ending 1635274117094
3: ending 1635274117432
***********************
GROUP 3 / 3
SIZE 3
***********************
6: starting 1635274117432
7: starting 1635274117432
8: starting 1635274117432
8: ending 1635274117863
6: ending 1635274118051
7: ending 1635274118412
finished! 1635274118412

This appears to run the way I want: wait for one group to finish before moving on to the next. Here's the thing that makes me doubt that: if I replace the body of waitWithPromise to be this:

const waitWithPromise = (item, index): Promise<void> =>
    new Promise((resolve) => {
        console.log(`${index}: starting time=${Date.now()}`);

        void axios({
            method: "POST",
            url: "...",
            data: item.body,
            headers: { signature: item.signature },
        })
            .then((response) => {
                console.log(
                    `${index}: success ${JSON.stringify(response.status)}`
                );
            })
            .catch((error) => {
                console.log(
                    `${index}: fail ${JSON.stringify(
                        error
                    )} item=${JSON.stringify(item)} time=${Date.now()}`
                );
            })
            .finally(() => {
                resolve();
            });
    });

The output remains in the correct order, but a lot of the time, it pauses for a few seconds while one group completes, then I see a bunch of groups fly by in a flurry. It's almost like the next few groups already received their results, they were just waiting to print them.

Since this is making a call over the network, it doesn't make sense to me that it could complete 3+ groups -- in my real production code, there's 10 axios requests in each group -- faster than the human eye can see. They all succeed, btw; it's not like they're going fast because the server is rejecting them.

So, is this example working the way I think, or does it have some flaw?

Daniel Kaplan
  • 62,768
  • 50
  • 234
  • 356
  • Are the urls all different? If not, there may be caching at play. – trincot Oct 26 '21 at 20:48
  • @trincot the urls are the same, but the `data` is different every time. – Daniel Kaplan Oct 26 '21 at 20:51
  • 1
    Off-topic: your `fetch`y implementation of `waitWithPromise` has the [Explicit Promise construction antipattern](https://stackoverflow.com/q/23803743/8376184) – FZs Oct 26 '21 at 20:58
  • @FZs - the OP has "promisified" setTimeout. Returning `new Promise(...)` does not necessarily equal an explicit construction antipattern, and this is an example of an appropriate use. DanielKaplan, the redefinition as async is superfluous. async is just as well applied to the "promise" version of the function. – danh Oct 26 '21 at 21:05
  • @danh which `async` are you referring to? – Daniel Kaplan Oct 26 '21 at 21:17
  • @DanielKaplan `const waitWithPromise` could just as well be declared `const async waitWithPromise` and the second function could be removed. – danh Oct 26 '21 at 21:22
  • 2
    @danh I know that, but the example with `axios` (I accidentally wrote `fetch` instead) does already have a promise -> antipattern – FZs Oct 26 '21 at 21:46
  • @FZs afaict, it functions fine this way, it's just unnecessarily ugly? Just want to confirm that my question is still valid with this mistake, but I'm happy to fix it either way. – Daniel Kaplan Oct 26 '21 at 22:04
  • Shouldn't `functionToPromiseGroup.map((functionToPromise) => functionToPromise() )` be just `functionToPromiseGroup` – Anatoly Oct 26 '21 at 22:08
  • 2
    I'm sure what you are seeing is a connection being open which takes time. Then the connection is being reused for all subsequent requests. Easiest way to validate is to check your network tab. – Daniel Duong Oct 26 '21 at 22:29
  • 1
    @DanielKaplan As you've put `resolve()` in a `.finally` that always runs, it'll work this way. Take my note just to learn not do that in the future. – FZs Oct 27 '21 at 06:35
  • *"it pauses for a few seconds while one group completes*" - my guess is that actually most of the group's request are as quickly completed as the groups that "*fly by in a flurry*". There's only one or two requests in that group which actually take so long. As Daniel suggested, look at the waterfall diagram in the network tab, maybe post a screenshot. – Bergi Oct 27 '21 at 19:26
  • Btw, there's no reason for `waitWithAsync` to return an `async` function. Also the name should make clear that it's doing partial application and returns a function. I'd suggest `const makeWaiter = (index, ms) => () => waitWithPromise(index, ms);` – Bergi Oct 27 '21 at 19:28
  • Can you elaborate on why there's no reason for `waitWithAsync` to return an async function? – Daniel Kaplan Oct 28 '21 at 16:18

0 Answers0