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

Return error objects not strings (bug 1034326)#88

Merged
kumar303 merged 1 commit into
mozilla:masterfrom
kumar303:errors
Mar 11, 2015
Merged

Return error objects not strings (bug 1034326)#88
kumar303 merged 1 commit into
mozilla:masterfrom
kumar303:errors

Conversation

@kumar303

Copy link
Copy Markdown
Contributor

@muffinresearch r?

Kind of a beast but I think this will make error handling a lot nicer.

@kumar303 kumar303 force-pushed the errors branch 2 times, most recently from 8f8dca8 to 60cfafb Compare March 11, 2015 00:38
Comment thread lib/fxpay/jwt.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could this be in the constructor or otherwise centralised?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also if the constructor is updated to allow ditching the use of new, assigning the error object might be avoided completely.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the only reason the object is assigned (in most cases) is for the logging. Hmm, I guess I could log in the constructor but that might be weird since constructing an error doesn't always mean you are returning an error.

@muffinresearch

Copy link
Copy Markdown
Contributor

This is really nice.

I think if the constructor is updated you'll be able to remove some repetition and make it easier to use. See the comments for details.

r+wc

@kumar303 kumar303 force-pushed the errors branch 4 times, most recently from 6d1d747 to 73e538d Compare March 11, 2015 20:26
kumar303 added a commit that referenced this pull request Mar 11, 2015
Return error objects not strings (bug 1034326)
@kumar303 kumar303 merged commit e7f9e17 into mozilla:master Mar 11, 2015
@kumar303 kumar303 deleted the errors branch March 11, 2015 20:37
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.

2 participants