1

I have been reading up on methods to implement a polling function and found a great article on https://davidwalsh.name/javascript-polling. Now using a setTimeout rather than setInterval to poll makes a log of sense, especially with an API that I have no control over and has shown to have varying response times.

So I tried to implement such a solution in my own code in order to challenge my understanding of callbacks, promises and the event loop. I have followed guidance outlined in the post to avoid any anti-patterns Is this a "Deferred Antipattern"? and to ensure promise resolution before a .then() promise resolve before inner promise resolved and this is where I am getting stuck. I have put some code together to simulate the scenario so I can highlight the issues.

My hypothetical scenario is this: I have an API call to a server which responds with a userID. I then use that userID to make a request to another database server which returns a set of data that carries out some machine learning processing that can take several minutes.

Due to the latency, the task is put onto a task queue and once it is complete it updates a NoSql database entry with from isComplete: false to isComplete: true. This means that we then need to poll the database every n seconds until we get a response indicating isComplete: true and then we cease the polling. I understand there are a number of solutions to polling an api but I have yet to see one involving promises, conditional polling, and not following some of the anti-patterns mentioned in the previously linked post. If I have missed anything and this is a repeat I do apologize in advance.

So far the process is outlined by the code below:

let state = false;
const userIdApi = ()  => {
    return new Promise((res, rej) => {
  console.log("userIdApi");
  const userId = "uid123";
      setTimeout(()=> res(userId), 2000)
    })
}

const startProcessingTaskApi = userIdApi().then(result => {
    return new Promise((res, rej) => {
    console.log("startProcessingTaskApi");
        const msg = "Task submitted";
      setTimeout(()=> res(msg), 2000)
    })
})

const pollDatabase = (userId) => {
    return new Promise((res, rej) => {
  console.log("Polling databse with " + userId)
      setTimeout(()=> res(true), 2000)
    })
}


Promise.all([userIdApi(), startProcessingTaskApi])
    .then(([resultsuserIdApi, resultsStartProcessingTaskApi]) => {
      const id = setTimeout(function poll(resultsuserIdApi){
        console.log(resultsuserIdApi)
        return pollDatabase(resultsuserIdApi)
        .then(res=> {
            state = res
            if (state === true){
              clearTimeout(id);
              return;
              }
            setTimeout(poll, 2000, resultsuserIdApi);
            })
            },2000)
        })

I have a question that relates to this code as it is failing to carry out the polling as I need:

I saw in the accepted answer of the post How do I access previous promise results in a .then() chain? that one should "Break the chain" to avoid huge chains of .then() statements. I followed the guidance and it seemed to do the trick (before adding the polling), however, when I console logged out every line it seems that userIdApi is executed twice; once where it is used in the startProcessingTaskApi definition and then in the Promise.all line.

Is this a known occurrence? It makes sense why it happens I am just wondering why this is fine to send two requests to execute the same promise, or if there is a way to perhaps prevent the first request from happening and restrict the function execution to the Promise.all statement?

I am fairly new to Javascript having come from Python so any pointers on where I may be missing some knowledge to be able to get this seemingly simple task working would be greatly appreciated.

Snek
  • 59
  • 1
  • 11

2 Answers2

1

I think you're almost there, it seems you're just struggling with the asynchronous nature of javascript. Using promises is definitely the way to go here and understanding how to chain them together is key to implementing your use case.

I would start by implementing a single method that wraps setTimeout to simplify things down.

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

Then you can re-implement the "API" methods using the delay function.

const userIdApi = () => {
    return delay(2000).then(() => "uid123");
};

// Make userId an argument to this method (like pollDatabase) so we don't need to get it twice.
const startProcessingTaskApi = (userId) => {
    return delay(2000).then(() => "Task submitted");
};

const pollDatabase = (userId) => {
    return delay(2000).then(() => true);
};

You can continue polling the database by simply chaining another promise in the chain when your condition is not met.

function pollUntilComplete(userId) {
    return pollDatabase(userId).then((result) => {
        if (!result) {
            // Result is not ready yet, chain another database polling promise.
            return pollUntilComplete(userId);
        }
    });
}

Then you can put everything together to implement your use case.

userIdApi().then((userId) => {
    // Add task processing to the promise chain.
    return startProcessingTaskApi(userId).then(() => {
        // Add database polling to the promise chain.
        return pollUntilComplete(userId);
    });
}).then(() => {
    // Everything is done at this point.
    console.log('done');
}).catch((err) => {
    // An error occurred at some point in the promise chain.
    console.error(err);
});
Jake Holzinger
  • 5,783
  • 2
  • 19
  • 33
  • Hi Jake, that's great I never thought of pulling the setTimeout into its own promise like that. I will give this a try. I noticed your second code block was carried out like I had initially written mine, but I have seen some posts that state this is not best practise as it begins to look like the "pyramid of doom" once again. Hence I adopted the chaining in the promise.all argument array as my linked post indicates one should. Usually it wouldn't be an issue if two api calls are executed but if the first is to download a 10MB file then it does become an issue. So I will give your method a try. – Snek Oct 28 '19 at 16:39
  • 1
    You can flatten the promise chain, but three levels of nesting is not that extreme. In this case it's not ideal because we need access to the `userId` so it's useful to chain then methods together where they have access to the `userId` without passing it around in some way. – Jake Holzinger Oct 28 '19 at 16:41
  • 1
    @Snek [Nesting closures](https://stackoverflow.com/a/28250687/1048572) is a totally valid and very simple way to access the `userId`, and there's nothing wrong with it as long as it doesn't impede readability. – Bergi Oct 28 '19 at 20:02
  • @Bergi thanks for the confirmation. I think I have spent too many hours on medium today with conflicting articles. – Snek Oct 28 '19 at 20:07
0

This becomes a lot easier if you're able to actually use the async and await keywords.

Using the same delay function as in Jake's answer:

async function doItAll(userID) {
    await startTaskProcessingApi(userID);
    while (true) {
        if (await pollDatabase(userID)) break;
    }
}
Alnitak
  • 334,560
  • 70
  • 407
  • 495
  • Thanks @Alnitak. I wanted to attempt using the .then as a first attempt as some of the browsers I am testing this on are unfortunately very old with no view of upgrading right now. I will have a look at it though. – Snek Oct 28 '19 at 17:26