0

Ive been learning about promises and I have a question. I have a function named getNumber which return an array of number (for the sake of understanding). The I used that function to iterate over that array and make a http request for each value (with a setTimeout to make a delay between calls)

Then I want to used that information gathered in a then function, but it's giving me a 'undefined error'. obviously something is wrong here, but I cant see it. Do you guy know how can I fix this and what is wrong?

var getNumbers  = () => {
  return new Promise(function(resolve, reject) {

    console.log("In function getNumbers");
    var data = [1,2,3,4,5,6,7,8,9];
    resolve(data);
  });
};


getNumbers()

    .then(numbersArray => {
        //Supposed to return array of posts title
        return numbersArray.map(number => {
          console.log("Reading number" + number);

            setTimeout(() => {
                //make a http request
                return getHtml("https://jsonplaceholder.typicode.com/posts/"+number)
                    .then(function(post) {
                        return post.title;
                    })

            }, 10000);//make a request each ten seconds
        });
    })
    .then(postTitlesArray => {
    //Shows array of undefined
      console.log(postTitlesArray)

    });



function getHtml(webUrl) {
    return fetch(webUrl)
        .then(function(res) {
            return res.json();
        });
}
Bharata
  • 11,779
  • 6
  • 27
  • 42
  • `setTimeout()` is not blocking. It doesn't "wait" for the timer. It just schedules an event for some time in the future and just keeps executing the rest of your script. So, putting a `setTimeout()` inside a `.map()` doesn't accomplish anything. Your `.map()` callback returns nothing. The `return` inside of `setTimeout()` goes nowhere. – jfriend00 Jan 15 '18 at 01:47
  • 1
    You may be interested in this: [using setTimeout on promise chain](https://stackoverflow.com/questions/39538473/using-settimeout-on-promise-chain/39538518#39538518) if you're trying to delay a `.then()` handler. – jfriend00 Jan 15 '18 at 01:48
  • What are you really trying to accomplish with the delay? – jfriend00 Jan 15 '18 at 01:49
  • I have to wait like 10 seconds to make requests, if I dont do this, I can get blocked. – Joel Alvarez Myrrie Jan 15 '18 at 02:35
  • OK, now that I understand that, I wrote an answer with a couple different methods to both sequence the async iteration of the array and to insert the delay. – jfriend00 Jan 15 '18 at 03:52

3 Answers3

2

There are several conceptual things in the way of your approach doing what you want.

First, .map() is synchronous. That means it runs to completion and doesn't wait for any async operations to finish.

Second, setTimeout() is non-blocking. It just schedules a timer for some time in the future and then your .map() callback returns immediately, returning nothing.

So, your approach doesn't work at all.

From your comments, it appears that what you're trying to accomplish is to make a bunch of network calls in a loop, but put a delay between them so you don't get rate limited. There are a bunch of ways to do that.

There are two basic concepts you need to make that work:

  1. Make your async operations be sequential so the next one doesn't get initiated until the prior one is done.

  2. Put a delay that works with promises before starting the next one.

I'll first show an ES7 approach using async/await as it probably looks conceptually the simplest.

Using async/await to sequence asynchronous array access

function delay(t) {
    return new Promise(resolve => {
        setTimeout(resolve, t);
    });
}

getNumbers().then(async function(numbersArray) {
    //Supposed to return array of posts title
    let results = [];
    let delayT = 0;    // first delay is zero
    for (let number of numbersArray) {
        console.log("Reading number" + number);
        let r = await delay(delayT).then(() => {
            delayT = 10 * 1000;   // 10 seconds for subsequent delays
            return getHtml("https://jsonplaceholder.typicode.com/posts/"+number).then(function(post) {
                return post.title;
            });
        });
        results.push(r);
    }
    return results;
});

Using .reduce() to sequence asynchronous array acess

If you wanted to do it without async/await, then you could use the .reduce() design pattern for sequencing async iteration of an array:

function delay(t) {
    return new Promise(resolve => {
        setTimeout(resolve, t);
    });
}

getNumbers().then(numbersArray => {
    //Supposed to return array of posts title
    let results = [];
    let delayT = 0;    // first delay is zero
    return numersArray.reduce((p, number) => {
        return p.then(() => {
            return delay(delayT).then(() => {
                delayT = 10 * 1000;   // 10 seconds for subsequent delays
                return getHtml("https://jsonplaceholder.typicode.com/posts/"+number).then(function(post) {
                    results.push(post.title);
                });
            });
        });
    }, Promise.resolve()).then(() => {
        // make array of results be the resolved value of the returned promise
        return results;
    });
});

Note that both of these algorithms are coded to not delay the first operation since presumably you don't need to, so it only delays between successive operations.


As coded, these are modeled after Promise.all() and they will reject if any of your getHtml() calls reject. If you want to return all results, even if some reject, then you can change:

return getHtml(...).then(...)

to

return getHtml(...).then(...).catch(err => null);

which will put null in the returned array for any result that failed, or if you want to log the error, you would use:

return getHtml(...).then(...).catch(err => {
    console.log(err);
    return null;
});

Generic Helper Function

And, since this is a somewhat generic problem, here's a generic helper function that lets you iterate an array calling an async operation on each item in the array and accumulating all the results into an array:

// Iterate through an array in sequence with optional delay between each async operation
// Returns a promise, resolved value is array of results
async iterateArrayAsync(array, fn, opts = {}) {
    const options = Object.assign({
        continueOnError: true, 
        delayBetweenAsyncOperations: 0,
        errPlaceHolder: null
    }, opts);
    const results = [];
    let delayT = 0;      // no delay on first iteration
    for (let item of array) {
        results.push(await delay(delayT).then(() => {
            return fn(item);
        }).catch(err => {
            console.log(err);
            if (options.continueOnError) {
                // keep going on errors, let options.errPlaceHolder be result for an error
                return options.errPlaceHolder;
            } else {
                // abort processing on first error, will reject the promise
                throw err;
            }
        }));
        delayT = options.delayBetweenAsyncOperations;   // set delay between requests
    }
    return results;
}

This accepts options that let you continueOnError, lets you set the delay between each async operation and lets you control the placeholder in the array of results for any failed operation (only used if continueOnError is set). All the options are optional.

jfriend00
  • 580,699
  • 78
  • 809
  • 825
  • Why can you use 'p.then' in the reduce part? – Joel Alvarez Myrrie Jan 15 '18 at 15:00
  • @JoelAlvarezMyrrie - `p.then()` is what makes thing sequential. That makes the next async operation wait for the previous one to finish before it runs. `.reduce()` passes a value from one iteration of the loop to the next. In this case, we're passing a new promise from one to the next so that the next iteration will wait for this iteration's promise to be done before it does its work. We're essentially doing `Promise.resolve().then(doFirstIteration).then(doSecondIteration).then(doThirdIteration).then(...)`. – jfriend00 Jan 15 '18 at 19:12
1

I assume what you want to do is: 1) Get a list of numbers using getNumbers. 2) Iterate through each number from step one and form a url, with which an http request is made every ten seconds. 3) If a request is successfully sent, wait for its response. 4) Get post.title from response. 5) Wait until the iteration in step 2 ends, and return an array of all post.titles received from each call.

With the above assumptions in mind, I edit your code a bit and the following solution will work. See in jsfiddle.

I think the main problem with your code is that the map method doesn't return anything.

const getNumbers  = () => {
  return new Promise(function(resolve, reject) {

    console.log("In function getNumbers");
    var data = [1,2,3,4,5,6,7,8,9];
    resolve(data);
  });
};

const delay = (number, t) => {
    return new Promise((resolve) => {
      setTimeout(() => { 
        //make a http request
        resolve(
          getHtml("https://jsonplaceholder.typicode.com/posts/"+number)
          .then(function(post) {
            console.log('title', post.title)
            return post.title;
          })
        )
      }, t)
    })

}

const getHtml = (webUrl) => {
    return fetch(webUrl)
        .then(function(res) {
            return res.json();
        });
}

getNumbers()
    .then(numbersArray => {
        //Supposed to return array of posts title
        return Promise.all(numbersArray.map((number, i) => {
          console.log("Reading number" + number);
          return delay(number, 10000*(i+1));//make a request each ten seconds
        }))
        .then(postTitlesArray => {
            console.log(postTitlesArray)
            });
    })
Liutong Chen
  • 1,884
  • 2
  • 15
  • 21
  • This doesn't make a request every 10 seconds. It schedules all your rqeuests to be done at once 10 seconds from now. `.map()` is synchronous so it does a whole bunch of `delay(number,10)` calls at once. They all get schedule for 10 seconds from now. I don't think that solves the OP's problem at all. – jfriend00 Jan 15 '18 at 03:34
  • @jfriend00 Thanks for pointing it out. I have edit my answer and use `map` functions's second parameter `i` to set different timer for each call. I have tested it and it works as expected now. – Liutong Chen Jan 15 '18 at 03:56
  • It's pretty weird to be doing `resolve(getHtml(...).then())`. Usually, you would put the call to `resolve(post.title)` inside the `.then()` handler. But, really it's probably better to have a generic `delay()` function and use that in combination with `getHtml()` (as shown in my answer). – jfriend00 Jan 15 '18 at 04:11
0

You can use Promise.all, assuming numbers are not in the thousands or else you can use batched Promise.all.

Then use throttlePeriod from here to make sure only 1 request is made every 10 seconds.

And then resolve failed requests with a special value so you don't loose all successes if one fails:

var getNumbers = () => {
  return new Promise(function (resolve, reject) {

    console.log("In function getNumbers");
    var data = [1, 2, 3, 4, 5, 6, 7, 8, 9];
    resolve(data);
  });
};

function getHtml(webUrl) {
  return fetch(webUrl)
    .then(function (res) {
      return res.json();
    });
}

const Fail = function(reason){this.reason=reason;};
const isFail = x=>(x&&x.constructor)===Fail;
const notFail = x=>!isFail(x);
//maximum 1 per 10 seconds
//you can get throttle period from here:
//https://github.com/amsterdamharu/lib/blob/master/src/index.js
const max1Per10Seconds = lib.throttlePeriod(1,10000)(getHtml);
getNumbers()
.then(
  numbersArray =>
    Promise.all(//process all numbers
      numbersArray
      .map(//map number to url
        number => 
          `https://jsonplaceholder.typicode.com/posts/${number}`
      )
      //map url to promise
      //max1Per10Seconds calls getHtml maximum 1 time per 10 seconds 
      //  (will schedule the calls)
      .map(max1Per10Seconds)
      .map(//map promise to promise that does not reject
        p=>//instead of rejecting promise, resolve with Fail value
          //these Fail values can be filtered out of the result later.
          //(see last then)
          p.catch(err=>new Fail([err,number]))
      )
    )
).then(
  //got the results, not all results may be successes
  postTitlesArray => {
    //is a comment really needed here?
    const successes = postTitlesArray.filter(notFail);
    const failed = postTitlesArray.filter(isFail);
  }
);
HMR
  • 30,349
  • 16
  • 67
  • 136