3

I am new to NodeJS and to Promise functionality so please be polite if this is an ignorant question.

I'm trying first read a database of records and then check the links actually work (looking for a 200 response). For my current test data this should will always return a 200 response. I'm getting 302 (too many requests) response and then the development server crashes. I need to slow down how I send through requests to the database but I cannot work out how to do it. It seems to me the promise simply sends everything as soon as it is resolved.

I have tried building in time delays in the then section, but to no avail. Here's the code:

var http404Promise = new Promise(function(resolve, reject) {
        var linkArray = new Array()

        db.sequelize.query(photoQuery, {
            replacements: queryParams
        }).spread(function(photoLinks) {
            photoLinks.forEach(function(obj) {
                var siteLink = hostname + 'photo/' + obj.img_id
                linkArray.push(siteLink)

                //console.log(siteLink);
            });

            resolve(linkArray);
        });
    });

http404Promise.then(function(linkArray) {
    linkArray.forEach(function(element) {
        console.log(element);
        http.get(element, function(res) {
            console.log(element);
            console.log("statusCode: ", res.statusCode); // <======= Here's the status code
        }).on('error', function(e) {
            console.log(element);
            console.error(e);
        })
    })    
});
HenryM
  • 5,557
  • 7
  • 49
  • 105
  • 1
    Thank you to all respondents. I'm sure they all work but I marked correct the one I've used – HenryM Apr 02 '18 at 07:52
  • 1
    302 is "redirect", not "too many requests" - that's 429. – Tomalak Apr 02 '18 at 08:36
  • 1
    Also, you are committing the [Explicit Promise Creation antipattern](https://stackoverflow.com/q/23803743/18771). Don't use `new Promise()` with functions that already return promises - simply keep using the promises you already have. – Tomalak Apr 02 '18 at 09:20

4 Answers4

1

The reason a normal timeout in the forEach doesn't work, is that the forEach does not wait for the timeout to finish. So every request waits simultaneously, and you don't get the staggering you want.
You can however use the index of each element to calculate the timeout.

linkArray.forEach(function(element, index) {
    setTimeout(function() {
        console.log(element);
        http.get(element, function(res) {
            console.log(element);
            console.log("statusCode: ", res.statusCode); // <======= Here's the status code
        }).on('error', function(e) {
            console.log(element);
            console.error(e);
        });
    }, index * 500);
});
Tomalak
  • 332,285
  • 67
  • 532
  • 628
lukas-reineke
  • 3,132
  • 2
  • 18
  • 26
  • Its not the most elegant solution, but if the backend restricts on time, this is the only solution that will reliably work. If we just wait until the previous request is done, it might sometimes work if the request was slow, and sometimes not if it was fast. – lukas-reineke Apr 02 '18 at 08:00
  • of course this is not something you should do in production, but the question states development server, and it should be just fine for that. – lukas-reineke Apr 02 '18 at 08:03
  • If the backend has a restriction on how many requests can be made in a certain amount of time, there is nothing you can with promises or callbacks to check this in the frontend other than waiting a certain amount of time. A better solution would be to question why you need to make so many request that the backend can't handle them in the first place, but this is a different question. – lukas-reineke Apr 02 '18 at 08:21
  • 1
    Hm, you're right. I'll revert my vote. I was too fixed on the HTTP request+timeout antipattern. – Tomalak Apr 02 '18 at 08:37
  • totally understand, its not a nice solution. But I can't think of anything better in this situation. – lukas-reineke Apr 02 '18 at 08:44
  • It would certainly be more involved, and it would in part depend on the server behavior, i.e. whether it blocks on too many concurrent requests or on too many within a certain amount of time. Concurrency issues could be solved with [Bluebird#map](http://bluebirdjs.com/docs/api/promise.map.html) with a `concurrency` parameter (`request-promise` is based on Bluebird and using a promise-based Ajax library here would be a good thing anyway). Timing issues could be solved with `Promise.delay`. Maybe a combination of both. It depends. – Tomalak Apr 02 '18 at 08:51
  • True, it depends. If it is concurrency, it could be solved by just chaining the promises as well. But all of these solutions won't scale with more users. The only real solution is fixing the underlying problem of having so many requests. – lukas-reineke Apr 02 '18 at 08:59
  • Chaining them would mean concurrency=1, which will certainly never hit a limit but would also be a couple of times slower than it would need to be. For the a test, the quick-and-dirty approach is actually all-right. For production a smarter/more informed approach should be taken. – Tomalak Apr 02 '18 at 09:18
0
linkArray.forEach(function(element) {
        .....
    })

Here instead of doing forEach you can use Async eachLimit to do a few of the requests at a time. You can also write the code such that the next request is done after the previous one completes or use Async eachSeries but with eachLimit you have a bit of parallelism.

rahulroy9202
  • 2,730
  • 3
  • 32
  • 45
0

You can use async module for this

https://github.com/caolan/async

or if you wish to go with current code then you need to change the follow

linkArray = [];
var counter = 0;
http404Promise.then(function(responseData) {
  linkArray  = responseData;
  if (linkArray.length > 0) {
    requestHandler(linkArray.pop());
  }
});

function requestHandler(data) {
  http
    .get(data, function(res) {
      counter++;
      if (counter == linkArray.length) {
        console.log("finished");
      } else {
        reuqestCheckPromiss(linkArray.pop());
      }
    })
    .on("error", function(e) {
      counter++;
      if (counter == linkArray.length) {
        console.log("finished");
      } else {
        reuqestCheckPromiss(linkArray.pop());
      }
    });
}
Nishant Dixit
  • 5,388
  • 5
  • 17
  • 29
0

You can use reduce function to create pipe of delayed requests

    const timeout = 500;
    const linkArray = [1,2,3,4]
    
    const httpFunc = el => {
      // your http function here
      console.log('request num ', el)
    }
    
    
    const func = el => new Promise(resolve => {
      setTimeout(() => {
        resolve(httpFunc(el));
      }, timeout)
    })
    
    linkArray.reduce((acc, el) => acc.then(() => func(el)), Promise.resolve())
Bartłomiej Gładys
  • 4,525
  • 1
  • 14
  • 24