feat: core ExtraErrorData integration #1778
Conversation
Generated by 🚫 dangerJS |
| // We are trying to enhance already existing event, so no harm done if it won't succeed | ||
| try { | ||
| const name = error.name || error.constructor.name; | ||
| const errorKeys = Object.keys(error).filter(key => !(key in ['name', 'message', 'stack'])); |
There was a problem hiding this comment.
I know this is copy/paste but the in operator usually doesn't work with arrays?
There was a problem hiding this comment.
In fact, I believe this isn't really needed because Object.keys doesn't return name, message, and stack.
At the same time Object.keys doesn't capture properties such as lineNumber in firefox's SyntaxError.
Object.getOwnPropertyDescriptors might be better suited for fetching properties like that.
There was a problem hiding this comment.
And you are totally right about in usage here, however, we should keep the filter in place, just fixed one. The issue here is that different browsers return different keys (how lovely, right? :p)
Chrome:
Object.keys: ["something"]
Object.getOwnPropertyDescriptors: {stack: {…}, message: {…}, something: {…}}
Firefox:
Object.keys: ["something"]
Object.getOwnPropertyDescriptors: { fileName: {…}, lineNumber: {…}, columnNumber: {…}, message: {…}, something: {…} }
Safari:
Object.keys: ["something", "line", "column"]
Object.getOwnPropertyDescriptors: {message: Object, something: Object, line: Object, column: Object, stack: Object}
var foo = new SyntaxError('asd');
foo.something = 42;
var nativeKeys = ['name', 'message', 'stack', 'line', 'column', 'fileName', 'lineNumber', 'columnNumber'];
Object.keys(foo).filter(key => nativeKeys.indexOf(key) === -1);This seems to be the most unified way now.
| * Just an Error object with arbitrary attributes attached to it. | ||
| */ | ||
| interface ExtendedError extends Error { | ||
| [key: string]: any; |
There was a problem hiding this comment.
Not sure what your TS version is, but you might want to use unknown.
There was a problem hiding this comment.
Good point, we never actually used unkown yet because we started with typescript < 3.
But it makes sense here.
| } | ||
|
|
||
| return null; | ||
| } catch (_oO) { |
There was a problem hiding this comment.
Shouldn't that log debug data, just in case?
| ...errorData, | ||
| }, | ||
| }; | ||
| } else { |
|
Updated. Thanks for the review @SimonSchick! |
Fixes #1777 in a global manner, instead of piling more code in browser package.
TODO: Update https://docs.sentry.io/platforms/javascript/default-integrations/ after it gets merged