hapijs / pinpoint

Return the filename and line number of the calling function
Other
6 stars 7 forks source link

Firefox Has no "Error.captureStackTrace" #8

Open lloy0076 opened 2 years ago

lloy0076 commented 2 years ago

Context

What are you trying to achieve or the steps to reproduce?

Create a basic app that does: console.log('What is the location?', location()); where location() is the import { location } from '@sideway/pinpoint'; then see:

image

import { location } from '@sideway/pinpoint';
console.log('Location: ', { l: location() });

What was the result you got?

See the screenshot above.

Uncaught TypeError: Error.captureStackTrace is not a function
    node_modules 1.chunk.js:6264
    jsx Home.jsx:10
    Webpack 10
index.js:14
    location index.js:14
    jsx Home.jsx:10
    Webpack 10

What result did you expect?

It should show the location.

NB. It works on Chrome and Edge. NB. See https://github.com/gr0uch/error-class/blob/b1e9193254941ec2a549a40185cc4a05db98d241/index.js#L34-L40 for a possible work around or solution.

lloy0076 commented 2 years ago

It seems to display the same behaviour using Firefox on Raspian as well (91.11.0esr).

pi@raspberrypi:~ $ dpkg -l | grep firefox
ii  firefox-esr                           91.11.0esr-1~deb10u1+rpi1               armhf        Mozilla Firefox web browser - Extended Support Release (ESR
kanongil commented 2 years ago

You probably should not expect this module to work outside the V8 engine. Stacktraces are engine-specific and your workaround would require parsing a formatted string without any formal specification.

Marsup commented 2 years ago

Since it's used by joi, which works in the browser, it's probably an issue that should be fixed anyway. The suggested workaround seems fine to me.

kanongil commented 2 years ago

The suggested workaround is useless without a new string parsing engine.

lloy0076 commented 2 years ago

Nb. I didn't suggest the work around; it's from another project that seems to have encountered the same problem - I'm not immediately familiar with how to get a stacktrace that is somewhat standard across JS engines unfortunately.

Marsup commented 2 years ago

@kanongil Care to explain why? IMHO the workaround should be best effort, captureStackTrace is nice to have but not a mandatory feature.

kanongil commented 2 years ago

The current code uses V8 internal stack objects, exposed through the Error.prepareStackTrace hook.

That means that the stack property here is not a string, but an array of V8 stack objects: https://github.com/sideway/pinpoint/blob/d9b4e37a474bda32c1297a437726310cc9b0356f/lib/index.js#L13

As such, the "workaround" will throw once line.getFileName() is called here, since line will be a string: https://github.com/sideway/pinpoint/blob/d9b4e37a474bda32c1297a437726310cc9b0356f/lib/index.js#L18

FYI, this details how Error.prepareStackTrace works: https://v8.dev/docs/stack-trace-api#customizing-stack-traces

You can make it return a fallback value on non-V8 engines, if you think that is a better solution than throwing an Error.

Marsup commented 2 years ago

Is it a solved problem anywhere in the ecosystem? We can't possibly be the only trying to do that.

lloy0076 commented 2 years ago

Another possible work around is to not die horridly if captureStackTrace is unusable.

try {
    const capture = {};
    Error.captureStackTrace(capture, this);
    const line = capture.stack[depth + 1];
}

// catch it and `console.error` or something if we die due to Error not having `captureStackTrace`.

It's unlikely that anyone should be depending on location except for debugging purposes [but that's an assumption] so if it just error logs vs crashes with a runtime error, wouldn't that be a slightly better outcome?