Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Adding Failed Body for extended error objects #2061

Open
zachlevy opened this issue Jun 4, 2021 · 1 comment
Open

Enhancement: Adding Failed Body for extended error objects #2061

zachlevy opened this issue Jun 4, 2021 · 1 comment

Comments

@zachlevy
Copy link

zachlevy commented Jun 4, 2021

I'm sure a lot of projects, including a couple of mine, use extended Error objects

A simple example would be

const err = new Error('An error message');
err.code = 'SOME_APP_ERROR_CODE';
> JSON.stringify(err);
'{"code":"SOME_APP_ERROR_CODE"}'

https://github.com/goldbergyoni/nodebestpractices/blob/master/sections/errorhandling/useonlythebuiltinerror.md#use-only-the-built-in-error-object

In bull, when saving a failed job processing attempt, it stores the stack trace and the error message, it would be great to store any extended error attributes also.

params.failedReason = err.message;
params.failedBody = JSON.stringify(err);
@alolis
Copy link
Contributor

alolis commented Aug 19, 2021

@zachlevy, this is definitely something useful. In the meanwhile, you can check this out as an alternative approach. Works very nicely for me.

@manast, I have another proposal for this. Instead of introducing a new property, you could also do the following here:

Job.prototype.moveToFailed = async function(err, ignoreLock) {
  err = err || { message: 'Unknown reason' };

  this.failedReason = (err instanceof Error) && _.isFunction(err.toJSON) ? err.toJSON() : err.message;

  // ... code
}

So, if someone wants to use a custom error object, all they have to do is implement toJSON in their extended Error object. Needs some further thinking though; what if someone wants the message even if Error implements toJSON ? or is it just fine?

Using the property failedReason as both JSON and string is not something new with bull; progress does the same so it might be a good idea in order to have a unified behavior instead of introducing another property to the Job object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants