feat(crashtracking): add unhandled exception libdatadog binding#73
feat(crashtracking): add unhandled exception libdatadog binding#73
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
1503cc4 to
7226d08
Compare
56126dd to
d01d35c
Compare
d01d35c to
fbb872e
Compare
watson
left a comment
There was a problem hiding this comment.
Is the name of the function reportUnhandledException set in stone? I ask because I think all we care about is that it's an error right? By calling it something with an "unhandled exception" we bind it very tightly to the concept of unhandled exceptions in Node.js, where in fact both the uncaughtException handler and the uncaughtExceptionMonitor handler is fired for both uncaught exceptions and unhandled promise rejections (they both provide a 2nd argument to their callback functions called origin being either the string uncaughtException or unhandledRejection).
When the origin is uncaughtException, the 1st argument is guarenteed to be an instance of Error, but for unhandledRejection it might be a non-Error. So we need to ensure that either crashtracker.reportUnhandledException is able to ignore those non-Error values, or that when we implement this in the Node.js client lib, that we don't call crashtracker. reportUnhandledException with a non-Error value.
On the library side, it is called However, I agree that this API should probably be steered towards supporting all crashes from different types of "errors". I can make an update on the library side, so that the name of the function is not limiting. |
Ah, yeah. I was thinking about this here. We also do want to support |
Overall package sizeSelf size: 37.28 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------|🤖 This report was automatically generated by heaviest-objects-in-the-universe |
I just looked into this, and actually if I So it's not an issue for us in that case. However, there's still a slight possibility that Output: In that case I think we should do generate a fallback error message, eg: |
7b7956b to
c17bd04
Compare
1fdf204 to
fdabc55
Compare
fdabc55 to
a38c404
Compare
a38c404 to
4f77fef
Compare
watson
left a comment
There was a problem hiding this comment.
Nice job! 💯
I've reviewed all files except crates/crashtracker/src/unhandled_exception.rs, as I'm not familiar with that part of the code base.

This PR adds Rust binding for libdatadog's
report_unhandled_exceptionAPI.Honestly, I don't know much about JS conventions or style or internals; I am very open to feedback.
The design choice here is that the caller should only ever have to pass in some payload given to them by
process.on("uncaughtExceptionMonitor"). The binding side will take care of the work of checking if it is anErrortype and parsing into libdatadogStackFrame's and call the Rust library API.I also added some unit tests for the stacktrace parsing logic, and added new "integration test" types that run the different scenarios:
Note: node wraps unhandled rejection that throws a non-error type into an Error itself
I also ran
cargo fmton the native extension code, which formatted some of the older code.This can be used on the
dd-trace-jsside. All the caller must do is ensure that the crashtracker has been started for the program, and do as suchHere is a crash report from the "integration test"