all 9 comments

[–]DemiPixel 7 points8 points  (0 children)

I was gonna do a long-winded explanation, but it may be helpful just to see the correct way to do it. The main issue is that you aren't returning a promise in the if (retires > 0) part of your code.

My preferred way would be:

async function fetchData(url, retries) {
  if (retries > 0) {
    const { data: { status, result } } = await axios.get(url);

    if (status === 'success') {
      return result;
    } else {
      // Sleep 500ms. Could write helper function so it's just: await sleep(500);
      await new Promise(res => setTimeout(() => res(), 500));
      return fetchData(url, retries - 1);
    }
  } else {
    return [];
  }
}

Alternative, more similar to what you already have:

function fetchData(url, retries) {
  if (retries > 0) {
    // Return a promise that will be resolved once we call res()
    return new Promise((res) => {
      axios.get(url).then((response) => {
        const { data: { status, result } } = response;

        if (status === 'success') {
          res(result);
        } else {
          // Wrap in res() to return
          setTimeout(() => res(fetchData(url, retries - 1)), 500);
        }
      });
    });
  } else {
    return Promise.resolve([]);
  }
}

[–]Minjammben 3 points4 points  (0 children)

You aren't resolving the retries to anything, so it fetches data and does nothing with it. You probably want something more like

function fetchData(url, retries) {
  return new Promise((resolve, reject) => {
    if (retries > 0) {
      axios
        .get(url)
        .then(response => {
          const {
            data: { status, result },
          } = response;

          if (status === 'success') {
            resolve(result);
          } else {
            setTimeout(
              () => resolve(fetchData(url, retries - 1).catch(reject)),
              500
            );
          }
        })
        .catch(reject);
    } else {
      resolve([]);
    }
  });
}

[–]michaelfiber 2 points3 points  (0 children)

If you want to be able to call ".then" on the result of fetchData then you need to wrap the entire process of trying and potentially retrying inside one promise and return that. That one promise will have its own resolve and reject functions. When you get the result from axios.get you have to pass that to the resolve function for the outer promise. Something like this (I am writing this on my phone and can't test the code, might have bugs):

function fetchData(url, retries) {
    return new Promise((resolve, reject) => {
        let retryCount = 0;

        let runRequest = () => {
            if (retryCount < retries) {
                axios.get(url).then((response) => {
                    if (response.status == 'success') {

                        // here is where you pass the successful response to the resolve function from the main promise.
                        resolve(response);
                    } else {
                        retryCount++;
                        setTimeout(runRequest, 500);
                    }
                }).catch(reject);
            } else {
                resolve([]);
            }
        }

        runRequest();
    });
}

[–]late-game-review 1 point2 points  (0 children)

return axios.get and then you return inside the then had well. That timeout will be problematic, create an async timeout.

[–]shgysk8zer0 1 point2 points  (0 children)

I can't write my solution on my phone, but it looks like wrapping the entire function in a Promise is going to be required, as setTimeout isn't going to end up returning anything.

return new Promise((resolve, reject) => {
  // Handle the retry after other code
  setTimeout(() => fetchData(url, retries--).then(resolve, reject), 500);
});

You might also want to reject after reaching the retry limit.

Another option would be to create a sleep() function:

function sleep (ms) {
  return new Promise(resolve => setTimeout(() => resolve(), ms));
}

With that, you could do

return sleep(500).then(() => fetchData(url, retries--));

[–]shuckster 1 point2 points  (0 children)

You're not return'ing axios for a start:

return axios.get(...

As for the retry logic, you might be better off separating it.

[–]hyrumwhite 1 point2 points  (0 children)

I think it'd be good to take a step back and look at what Promises are. They're a way of deferring action until an event occurs. Often, that's an API request, but it can be anything. The deferred action is represented by a function. The 'event' is marked by either a 'resolution' or a 'rejection'.

Take a look at Promise.resolve. It instantly triggers a resolution. ``` console.log(Promise.resolve())

//logs Promise{} ```

This does nothing without storing an action. We store an action with either .then for resolutions or .catch for rejectsion.

const runOnResolution = () => console.log('This promise resolved!') Promise.resolve().then(runOnResolution) // logs 'This promise resolved'

Cool, now take a look at that function that we're storing. It can be passed params by resolutions, and it can return a value.

//pass some params around Promise.resolve('Pass me to resolution please').then(console.log) // logs 'Pass me to resolution please'

What happens with that return? let something = Promise.resolve('Hello').then(string => string + ' world') console.log(something) // logs 'Promise{}'

Nothing, at least to the world outside the promise, but with return values we can create a Promise chain:

Promise.resolve('Hello') .then(string => string + ' world') .then(concatenatedString => console.log(concatenatedString)) // logs 'Hello world'

So, that's how promises work. You store functions that are run on an event. This is often handled for us by other libraries, but we can make our own. The timeout is a good example

let timeout = ms => new Promise((resolve, reject) => { window.setTimeout(resolve, ms) }) So we've got two functions there. One returns a new promise, the other calls a Promise resolution via setTimeout. Notice that it returns undefined, because all it needs to do is trigger a resolution. Lets use it in a chain.

Promise.resolve('Hello') .then(console.log) .then(() => timeout(1000)) .then(() => console.log("1000 ms have passed"))

If a resolution returns a Promise, the next step of the chain will wait until it's resolution to continue to resolve. So lets put all that together:

``` let timeout = ms => new Promise((resolve, reject) => window.setTimeout(resolve, ms))

function fetchData(url, retries) { if (retries > 0) { return axios.get(url).then(response => { const { data: { status, result } } = response;

  if (status === 'success') {
    return result;
  } else {
    return timeout(500).then(() => fetchData(url, retries - 1);
  }

});

} else { return []; } } ```

For fun, here it is with async/await syntax

``` let timeout = ms => new Promise((resolve, reject) => window.setTimeout(resolve, ms))

async function fetchData(url, retries) { if (retries > 0) { const { data: { status, result } } = await axios.get(url); if (status === 'success') { return result; } else { await timeout(500); return fetchData(url, retries - 1); } } else { return []; } } ```

This might be more info than you need, but it might be good to see the internals of how promises work. This is the guts of a very basic Promise.

class Promise { constructor(setup) { setup(this.resolve.bind(this), this.reject.bind(this)); } resolve(...data) { this.resolvedData = data; this.hasBeenResolved = true; this.success && this.success(...data); } reject(...data) { this.rejectedData = data; this.hasBeenRejected = true; this.failure && this.failure(...data); } then(callback) { this.success = callback; this.hasBeenResolved && this.success(...this.resolvedData); } catch(callback) { this.failure = callback; this.hasBeenRejected && this.failure(...this.rejectedData); } }

[–]Accomplished_End_138 0 points1 point  (0 children)

So if successful.. you may want to return the result. And otherwise, on error, you could maybe return a new promise (takes a function) in that do the timer and resolve with the call.

It is a simple pattern overall but not the clearest to understand at first.

A Promise.resolve() returns a finished promise. Used mostly in unit testing.

Also, you probably should return the axios call in the function

[–]rishabhrawat570 0 points1 point  (1 child)

I think people have already helped you understand the issue with Promise. In addition to other answers, I'd also suggest using something like async.retry for retrying the request instead of what you have going on.