Skip to content
This repository was archived by the owner on Mar 17, 2025. It is now read-only.

Conversation

cbraynor
Copy link
Contributor

Changes to the way error objects are handled by the reject() callback function and adding an option to suppress storing stack traces in the task's database location for security reasons

/cc @dreadjr

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@dreadjr
Copy link
Contributor

dreadjr commented Jun 15, 2015

this looks great.

@cbraynor
Copy link
Contributor Author

@googlebot confirm

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this? Passing a plain string into reject() should work as well correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a fan of using Error because it allows you to attach other
properties, etc. You could put the number of errors, etc. Also could create
custom errors for these scenarios. But yes, a string works too.

On Tue, Jun 16, 2015 at 10:20 AM, Matthew Tse notifications@github.com
wrote:

In src/lib/queue_worker.js
#24 (comment):

@@ -113,7 +119,7 @@ QueueWorker.prototype._resetTask = function(taskRef, deferred) {
} else {
var errorMsg = 'reset task errored too many times, no longer retrying';
logger.debug(self._getLogEntry(errorMsg), error);

  •    deferred.reject(errorMsg);
    
  •    deferred.reject(new Error(errorMsg));
    

What's the reason for this? Passing a plain string into reject() should
work as well correct?


Reply to this email directly or view it on GitHub
https://github.com/firebase/firebase-queue/pull/24/files#r32546129.

@m-tse
Copy link
Contributor

m-tse commented Jun 16, 2015

LGTM

m-tse added a commit that referenced this pull request Jun 16, 2015
v1.2.0 release candidate
@m-tse m-tse merged commit 993d57f into master Jun 16, 2015
@m-tse m-tse deleted the 1-2-0 branch June 16, 2015 22:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants