Skip to content
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

[22.9.0] util.getCallSite() doesn't report original line number for TS files #55109

Closed
bruce-c-liu opened this issue Sep 24, 2024 · 8 comments · Fixed by #55589
Closed

[22.9.0] util.getCallSite() doesn't report original line number for TS files #55109

bruce-c-liu opened this issue Sep 24, 2024 · 8 comments · Fixed by #55589
Labels
source maps Issues and PRs related to source map support. util Issues and PRs related to the built-in util module.

Comments

@bruce-c-liu
Copy link

bruce-c-liu commented Sep 24, 2024

What is the problem this feature will solve?

Lot of people use tools like ts-node or tsx when developing locally or even in production. The new util.getCallSite() function doesn't seem to report the line number from the original typescript file. Rather, it reports the transformed version of the file (e.g. by tsx) with types stripped out/minified/etc?

Example

expected: line 20, col 12
received: line 1, col 15123

What is the feature you are proposing to solve the problem?

Have util.getCallSite() report original line number for TS files.

What alternatives have you considered?

I currently have a custom function that does report the correct line number for the original typescript file. However, I would like to use the Node version if possible so I don't have to maintain this.

const ORIGINAL_STACK_TRACE_LIMIT = Error.stackTraceLimit;

/**
 * Returns information about the indicated frame in the stack trace.
 * frameIndex = 0 corresponds to the function that calls stackedTrace().
 *
 * Below sample values assume frameIndex = 1.
 * Inspiration: https://www.npmjs.com/package/callsites/v/3.1.0
 */
export function stackTrace(frameIndex = 0) {
  // Optimization: Adjust stackTraceLimit to the minimum depth required to get the desired frameIndex.
  // This significantly reduces the depth of the produced stack trace. (Node 22's default limit is 10)
  Error.stackTraceLimit = frameIndex + 1;
  const error = { stack: "" };

  // https://nodejs.org/api/errors.html#errorcapturestacktracetargetobject-constructoropt
  // Creates a `.stack` property on provided object
  Error.captureStackTrace(
    error,
    stackTrace // don't include the stackTrace() function itself in the trace
  );
  Error.stackTraceLimit = ORIGINAL_STACK_TRACE_LIMIT; // restore original stackTraceLimit

  // "Error
  //   at DerivedLogger.logger.<computed> (/Users/bruce.liu/Desktop/ai-search/server/logging/logger.ts:50:20)
  //   at Server.<anonymous> (/Users/bruce.liu/Desktop/ai-search/server/index.ts:58:12)"
  const stackErrorMsg = error.stack;

  // [
  //   "at DerivedLogger.logger.<computed> (/Users/bruce.liu/Desktop/ai-search/server/logging/logger.ts:50:20)",
  //   "at Server.<anonymous> (/Users/bruce.liu/Desktop/ai-search/server/index.ts:58:12)"
  // ]
  const frames = stackErrorMsg.split("\n").slice(1);

  // "at Server.<anonymous> (/Users/bruce.liu/Desktop/ai-search/server/index.ts:58:12)"
  let frameOfInterest = frames[frameIndex]!;

  // "at Server.<anonymous> /Users/bruce.liu/Desktop/ai-search/server/index.ts:58:12"
  frameOfInterest = frameOfInterest.replace(/[()]/g, ""); // remove "(" and ")"

  // "/Users/bruce.liu/Desktop/ai-search/server/index.ts:58:12"
  frameOfInterest = frameOfInterest.split(" ").at(-1)!;

  return frameOfInterest;
}
@bruce-c-liu bruce-c-liu added the feature request Issues that request new features to be added to Node.js. label Sep 24, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Sep 24, 2024
@avivkeller
Copy link
Member

IMO this makes sense. These third party tools and compiling the code before running it.

You can use node's experimental native typescript support instead?

@bruce-c-liu
Copy link
Author

@RafaelGSS for vis.

@redyetidev I understand, but I still need this functionality for debugging/logging purposes when using things like tsx/ts-node.

Seeing as how my custom function is able to achieve that, I assume there must be some way for util.getCallSite() to support it as well?

You can use node's experimental native typescript support instead?

I'd like to hold off on that until it's non-experimental.

@avivkeller
Copy link
Member

avivkeller commented Sep 24, 2024

IMO it's not Node.js's responsibility to account for the compilation of third party tools before the code is even executed, that should be on them

Even more so, the call site (IIUC) is gotten from V8 under the hood, so there's not really a reliable way to translate that into the values before compilation

@avivkeller avivkeller added the util Issues and PRs related to the built-in util module. label Sep 24, 2024
@artur-ma
Copy link

IMO it's not Node.js's responsibility to account for the compilation of third party tools before the code is even executed, that should be on them

Even more so, the call site (IIUC) is gotten from V8 under the hood, so there's not really a reliable way to translate that into the values before compilation

But there is a sourcemap that is supported natively in node js, so if ts-node / tsx generates sourcemap and the option is enabled in node, it should show the right line number, am I wrong?

@avivkeller
Copy link
Member

True, that doesn't occur. @bruce-c-liu is that what you are describing, getCallSite not respecting --enable-source-maps?

@bruce-c-liu
Copy link
Author

Correct. I'm not well-versed in the technical details, but it seems like tools like tsx do turn on source maps?
https://github.com/privatenumber/tsx/blob/master/src/source-map.ts

@alexanderniebuhr
Copy link

It even doesn't for with experimental TS support in node #55211

@avivkeller avivkeller added source maps Issues and PRs related to source map support. and removed feature request Issues that request new features to be added to Node.js. labels Oct 12, 2024
@joyeecheung
Copy link
Member

It comes down to whether util.getCallSite() wants to reach feature parity with err.stack. err.stack is generated by Node.js's stack trace preparation handler, which will map the source positions using the source maps it read from disk; util.getCallSite() on the other hand only does a small subset of what the stack trace preparation handler does. Not sure if anyone or @RafaelGSS wants to work on taking care of the mapping in util.getCallSite() as well, or if we just want to keep the API to only handle the basics as it does now.

nodejs-github-bot pushed a commit that referenced this issue Nov 4, 2024
PR-URL: #55589
Fixes: #55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 5, 2024
PR-URL: #55589
Fixes: #55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
aduh95 pushed a commit that referenced this issue Nov 16, 2024
PR-URL: #55589
Fixes: #55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
tpoisseau pushed a commit to tpoisseau/node that referenced this issue Nov 21, 2024
PR-URL: nodejs#55589
Fixes: nodejs#55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
marco-ippolito added a commit to marco-ippolito/node that referenced this issue Dec 10, 2024
PR-URL: nodejs#55589
Fixes: nodejs#55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
marco-ippolito added a commit that referenced this issue Dec 14, 2024
PR-URL: #55589
Backport-PR-URL: #56209
Fixes: #55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
aduh95 pushed a commit that referenced this issue Dec 18, 2024
PR-URL: #55589
Backport-PR-URL: #56209
Fixes: #55109
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source maps Issues and PRs related to source map support. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants