compatibility with Node.js' source-map implementation

[bcoe]

Hey @dougwilson, I was wondering if you'd be willing to work with me to figure out a way to make this compatible with Node.js' source-map support, we just landed an API for this here:

nodejs/node@521b222

What I'd love would be to have a well defined way that we can expose transpiled stack traces to express users.

[dougwilson]

Absolutely! Please forgive me, as I have never used any kind of transform thing, though I am aware of their existence. Just reading the docs nothing jumps out to me on what to write here. Is it possible to provide some kind of example app I can play with? Like something where the stack trace would be incorrect unless run with the enable stack traces?

[bcoe]

@dougwilson yeah, I'd me than happy to provide you something; I'll start drafting up a testing repo tonight.

What we need to be mindful of, is a lot of folks might already be using express and source-map-support in conjunction; both of which override prepareStackTrace.

Perhaps we can pick @LinusU's brain too, for some requirements.

[bcoe]

Here's a minimal express + TypeScript app which demonstrates some of the challenges facing a TypeScript application, related to introspecting errors.

Interestingly, source-map-support does seem to work in conjunction with depd -- which I was a bit surprised by, since they both override prepareStackTrace.

What I'd love would be for us to figure out a way to make --enable-stack-trace work reasonably well for a vanilla deployment of express, without stepping on the toes of loggers like winston and bunyan.

[dougwilson]

Thank you, but I'm not sure you are using this module correctly. You cannot create the deprecation function and use it again in the same file. Running your code I'm not seeing a deprecation message from this module, but maybe I'm not calling it the right way. Can you tell me the steps to see the deprecation message from here? Maybe paste in a screenshot here of the message you see and point out where the message is incorrect and what a correct one would look like? Remember, I have never used typescript or any other transpiled language in node.js so don't have the same background as you may have to just "know" what to do here :)

[dougwilson]

I have also never used winston or bunyon myself, so again, do not know what it would mean to step on their toes... and specifics you can provide would help move this forward. I'm still lost on what, exactly, I need to do here, sorry :(

[bcoe]

You cannot create the deprecation function and use it again in the same file. Running your code I'm not seeing a deprecation message from this module.

If you curl -XGET localhost:3000/success, I get:

deprecate-namespace deprecated deprecatedBut200 node_modules/express/lib/router/layer.js:95:5

In the logs.

---

I suppose the larger goal I'm suggesting is separate from depd, but related to it.

Without breaking express, I would like us to be able to run:

NODE_OPTIONS=--enable-source-maps npm run serve

And receive a stack trace that includes the correct original source position, like this:

Error: my stack trace stinks
    at /Users/bencoe/oss/express-ts-errors/build/src/index.js:14:15
        -> /Users/bencoe/oss/express-ts-errors/src/index.ts:19:11

I only brought up winston and bunyan because we don't want to interfere with their own modifications to stack trace behavior.

[dougwilson]

So because it is express that is calling the deprecation, the message seems to be right, right? Is there is something wrong with the deprecation message you out above?

The stack trace question does not seem related to this module, unless you can show where this module is making said stack trace you are seeing? It should likely belong in a meta issue somewhere vs in this issue tracker unless it is something this module could affect?

[dougwilson]

In the example, the stack trace you are pointing to you created with new Error() and then the printing seen is from calling .toString() on the object to get the string representation. Should the actual .toString() method on Error objects give the stack annotations you are looking for, or does everything in the Node.js ecosystem need to use some new method to turn error objects into strings?

[bcoe]

@dougwilson the root of the problem I'm trying to figure out a solution for, is that if a library like depd provides their own override for prepareStackTrace, then Node.js' current source-map implementation calls the overridden method, so you don't get the original source position.

There is however an API available, such that an implementor could perform the source-map lookup themselves, from Node.js' internal cache:

const sm = findSourceMap(t.getFileName(), error);

It would be nice to figure out a way for someone to have both useable stack traces when working in a transpiled language like TypeScript, and to have this deprecation functionality.

[dougwilson]

Sure, but this module only overrides prepareStackTrace to get the internal v8 objects. The result of that is not actually visible to folks using new Error(), especially since as soon as this module overrides it, it puts it back to the original immediately. I'm not sure how the override this module is doing would be affecting the way Node.js would behave without this module being loaded. Perhaps you can clarify with an example, showing how loading depd is altering the way the stack trace is appearing somewhere?

[dougwilson]

Maybe if I provide my console output from your repo, you can see why I'm confused. Here is the output from npm run serve:

$ npm run serve

> express-ts-errors@1.0.0 preserve /Users/doug.wilson/Code/depd-39
> npm run compile


> express-ts-errors@1.0.0 compile /Users/doug.wilson/Code/depd-39
> tsc -p .


> express-ts-errors@1.0.0 serve /Users/doug.wilson/Code/depd-39
> node build/src/index.js

Example app listening on port 3000!
Error: my stack trace stinks
    at /Users/doug.wilson/Code/depd-39/build/src/index.js:18:15
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
    at /Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:281:22
    at Function.process_params (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:275:10)
    at expressInit (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/middleware/init.js:40:5)
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
^C

Yes, that does not include the -> reference thing as noted, but I assume that is expected since there is no NODE_OPTIONS provided. But when I run the second one:

$ NODE_OPTIONS=--enable-source-maps npm run serve

> express-ts-errors@1.0.0 preserve /Users/doug.wilson/Code/depd-39
> npm run compile


> express-ts-errors@1.0.0 compile /Users/doug.wilson/Code/depd-39
> tsc -p .


> express-ts-errors@1.0.0 serve /Users/doug.wilson/Code/depd-39
> node build/src/index.js

Example app listening on port 3000!
Error: my stack trace stinks
    at /Users/doug.wilson/Code/depd-39/build/src/index.js:18:15
        -> /Users/doug.wilson/Code/depd-39/src/index.ts:26:11
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
    at /Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:281:22
    at Function.process_params (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/index.js:275:10)
    at expressInit (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/middleware/init.js:40:5)
    at Layer.handle [as handle_request] (/Users/doug.wilson/Code/depd-39/node_modules/express/lib/router/layer.js:95:5)
^C

I see the stack trace as it seems you want it to be seen... I'm just lost at what, exactly, I'm missing here? I'm happy work to to fix something, but I am just at a total loss of what I'm supposed to be seeing that is wrong?

[bcoe]

@dougwilson I apologize for the confusion, this is working for me for the latest Node 13 too; I wasn't following that you set aside the original prepareStackTrace, and then put it back again -- and I was initially testing on a Node.js version prior to our fix.

This is great news, sorry for wasting your time.