4

I use Promise.allSettled to fetch websites in batch fashion. It works fine if I limit size of website list to 10 but it become stuck as soon as I increase it to 1000.

This is weird and never happened to me. I've waited for three days until the script finish but it's still stuck at the first 1000 items.

const rp = require('request-promise');
const lineReader = require('line-by-line');

const reader = new lineReader("./all.json");

let lines = [];
const limit = 1000;

let successCount = 0;

reader.on('line', async (line) => {
    line = JSON.parse(line);

    lines.push(line);

    if (lines.length === limit) {
        reader.pause();

        let promises = [];

        for (const line of lines) {
            promises.push(rp(line.website))
        }
    
        await Promise.allSettled(promises);

        successCount++
        console.log(`Success Count is ${successCount}`);

        lines = [];

        reader.resume();
    }
});

The data.json file has website list under following format,

{ website: "https://www.google.com" }
{ website: "https://www.bing.com" }
{ website: "https://www.microsoft.com" }

You can reproduce the behavior by duplicating a line like { website: "https://www.google.com" } 2000 times.

jeffbRTC
  • 1,941
  • 10
  • 29
  • @trincot No. It's 1000 concurrent requests and it's stuck forever. These requests goes for diff websites not a one but I found that you can reproduce the behavior by duplicating a line like `{website: "https://www.google.com"}` 2000 times – jeffbRTC Mar 21 '21 at 14:57
  • Would the following also hang forever? You have more than `Promise.allSettled` going one in your code `Promise.allSettled(Array.from({length: 2000}, () => 'https://www.google.com').map(x => fetch(x))).then(() => console.log('done'))` – geoffrey Mar 21 '21 at 15:15
  • @geoffrey Yours works but I have no idea what's wrong with my code. I have shorten the code now and edited the question. Do you see anything wrong with that? – jeffbRTC Mar 21 '21 at 15:50
  • @geoffrey I use line-by-line package to read thru lines and push to the Lines array. Once the Lines array hit the limit I specified the line-by-line reader become paused and I then loop over each item on Lines array and perform fetch request then push to `promises` array and then I invoke `Promise.allSettled` for the array containing promises. – jeffbRTC Mar 21 '21 at 15:54
  • Can you try with `const promises = lines.map((line, i) => rp(line.website).then(r => console.log(i+' fulflilled'), e => console.log(i+' rejected', e)));`? That should give you some insight into where it's stopping – Bergi Mar 21 '21 at 15:56
  • @Bergi I did and there were some `500` error codes, but this should not stuck `Promise.allSettled` because `Promise.allSettled`'s design is to skip these errors unlike `Promise.all` which stop at errors. – jeffbRTC Mar 21 '21 at 16:03
  • @Bergi By the way, yours didn't stuck the execution. – jeffbRTC Mar 21 '21 at 16:06
  • @jeffbRTC The more important question is whether there were 1000 responses, or whether `rp` timed out(?) without settling the promise(!?). – Bergi Mar 21 '21 at 16:06
  • Also try `reader.on('error', console.error)`. And there's a design mistake with your reader: the last batch will never be sent if its size doesn't hit exactly the limit. You still will need to listen for the `end` event and then send the requests for the last array of `lines`. – Bergi Mar 21 '21 at 16:09
  • @Bergi I ran your code again and it become stuck at `220 rejected RequestError: Error: connect ETIMEDOU` – jeffbRTC Mar 21 '21 at 16:11
  • This means you probably can't/shouldn't use 1000 parallel connections. But yeah, still the `Promise.allSettled` should complete and continue the reader - if you say the other 780 request promises are neither fulfilled nor rejected after one timed out, that sounds like a serious bug in that library. – Bergi Mar 21 '21 at 16:33
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/230187/discussion-between-jeffbrtc-and-bergi). – jeffbRTC Mar 21 '21 at 16:40
  • 1
    @Bergi Seems there is some bug in lib. Found an another question. https://stackoverflow.com/q/56320940/14659574 – jeffbRTC Mar 21 '21 at 16:54

1 Answers1

5

The solution is to wrap up my promise with a promise that timeout after certain period.

Here is the function that provides the ability timeout the promise,

const withTimeout = (millis, promise) => {
    const timeout = new Promise((resolve, reject) =>
        setTimeout(
            () => reject(`Timed out after ${millis} ms.`),
            millis));
    return Promise.race([
        promise,
        timeout
    ]);
};

You can use it like below,

await Promise.allSettled([
   withTimeout(5000, rp("https://www.google.com"),
   withTimeout(5000, rp("https://www.bing.com"),
]));

I know you can specify a timeout in the request-promise options, but for some reason it didn't work for me, but I noticed certain improvements when I added it.

jeffbRTC
  • 1,941
  • 10
  • 29