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

Followed source map instructions, but line numbers are wrong, etc #2701

Closed
7 tasks done
ffxsam opened this issue Jun 27, 2020 · 25 comments
Closed
7 tasks done

Followed source map instructions, but line numbers are wrong, etc #2701

ffxsam opened this issue Jun 27, 2020 · 25 comments

Comments

@ffxsam
Copy link

ffxsam commented Jun 27, 2020

Package + Version

  • @sentry/node - 5.18.1
  • @sentry/integrations - 5.18.1
  • @sentry/webpack-plugin - 1.11.1

Description

I've followed the instructions on source maps to the best of my knowledge. I'm trying this out with a Serverless Framework setup.

It's quite an improvement over how reporting looked before I followed the steps, but the line number is totally wrong and the module name is missing. There's also still remnants of webpack variable names.

https://sentry.io/organizations/reelcrafter/issues/1749922899/?project=5298557

Screen Shot 2020-06-27 at 6 19 00 PM

webpack.config.js

const path = require("path");
// eslint-disable-next-line import/no-unresolved
const slsw = require("serverless-webpack");
const SentryPlugin = require("@sentry/webpack-plugin");

module.exports = {
  entry: slsw.lib.entries,
  devtool: "source-map",
  output: {
    libraryTarget: "commonjs",
    filename: "[name].js",
    path: path.join(__dirname, ".webpack"),
  },
  mode: "development",
  target: "node",
  module: {
    rules: [
      {
        test: /\.js$/, // include .js files
        enforce: "pre", // preload the jshint loader
        exclude: /node_modules/, // exclude any and all files in the node_modules folder
        include: __dirname,
        use: [
          {
            loader: "babel-loader",
          },
        ],
      },
    ],
  },
  plugins: [
    new SentryPlugin({
      release: process.env.RELEASE,
      include: "./.webpack",
    }),
  ],
};

test.js

import * as Sentry from "@sentry/node";
import { RewriteFrames } from "@sentry/integrations";

Sentry.init({
  dsn: "https://xx",
  release: process.env.RELEASE,
  integrations: [new RewriteFrames()],
});

/**
 ...
*/
@ffxsam ffxsam changed the title Followed source map instructions, line numbers are wrong, etc Followed source map instructions, but line numbers are wrong, etc Jun 27, 2020
@ffxsam
Copy link
Author

ffxsam commented Jun 27, 2020

The only part that maybe I deviated from:

If you want to rely on Sentry’s source map resolution, make sure that your code is not using the source-map-support package, as it overwrites the captured stack trace in a way that makes it impossible for our processors to correctly parse it.

source-map-support is in my project, which I think may break something if I were to remove it:

$ yarn why source-map-support                                                                                                  <aws:default>
yarn why v1.22.4
[1/4] 🤔  Why do we have the module "source-map-support"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "source-map-support@0.5.19"
info Has been hoisted to "source-map-support"
info Reasons this module exists
   - Hoisted from "serverless-webpack#ts-node#source-map-support"
   - Hoisted from "webpack#terser-webpack-plugin#terser#source-map-support"
info Disk size without dependencies: "940KB"
info Disk size with unique dependencies: "1.71MB"
info Disk size with transitive dependencies: "1.71MB"
info Number of shared dependencies: 2
=> Found "babel-register#source-map-support@0.4.18"
info This module exists because "babel-core#babel-register" depends on it.
info Disk size without dependencies: "96KB"
info Disk size with unique dependencies: "892KB"
info Disk size with transitive dependencies: "892KB"
info Number of shared dependencies: 1
✨  Done in 0.27s.

@kamilogorek
Copy link
Contributor

@ffxsam it looks like we don't even resolve your source maps, and the data itself comes from the local filesystem (its called pre/post_context data).

Can you try to turn it off and upload sourcemaps with a path that matches your stacktrace?

  1. in Sentry.init call, set frameContextLines to 0
  2. in WebpackPlugin change include: "./.webpack", to `include: "./.webpack/service"

Let me know once you change it and trigger an event. I'll then take a look at it again.

@ffxsam
Copy link
Author

ffxsam commented Jul 7, 2020

Hey @kamilogorek, thanks for the help!

So, I made the two changes you suggested. It's definitely better, but the referenced line number is just slightly off for some reason:

Screenshot_2020-07-07 ReferenceError fakeit is not defined

@ffxsam
Copy link
Author

ffxsam commented Jul 7, 2020

Another interesting note:

With the frameContextLines: 0 option, I get an error in the Sentry record:

Screen Shot 2020-07-07 at 10 50 24 AM

But if I comment out frameContextLines: 0, I no longer get this error.

@kamilogorek
Copy link
Contributor

But if I comment out frameContextLines: 0, I no longer get this error.

Because if we already have context lines, we don't resolve source maps.

Can you link the issue above?

When you see error like this, it means that release artifacts doesnt contain the file it points to.

@ffxsam
Copy link
Author

ffxsam commented Jul 7, 2020

Sure: https://sentry.io/organizations/reelcrafter/issues/1770164594/?project=5298557

This is my release script:

#!/bin/sh
export RELEASE=$(sentry-cli releases propose-version)

# Create a release
sentry-cli releases new -p big-test --finalize $RELEASE

# Associate commits with the release
sentry-cli releases set-commits --auto $RELEASE

# Deploy
sls deploy

And the output from running that script:

Created release ad78add8289b9e871192d9c17d98529f82015fda.
Could not determine any commits to be associated with a repo-based integration. Proceeding to find commits from local git tree.
Success! Set commits for release ad78add8289b9e871192d9c17d98529f82015fda.
Serverless: Bundling with Webpack...
> Found 2 release files
> Analyzing 2 sources
> Rewriting sources
> Adding source map references
> Bundled 2 files for upload
> Uploaded release files to Sentry
> File upload complete

Source Map Upload Report
  Minified Scripts
    ~/test.js (sourcemap at test.js.map)
  Source Maps
    ~/test.js.map
Time: 3183ms
Built at: 07/07/2020 2:14:39 PM
      Asset      Size  Chunks                   Chunk Names
    test.js  79.4 KiB       0  [emitted]        test
test.js.map   373 KiB       0  [emitted] [dev]  test
Entrypoint test = test.js test.js.map
 [3] ./node_modules/@sentry/hub/esm/index.js 197 bytes [built]
 [7] external "domain" 42 bytes {0} [built]
 [8] ./node_modules/@sentry/types/esm/index.js 146 bytes [built]
[18] multi ./node_modules/@sentry/webpack-plugin/src/sentry-webpack.module.js ./test.js 40 bytes {0} [built]
[19] ./node_modules/@sentry/webpack-plugin/src/sentry-webpack.module.js 187 bytes {0} [built]
[34] ./test.js + 49 modules 190 KiB {0} [built]
     | ./test.js 715 bytes [built]
     | ./node_modules/@sentry/node/esm/index.js 1.39 KiB [built]
     | ./node_modules/tslib/tslib.es6.js 10 KiB [built]
     | ./node_modules/@sentry/node/esm/version.js 118 bytes [built]
     | ./node_modules/@sentry/node/esm/transports/index.js 155 bytes [built]
     | ./node_modules/@sentry/node/esm/client.js 1.37 KiB [built]
     | ./node_modules/@sentry/core/esm/index.js 674 bytes [built]
     | ./node_modules/@sentry/node/esm/handlers.js 10.4 KiB [built]
     | ./node_modules/@sentry/node/esm/integrations/index.js 309 bytes [built]
     | ./node_modules/@sentry/node/esm/sdk.js 4.26 KiB [built]
     | ./node_modules/@sentry/integrations/esm/rewriteframes.js 3.54 KiB [built]
     | ./node_modules/@sentry/hub/esm/scope.js 12.5 KiB [built]
     | ./node_modules/@sentry/hub/esm/hub.js 14.2 KiB [built]
     | ./node_modules/@sentry/minimal/esm/index.js 5.99 KiB [built]
     | ./node_modules/@sentry/core/esm/baseclient.js 16.3 KiB [built]
     |     + 35 hidden modules
[36] ./node_modules/@sentry/integrations/esm/index.js 502 bytes [built]
[42] ./node_modules/@sentry/integrations/esm/angular.js 3.22 KiB [built]
[43] ./node_modules/@sentry/integrations/esm/captureconsole.js 2.68 KiB [built]
[44] ./node_modules/@sentry/integrations/esm/debug.js 1.52 KiB [built]
[45] ./node_modules/@sentry/integrations/esm/dedupe.js 5.91 KiB [built]
[46] ./node_modules/@sentry/integrations/esm/ember.js 1.98 KiB [built]
[47] ./node_modules/@sentry/integrations/esm/extraerrordata.js 3.69 KiB [built]
[48] ./node_modules/@sentry/integrations/esm/reportingobserver.js 3.54 KiB [built]
[49] ./node_modules/@sentry/integrations/esm/sessiontiming.js 1.31 KiB [built]
    + 40 hidden modules
Serverless: Packaging service...
Serverless: Uploading CloudFormation file to S3...
Serverless: Uploading artifacts...
Serverless: Uploading service sentry-serverless.zip file to S3 (154.57 KB)...
Serverless: Validating template...
Serverless: Updating Stack...
Serverless: Checking Stack update progress...
..............
Serverless: Stack update finished...
Service Information
service: sentry-serverless
stage: dev
region: us-east-1
stack: sentry-serverless-dev
resources: 11
api keys:
  None
endpoints:
  GET - https://█████.execute-api.us-east-1.amazonaws.com/dev/test
functions:
  test: sentry-serverless-dev-test
layers:
  None
Serverless: Removing old service artifacts from S3...

@kamilogorek
Copy link
Contributor

@ffxsam I spent some time reproducing everything on AWS with Serverless Framework and was able to get it working just fine

image

There is one necessary change in order for stacktrace to resolve only your real frames, as well as skip internal and mark them as "non-app frames".

import { basename } from "path";

Sentry.init({
  dsn: "...",
  release: process.env.RELEASE,
  integrations: [
    new RewriteFrames({
      iteratee(frame) {
        if (frame.function.startsWith("Runtime")) {
          frame.in_app = false;
        } else {
          frame.filename = `app:///${basename(frame.filename)}`;
        }
        return frame;
      }
    })
  ],
  frameContextLines: 0
});

However, it still doesn't answer the question why your sourcemaps are malformed. There's definitely a problem with webpack pipeline though. I used your exact config and it works just fine for me.
Notice, that when you download your sourcemaps and use tool like https://www.npmjs.com/package/source-map-cli it's already broken. Which means that they are not produced correctly in the first place.

@ffxsam
Copy link
Author

ffxsam commented Jul 8, 2020

Thanks for the additional info. I've created a fresh repository and just invited you to it:
https://github.com/ffxsam/source-map-test/

This was created via sls create -t aws-nodejs-ecma-script -p source-map-test.

This time, it doesn't work well at all, as you can see here: https://sentry.io/organizations/reelcrafter/issues/1772989686/?project=5319096

Any help would be greatly appreciated! We rely heavily on serverless architecture, so we really need Sentry to log everything accurately. Thanks!

@ffxsam
Copy link
Author

ffxsam commented Jul 8, 2020

Oh, and run sh release to commit the release to Sentry and deploy.

@kamilogorek
Copy link
Contributor

@ffxsam you forgot to import basename method from builtin path package 😅
I'll take a look at the rest of the code tomorrow and get back to you.

@ffxsam
Copy link
Author

ffxsam commented Jul 8, 2020

Oops! I don't remember if that was in there last time. The webpack config is pretty much what Serverless creates out of the box.

Thanks for your help! And feel free to modify my repo directly.

@kamilogorek
Copy link
Contributor

@ffxsam works like a charm, just import basename and you're done :)

image

@ffxsam
Copy link
Author

ffxsam commented Jul 9, 2020

@kamilogorek Still no luck for me.

Could you please clone this repo (https://github.com/ffxsam/source-map-test/), change the DSN, run sh release, and see what happens for you? I'm still getting this, even after your recommended changes:

image

@kamilogorek
Copy link
Contributor

@ffxsam there are 2 things I found.

  1. Your release script always uses same version (as propose-version is based on previous release + new commits [previous release is always same for this simple repro case]). This causes new events to use cached artifacts and resolve lines incorrectly. For testing purposes, you can use export RELEASE=$(date +%s) instead, which will always create a fresh release.

  2. mode: "production" is enabling source-map-support, which is messing up source completely, unfortunately I didn't find a way to disable it, as it's inside @babel/register plugin itself :|

Once I changed these 2 things, I got it working without any code changes.

@ffxsam
Copy link
Author

ffxsam commented Jul 9, 2020

@kamilogorek I've been purposely making commits in order to have a unique release hash each time, so that shouldn't be an issue.

I'll try removing mode: "production" and see what happens. I'll get back to you!

@ffxsam
Copy link
Author

ffxsam commented Jul 9, 2020

No luck for me. 😢

Same result as above, it's off by a few lines. I did try using date +%s as the release, just to be safe, and removed mode: "production" from the webpack config. You can check the repo. I even started with a fresh project/DSN too.

After I run sh release, I just do a curl https://xx.execute-api.us-east-1.amazonaws.com/dev/test to force an error, then I check Sentry.

BTW, I appreciate all your time and help! Don't worry, I'm a paid Sentry customer. 😆

@kamilogorek
Copy link
Contributor

WARNING in configuration. The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.

😶

I found the main issue. It's TerserPlugin messing around with the source code.

optimization: {
  minimize: false,
},

Add this to your webpack config and you should be good no matter what mode is set! (this is the only change I did base on your current repo state).

You can play around with TerserPlugin default options to find which exactly is the culprit if you want to use it nonetheless - https://webpack.js.org/plugins/terser-webpack-plugin/#terseroptions

BTW, I appreciate all your time and help! Don't worry, I'm a paid Sentry customer. 😆

Anytime, I'm curious myself what's going in here :D

@ffxsam
Copy link
Author

ffxsam commented Jul 10, 2020

@kamilogorek You rock!! This totally solved it, thank you!

So is this an issue with Webpack in general? Or just the way this Serverless template is set up?

@ffxsam
Copy link
Author

ffxsam commented Jul 10, 2020

BTW, it seems removing frameContextLines: 0 has no effect either way after all this.

@botond-veress
Copy link

@ffxsam I was struggling with this issue as well for a while. A few weeks ago I wrote an article on how to make it work. Maybe it helps you: Fix sentry sourcemaps in AWS lambda functions

TLDR:

  • package functions individually
  • prefix the file name with the function name in stacktraces so it will match the sls webpack structure that has been uploaded to sentry with a release

@kamilogorek
Copy link
Contributor

kamilogorek commented Jul 13, 2020

@botond-veress thanks for the write-up! :)

So is this an issue with Webpack in general? Or just the way this Serverless template is set up?

@ffxsam apparently there's a way to keep the optimizations if you preserve original sources (vide blogpost above). TIL!

@ffxsam
Copy link
Author

ffxsam commented Jul 13, 2020

@botond-veress There's something wrong with your blog post. The code blocks are all one line.

image

Also, the code in your repo doesn't seem to match the blog post.

@ffxsam
Copy link
Author

ffxsam commented Jul 13, 2020

Ahh, I just figured out why this solution was working fine on my test project, but not an actual real project!

In my real project, all my code is under src/. So I had to change the include in the webpack.config.js:

    new SentryPlugin({
      release: process.env.SENTRY_RELEASE,
      include: "./.webpack/service/src",
    }),

When it was just include: "./.webpack/service",, the source maps were way off.

@ipetrini
Copy link

For anyone who might still have problems with the source map line numbers while using Terser plugin, I could get it working properly by disabling the following compress options in terserOptions:

compress: {
  conditionals: false,
  sequences: false
}

@kamilogorek
Copy link
Contributor

Closing the issue, as it seems like the original issue has been resolved.
Please do not hesitate to ping me if it is still relevant, and I will happily reopen and work on it.
Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants