-
Notifications
You must be signed in to change notification settings - Fork 87
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
Why is this library overwriting Error type? #32
Comments
Hi @purpledrgn I'm sorry this is causing issues for you. It's not trying to actually change the global
etc. If there is a better way to accomplish getting the stack, I'm all for it. There is a pretty extensive test suite around this module which should help highlight why this is there; it was added to fix exceptions that were being encountered by users. If you're also encountering exceptions from this, I'd love to get it fixed! If you can provide a reproduction case I can look into, I can surely help to find a workable solution for you if you like, otherwise you can always make a pull request as well. |
The safest way to fix it is to actually check if the type of The problem with The following is probably the only way to actually ensure your code works: const NativeError = Error;
class Overwrite {
constructor() {
this.stack = "\n\n\n"
}
}
Object.defineProperty(Overwrite, 'prepareStackTrace', {
set: handle => {
NativeError.prepareStackTrace = handle;
},
get: () => {
return NativeError.prepareStackTrace;
}
});
Overwrite.captureStackTrace = NativeError.captureStackTrace;
Object.defineProperty(Overwrite, 'stackTraceLimit', {
set: value => {
NativeError.stackTraceLimit = value;
},
get: () => {
return NativeError.stackTraceLimit;
}
});
////////////////////////////////////////////////////////////////////////////////////
Error = Overwrite;
////////////////////////////////////////////////////////////////////////////////////
function prepareObjectStackTrace(obj, stack) {
return stack;
}
function getStack () {
var limit = Error.stackTraceLimit
var obj = {}
var prep = Error.prepareStackTrace
Error.prepareStackTrace = prepareObjectStackTrace
Error.stackTraceLimit = Math.max(10, limit)
// capture the stack
Error.captureStackTrace(obj)
// slice this function off the top
var stack = obj.stack.slice(1)
Error.prepareStackTrace = prep
Error.stackTraceLimit = limit
return stack
}
function callSiteLocation (callSite) {
var file = callSite.getFileName() || '<anonymous>'
var line = callSite.getLineNumber()
var colm = callSite.getColumnNumber()
if (callSite.isEval()) {
file = callSite.getEvalOrigin() + ', ' + file
}
var site = [file, line, colm]
site.callSite = callSite
site.name = callSite.getFunctionName()
return site
}
function a() {
var stack = getStack()
var site = callSiteLocation(stack[1])
return site;
}
function b() {
return a();
}
console.log(b()); Any implementation that doesn't do the setter/getter hack there will just fail in mystifying ways. How many times have you written You linked to nodejs docs, but only 2 of the 3 are listed there. I don't see any mention of |
Yea, I can definitely make changes! If you can provide a reproduction case I can look into, I can surely help to find a workable solution for you if you like, otherwise you can always make a pull request as well along with tests added to the test suite. |
Simply comment out in the example I gave you the following: Object.defineProperty(Overwrite, 'prepareStackTrace', {
set: handle => {
NativeError.prepareStackTrace = handle;
},
get: () => {
return NativeError.prepareStackTrace;
}
}); You'll get a perfect reproduction case. Besides Like I said above I think it's fair to expect overwrites to Error to provide |
Gotcha. I'm not going to support such an environment who is overwriting the global Error if that is what is happening. |
Is there no way to avoid this function?
More specifically the native
Error
type overwrites.I'm not against overwrites to native types but this sort of unrequested overwrite is just annoying. Overwrites are either at the user level or by request if libraries offer them (usually the case for unit test libraries).
What happens with cases such as this particular case is that it (no surprise) bumps heads with other requested overwrites resulting in hard to track errors such as
callSite.getFileName is not a function
. Since this included by things such as body-parser, you are forcing this overwrite on everyone who uses it.In addition the apis used there, seem to be V8 specific, throwing even more spanners around.
The text was updated successfully, but these errors were encountered: