10

I'm writing some code to do polling for a resource every N ms which should timeout after M seconds. I want the whole thing to be promise based using Bluebird as much as possible. The solution I've come up with so far uses node's interval, cancellable bluebird promises and bluebird's timeout function.

I'm wondering if there's a better way to do timing out intervals with bluebird and promises in general? Mostly by making sure the interval stops at the point and never continues indefinitely.

var Promise = require('bluebird');

function poll() {
  var interval;

  return new Promise(function(resolve, reject) {
    // This interval never resolves. Actual implementation could resolve. 
    interval = setInterval(function() {
      console.log('Polling...')
    }, 1000).unref();
  })
    .cancellable()
    .catch(function(e) {
      console.log('poll error:', e.name);
      clearInterval(interval);
      // Bubble up error
      throw e;
    });
}

function pollOrTimeout() {
  return poll()
    .then(function() {
      return Promise.resolve('finished');
    })
    .timeout(5000)
    .catch(Promise.TimeoutError, function(e) {
      return Promise.resolve('timed out');
    })
    .catch(function(e) {
      console.log('Got some other error');
      throw e;
    });
}

return pollOrTimeout()
  .then(function(result) {
    console.log('Result:', result);
  });

Output:

Polling...
Polling...
Polling...
Polling...
poll error: TimeoutError
Result: timed out
Chris911
  • 4,131
  • 1
  • 23
  • 33

2 Answers2

5

I would do something like this -

function poll() {
  return Promise.resolve().then(function() {
    console.log('Polling...');
    if (conditionA) {
      return Promise.resolve();
    } else if (conditionB) {
      return Promise.reject("poll error");
    } else {
      return Promise.delay(1000).then(poll);
    }
  })
    .cancellable()
}

Also be aware of Promise constructor anti-pattern

Community
  • 1
  • 1
vinayr
  • 11,026
  • 3
  • 46
  • 42
  • 1
    Nice. How would you complete the interval? Looks like the only way out if throwing an error. I know the Promise constructor is an anti-pattern but in this case it seemed to fit (same with events + Promises but that's another subject). – Chris911 Oct 20 '15 at 16:04
  • 6
    Warning: unfortunately, recursion in javascript like this will eventually saturate the call stack and result in out of memory exceptions. – Rene Wooller Feb 23 '16 at 23:33
1

Rene Wooller makes a really good point:

Warning: unfortunately, recursion in javascript like this will eventually saturate the call stack and result in out of memory exceptions

Even if it doesn't exception, this is wasted space, and the risk of an exception might encourage an overlong polling delay.

I think this is important enough to prefer setInterval:

var myPromise = new Promise((resolve, reject) => {
    var id = window.setInterval(() => {
        try {
            if (conditionA) {
                window.clearInterval(id);
                resolve("conditionA");
            } else if (conditionB) {
                throw new Error("conditionB!");
            }
        } catch(e) {
            window.clearInterval(id);
            reject(e);
        }
    }, 1000);
});

There are a few npm packages that address this requirement, of which I like promise-waitfor the best. It's 38 lines long and does the job.

var myPromise = waitFor(() => {
    if(conditionA) return true;
    if(conditionB) throw new Error("conditionB!");
    return false;
});
Martin Randall
  • 308
  • 2
  • 9
  • Can you explain to me how this would be achieved if (conditionA) is a promise? I can never escape the promise chain to return a simple 'true' or 'false' in waitFor(). – NZHammer Dec 11 '17 at 23:25
  • 1
    Feel free to add a separate question for when `conditionA` is a promise. Probably the solution involves `Promise.race`. – Martin Randall Dec 13 '17 at 01:01
  • I've read that if your poll takes longer to run than your setinverval, it will build queue. Then when it finishes, it has to process the queue. And that it is better to use setTimeout and recursion to kick off another setTimeout. – TamusJRoyce Jan 18 '18 at 16:17