joeattardi / promise-poller Goto Github PK

When I pass 60000 as the interval, it calls my function every 10 seconds (estimate) instead of every 60 seconds.

  promisePoller({
        taskFn: pollTask,
        interval: 60000, // milliseconds
        retries: 3,
        shouldContinue: () => shouldContinue,
      });

I've try a stupid test by setting the masterTimeout value and it seems not taken into account, there is a kind of timeout that occur but not at the time specified in masterTimeout value
I've look at the code, when masterTimeout then it create a setTiemout with the field options.globalTimeout:

if (options.**masterTimeout**) { setTimeout(() => reject('master timeout'), options.**globalTimeout**);

Retrying after a fixed interval may not be ideal in some applications. Extending this package to also support using exponential backoff would be helpful.

Some exceptions may be due to a condition that won't change over the course of polling - for example, a syntax error.

The idea would be, if we fail on one of these types of exceptions, to stop polling early since we won't have any different results.

You may want to cancel the polling early. For example, if the poll fails because of an invalid password, that's not likely to change, so it would be a waste of time to continue to poll. To cancel polling early, return false from the task function instead of a promise.

The use cases I have had so far for polling all involve async tasks, which cannot cancel the polling operation if they encounter an error that should be fatal, because they can't make the task function return false instead of a promise.

One possibility would be to add a unique sentinel value/error type for cancelation.

A possible workaround is to maintain a flag in the enclosing scope that allows cancellation on the next invocation, but this isn't as nice as canceling immediately especially if the interval is high.

The timeout-functionality seems to be broken:

require('promise-poller')({
  taskFn: () => {
    console.log('polling...');
    return Promise.reject();
  },
  interval: 10,
  timeout: 1000,
})
.then(() => console.log('success'))
.catch(() => console.log('failed'));

This will log "polling..." 5 times (which seems to be the default) and then fails. When adding retries: Infinity, it keeps polling infinitely.

Any idea what's up?

Currently, the timeout option sets the timeout for each individual poll. It might also be handy to have a timeout for the entire poll operation.

So, for example:

timeout: 10000 means "every time you poll, reject the promise for that poll iteration if it's not resolved after 10 seconds.

totalTimeout: 10000 means "while polling one or more times, reject the master promise if there isn't a successful poll after 10 seconds."

I'm wanting to make a function that polls a GitHub commit status until my CI build has finished. To do this with promise-poller, I have the following code:

taskFn: () => this.repos.owner(owner).repo(repo).commits.ref(ref).status().then(({state}) => {
  if (state === 'failure') return false
  if (state === 'pending') throw new Error('commit is still pending')
}),

This is kind of backwards, at least for my use case, because I can't return a "build failed" error if state === 'failure'. It would be better if I could return false when state === 'pending' and throw an error when state === 'failure'.

But really, considering that one might want to poll and eventually resolve to a boolean value, I think it would be best to support isDone and shouldContinue functions that default to the current behavior.

There may be use cases where a poll should fail after a given timeout if the promise isn't settled by then.

Apologies if I missed it in the documentation, but is it possible to pass arguments to the taskFn? Thanks for your work on this project!

On master timeout and on return false-style cancellation, a string rejection is used. That should be avoided.

Also, this may be a breaking change, but wrapping the list of rejections in an error would also be good (see Bluebird's AggregateError).

It appears that the main bluebird feature you're using is timeout. In general, it's nice to not force a bluebird dependency.

masterTimeout is implemented using a setInterval that is not cleared when the master promise resolves. If the timeout is long and the program is supposed to exit after polling completes, the interval will prevent the Node process from exiting.

If shouldContinue returns true, but the masterTimeout has fired, polling continues.

Let me know if you're unable to reproduce, and I'll create a test case.

When setting the timeout property any error caused by the poll task will be ignored.
The poller will wait for the timeout to run out and swallow the actual error message.

Is being addressed in #25

@joeattardi could you please approve the PR to be merged? Thanks 🙏