search

moll / js-standard-error

Tiny JavaScript library that simplifies subclassing and inheriting from Error while keeping the correct name and stack. Also supports constructing from an object of properties. Saves you from boilerplate.
Other
42 stars 2 forks source link

Add nested error support #1

Open ruimarinho opened 9 years ago

ruimarinho commented 9 years ago

Sometimes the original context of an error is lost due to throwing a new error without taking into consideration the previous error. This PR appends the previous error stack to help with that:

throw new StandardError({message: "File Not Found"}, err)
ruimarinho commented 9 years ago

Any chance of looking at this PR @moll? Thanks!

nunofgs commented 9 years ago

I'm also in need of nested errors and this PR fits the bill.

@moll, could you take a look at this?

moll commented 9 years ago

Hey, sorry, fellows, for the long wait. I've been superbly occupied lately.

Anywho, thanks @ruimarinho, for sharing your code.

I can see how wrapping existing errors comes in handy. I'm not 100% sure merging stack traces should be in StandardError.js's core though. What do you think? An alternative is that you, @ruimarinho, put that stack trace merging code in a separate module depending on StandardError.js and we'll put up a link to it. That would leave the core minimal, yet build upon it.

Looking at StandardError's implementation, you should be able to set the stack property before calling StandardError and it won't override it. You could then call Error.captureStackTrace yourself on a different object, too, to not cause serialization until necessary. The current code seems to serialize the trace immediately at https://github.com/moll/js-standard-error/pull/1/files#diff-168726dbe96b3ce427e7fedce31bb0bcR22.

PS. Your editor seems to strip trailing spaces. While usually useful, trailing spaces have special meaning in Markdown. You might want to configure to not do so there. ;-)

ruimarinho commented 9 years ago

Sorry @moll, this PR got lost in a multitude of other pending todos. While I understand your concern regarding keeping StandardError minimal at its core, which I sure agree with, it would make extremely difficult to build upon this unless it accepts a "middle class". For instance, your other package HttpError would not support this feature. What do you think?

Regarding your comment about calling trace, isn't it being automatically serialised on the line before (https://github.com/moll/js-standard-error/pull/1/files#diff-168726dbe96b3ce427e7fedce31bb0bcR20)?

Indeed, sorry about the trailing whitespaces - I'll remove them before getting the PR merged, if that happens to be the result of this discussion :)

nunofgs commented 8 years ago

@moll: Needed this again today. Any chance you could take a look at this PR?