feat: RewriteFrames pluggable integration for Node

[kamilogorek]

So that users don't have to write this nasty snippet anymore – https://docs.sentry.io/clients/node/sourcemaps/#updating-raven-configuration-to-support-source-maps

Configurable through:

init({
  integrations: [new Sentry.Integrations.RewriteFrames({
    iteratee(frame) {
      // do whatever to frames. Integration extracts them and itterate over for you.
    }
  })]
})

Ref: getsentry/sentry#2632 (comment)

[getsentry-bot]

	Messages
📖	✅ TSLint passed
📖	@sentry/browser gzip'ed minified size: 21.7666 kB

Generated by 🚫 dangerJS

[sheerun]

All good but honestly I don't like the basename(frame.filename) part. Wouldn't better default be paths relative from project root? Especially they are uploaded this way by sentry-cli.

[kamilogorek]

It's not that simple. I described it in here https://docs.sentry.io/clients/node/typescript/

[sheerun]

You don't need to figure out root directory by yourself, it can be passed as a flag to @sentry/node. This would be additionally beneficial as this way of implementing this would be backward compatible (no root provided, no paths rewritten).

[sheerun]

Another reason: bundling application into single file on node side practically never happens, usually application is executed without bundling anything

[kamilogorek]

Another reason: bundling application into single file on node side practically never happens, usually application is executed without bundling anything

It happens a lot in serverless environments :)

Everything you want to do can be easily achieved with:

init({
  integrations: [new Sentry.Integrations.RewriteFrames({
    iteratee: (frame) => ({
      ..frame,
      filename: frame.filename.startsWith('/') ? `app:///${path.relative(root, frame.filename)}` : frame.filename;
    })
  })]
})

Using basename itself worked fine for us since we introduced support for node source maps, that's why I ported it here in the same way.

[sheerun]

I guess this method it's fine if RewriteFrames is not enabled by default. Maybe little verbose and requires to import path and still write a bunch of code when almost always you mean to rewrite all paths relative to some dir. Maybe something like this would be better:

init({
  integrations: [new Sentry.Integrations.RewriteFrames({ root })]
})

(of course iteratee could stay for advanced use cases)

[kamilogorek]

Agree with root part. I'll change that. Aand no, RewriteFrames is not enabled by default, it's a "pluggable" integration. Thanks for the suggestion

[sheerun]

@kamilogorek Just one more question about decision to use app:/// prefix. It seems that sentry-cli upload-sourcemaps uploads files with ~/ prefix by default. Does Sentry treat these two on server side as equivalents? If not, isn't ~/ prefix a better default?

[kamilogorek]

No, app:/// is mapped to ~/, but they are not equivalent, so we have to use app:///

[sheerun]

maybe prefix option set by default to app:/// would be reasonable? this could be as well useful if uploaded sourcemaps need some directory prefix

init({
  integrations: [new Sentry.Integrations.RewriteFrames({ root, prefix })]
})

[kamilogorek]

You can already achieve this using custom iteratee, I don't want to pile up more and more options.

[kamilogorek]

Merged manually