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

bunyan-opbeat removes callsite of original error #1

Open
mderazon opened this issue Apr 28, 2016 · 10 comments
Open

bunyan-opbeat removes callsite of original error #1

mderazon opened this issue Apr 28, 2016 · 10 comments
Labels

Comments

@mderazon
Copy link
Owner

When sending an error to Opbeat, node-opbeat tries to be smart about it and figure out the callsite where the error occurred from.

Problem with Bunayn is that if you use the standard serializer it strips the error object and turns it into a plain object, losing some information along the way..

This is why I try to deserialize the object back into an error form - https://github.com/mderazon/bunyan-opbeat/blob/master/lib/index.js#L56
The problem is that I have to create a new error in the process and manually move some of the properties to the newly created error. Since the error was created in this module (bunyan-opbeat), Opbeat shows the "culprit" as this module.

I would like to be able to send Opbeat the original culprit and not this one. It will probably involve some stack manipulation, not sure how.

@watson think you can take a look and see if you have an idea?

@mderazon mderazon added the bug label Apr 28, 2016
@watson
Copy link

watson commented Apr 29, 2016

Opbeat is using the V8 Stack Trace api to get the callsites. I'm not sure your way of recreating the error works with this API unfortunately. I think you can get around that by using Error.captureStackTrace() to create the error object instead. If you make use of the 2nd argument you can have it ignore the callsites related to the bunyan-opbeat module. That should make it not think that bunyan-opbeat was the culprit anymore. If this doesn't work, let me know and I can have a 2nd look at it.

@mderazon
Copy link
Owner Author

mderazon commented May 1, 2016

@watson thanks.
Your advice was helpful and I was able to ignore the bunyan-opbeat module in stack trace like you mentioned using Error.captureStackTrace() but that left me with two other bunyan specific functions in stack trace which I have no control over (coming from Bunyan):

Error: my error message
    at Logger._emit (/home/proj/node_modules/bunyan/lib/bunyan.js:938:22)
    at Logger.error (/home/proj/node_modules/bunyan/lib/bunyan.js:1037:24)
    at /home/proj/index.js:269:15  <---- this is the one i'm looking for
    ...

Assuming I can "pop" these two functions out of the stack as well (maybe overriding prepareStackTrace(), not sure how yet), do you think it'll be a good idea ? I have no control of the Bunyan api, so this may break in some way (although I doubt it given the amount of updates to Bunyan)

Users can always avoid all this by not using Bunyan's stdSerializers.err. This way the error will be passed unchanged to bunyan-opbeat and it will all just work. But using the serializer has its merit, specifically with the Bunyan stdout formatter - it formats it in a nice way.

@watson
Copy link

watson commented May 1, 2016

Opbeat adds a custom property to the error object called __error_callsites. It's an array of all the raw callsites. It's generated the first time .stack is called on the error object. You might be able to shift that array?

@watson
Copy link

watson commented May 1, 2016

This is obviously a really bad idea since it's a private property that we might change at any time 🙈

@mderazon
Copy link
Owner Author

mderazon commented May 1, 2016

Basically I need to recreate an error given a stack trace (string) so that the newly created error will think and behave like it came from the original callsite.

I tried all kind of different hacks including parsing the given stack via various parsers and hot swaping it with the newly created error by overriding prepareStackTrace().

Needless to say it wasn't pretty.

I'm kind of stuck so I think i'll give up for now. I will pass the error object raw to Bunyan instead of passing it through the serializer.

If you (or anyone reading this) have a solution for this, i'm all ears 👂

@watson
Copy link

watson commented May 1, 2016

@mderazon what would be the best solution for you if you could change something in the current api of the Opbeat agent?

@mderazon
Copy link
Owner Author

mderazon commented May 2, 2016

@watson this is not node-opbeat agent problem here and I don't think anything should be changed regarding this issue. It works as expected - you pass an err and it logs it correctly. This is more of a Bunyan limitation than Opbeat's really.

The best solution IMO would have been if Bunyan allowed defining serializers on the stream level instead of the global level - i.e. for Opbeat stream I want err to pass as is and for stdout I would like to transform it a little bit. I will ask around in Bunyan repo if it's possible.

Anyway, thank you for your help!

@watson
Copy link

watson commented May 2, 2016

@mderazon I'm thinking you might be able to manually manipulate the array of stack trace elements before it's sent from the Opbeat agent to the API by using the filter option: https://opbeat.com/docs/articles/opbeat-for-nodejs-api/#filter

Edit: It would of cause be better if you didn't need to mock the error object at all - so allowing for this in bunyan is of cause preferable.

@mderazon
Copy link
Owner Author

mderazon commented May 2, 2016

@watson cool, didn't know this option existed. Think I can add the filter after opbeat.start() has been called ?

@watson
Copy link

watson commented May 2, 2016

@mderazon you can access it on opbeat.filter if need to set it or overwrite it later

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

No branches or pull requests

2 participants