Queue retries indefinitely upon transaction error

[mhkrebs]

Version info

Firebase:
[email protected]
[email protected]
Firebase Queue:
[email protected]
Node.js:
v6.9.4
Other (e.g. operating system) (if applicable):

Test case

I believe there's a bug in QueueWorker.prototype._tryToProcess regarding max retry attempts.

I ran into a "permission denied" error when someone posted to my Queue:

FIREBASE WARNING: transaction at /queues/voting/tasks/-Ke7IYBJL8obA_o9tXj6 failed: permission_denied
debug: QueueWorker 0:90f4e189-7f8b-4dec-90d5-bd4f11c8b9c8 errored while attempting to claim a new task, retrying Error: permission_denied
    at Error (native)                                            

The bug is that it got stuck in a retry loop, rather than bailing after 10 retries, and it ate up large amounts of download quota before I noticed it.

I believe the bug is that the following code from https://github.com/firebase/firebase-queue/blob/master/src/lib/queue_worker.js#L484 will retry the whole _tryToProcess function, which resets the retries variable.

    if (++retries < MAX_TRANSACTION_ATTEMPTS) {
      logger.debug(self._getLogEntry('errored while attempting to ' +
        'claim a new task, retrying'), error);
      return setImmediate(self._tryToProcess.bind(self), deferred);
    }

In most of the other functions that do something similar, they retry by calling a local function that still has retries in scope.

Steps to reproduce

I don't have a test case to reproduce, but I think the bug is apparent without it. I assume I have a bad security rule somewhere that results in the "permission denied" error.

Expected behavior

Actual behavior

[EECOLOR]

Another related problem is with this: https://github.com/firebase/firebase-queue/blob/master/src/lib/queue_worker.js#L510

self.taskNumber += 1;
...
self.currentTaskListener = self.currentTaskRef
  .child('_owner').on('value', function(ownerSnapshot) {
  var id = self.processId + ':' + self.taskNumber;
  
  if (ownerSnapshot.val() !== id ...
    ...
    self.currentTaskRef = null

For any asynchronous task (that takes longer than fetching the owner) the current taskref is set to null. This causes _reject (at https://github.com/firebase/firebase-queue/blob/master/src/lib/queue_worker.js#L264) and _resolve ( at https://github.com/firebase/firebase-queue/blob/master/src/lib/queue_worker.js#L169) to fail.

[katowulf]

@EECOLOR separate issue please.

[EECOLOR]

@mhkrebs This issue has been solved in out fork, this is a heavily trimmed version so it might not be what you need.