ionic-team / ionic-app-scripts

App Build Scripts for Ionic Projects
http://ionicframework.com/
MIT License
608 stars 303 forks source link

sourceMappingURL includes absolute path #856

Open DavidStrausz opened 7 years ago

DavidStrausz commented 7 years ago

Note: for support questions, please use one of these channels:

https://forum.ionicframework.com/ http://ionicworldwide.herokuapp.com/

Short description of the problem:

The last line of main.js specifies where the regarding sourcemap is located, I am not sure but I think in the previous versions of app-scripts this was just "//# sourceMappingURL=main.js.map". Is there a specifiy reason why the full path is included now? I'm asking because Sentry error tracking is unable to locate this path on their servers and won't find a matching sourcemap unless i manually change main.js. Sentry error: Source code was not found for C:\Users\\Desktop{project-folder}\www\build\main.js.map

What behavior are you expecting?

It should be a relative path => just "//# sourceMappingURL=main.js.map"

Steps to reproduce:

  1. ionic build android
  2. check the last line of www/build/main.js
insert any relevant code between the above and below backticks

Which @ionic/app-scripts version are you using? 1.2.2

Other information: (e.g. stacktraces, related issues, suggestions how to fix, stackoverflow links, forum links, etc)

elevente commented 7 years ago

The easiest way would be to use the sourceMapUrl parameter for uglifyJS, but it is not passed to uglifyJS in uglifyjs.ts I'm thinking about creating a pull request for it.

danbucholtz commented 7 years ago

@elevente,

Please do because I can pretty much guarantee we aren't going to get to this for a very long time 😸.

Thanks, Dan

DavidStrausz commented 7 years ago

@danbucholtz @elevente That would be great. I've been playing around with different configs for webpack/uglifyjs/sentry for a couple of hours now and I don't think the sourcemap is valid in case of a production build (when setting ionic_generate_source_map to true in package.json) because uglifyjs is running after generating the sourcemap.

Maybe uglifyjs should be part of the webpack config: new webpack.optimize.UglifyJsPlugin({ sourceMap: true })

elevente commented 7 years ago

@DavidStrausz Yes, uglifyJS is running after, but the sourcemaps still seem to be good, only the sourceMappingURL is wrong.

DavidStrausz commented 7 years ago

@elevente The odd thing is following:

I have ionic_generate_surce_map set to true in my package.json to keep the sourcemap when doing a production build. Then I upload the sourcemap to e.g. https://raygun.com/sourcemaps and check any position from main.js (line x, column: x) and I never get the correct source location.

This is basically the same wired behaviour I'm experiencing when changing the sourceMappingURL manually in main.js and uploading my files as artifacts to Sentry - when throwing an error the location in the minified code matches the position of my throw statement but when trying to view the "original" non minified code some completely different location is found by the sourcemap.

Correct position in minified main.js: error_mainjs_minified

Position in sourcemap: error_position_in_sourcemap

Same result on sentry: error_mapped_by_sentry

elevente commented 7 years ago

@DavidStrausz Hm, yes, I see it too... But for me it only seems to be a few lines further... I'll investigate more tomorrow.

DavidStrausz commented 7 years ago

@elevente Nice, let me know if you find something interesting :-) would be nice to have working sourcemaps for production builds.

elevente commented 7 years ago

@DavidStrausz I found exactly what you were saying: uglifying with webpack plugin makes a useable sourcemap, but then ionic-app-scripts uglify makes it totally wrong. :/ Best would be to somehow skip the ionic-app-scripts uglify...

DavidStrausz commented 7 years ago

@elevente maybe @danbucholtz can have a quick look at that if there is some time besides Ionic v3!

larssn commented 7 years ago

Wouldn't the solution just be:

I bet that would work.

elevente commented 7 years ago

@larssn yes, that would work

AndreasGassmann commented 7 years ago

@elevente Hey, thanks for the pull request. Sadly I still didn't get it to work. I uploaded the js-files/source maps to sentry with the --validate flag and got no errors, but it doesn't seem to work (I can just see the error messages, but no location of where the error occurred). Maybe it's not the source map but my sentry configuration, not sure... Did you do anything else? Or is there a way to validate everything once it has been uploaded to sentry?

elevente commented 7 years ago

@AndreasGassmann Hi! With this modification(and using the --noMinifyJs flag and using UglifyJS plugin from webpack config) when I upload the js and the map files to Sentry, it works perfectly for me... I can see the Minified files and the location of the error in them and there is a switch to use the Original files(from sourcemap) and show the errors in them. Do you see this switch in Sentry(so it recognizes the sourcemaps) or just the Minified files(so it does not even use the sourcemaps)?

AndreasGassmann commented 7 years ago

@elevente Ah, I didn't use the UglifyJS plugin from the webpack config, I just left everything how it was besides the --noMinifyJs flag.

It's really strange, I can see the files/sourcemaps in the release artifacts in the web-ui, but it somehow doesn't show any code snippets when I get an error. For example:

TypeError: undefined is not an object (evaluating 'this.photoLibrary')
  at onload(/main.js:47664:29)
  at n(/polyfills.js:2:27345)
  at wrapped(/main.js:130519:34)
  at invokeTask(/polyfills.js:3:10289)
  at onInvokeTask(/main.js:8103:47)
  at invokeTask(/polyfills.js:3:10232)
  at runTask(/polyfills.js:3:7647)
  at invoke(/polyfills.js:3:11404)

with a switch from "Raw" to "Full", which just changes the formatting a little. I assume I should be seeing the original snippet even if my source maps are wrong (just the wrong location)? It's strange I can't see anything at all... I have to look at my configuration again, maybe I made a mistake somewhere... Anyway, thanks for your help!

DavidStrausz commented 7 years ago

Hi @AndreasGassmann Make sure you verify your sourcemaps using a tool like that one: https://raygun.com/sourcemaps Another pitfall with sourcemaps on mobile devices is the url-prefix, you have to cut the url before sending an error to sentry. This blogpost explains the details: https://gonehybrid.com/how-to-log-errors-in-your-ionic-2-app-with-sentry/

AndreasGassmann commented 7 years ago

@DavidStrausz Thanks for the feedback. I didn't include the "url-prefix" parameter and it saved it like "~/main.js.map". Is that wrong? I also uploaded the source map to https://raygun.com/sourcemaps and it seemed to be valid, so I don't know what I'm doing wrong...

DavidStrausz commented 7 years ago

@AndreasGassmann If you're cutting the error url's in your dataCallback like described in the blog post your url-prefix should be "/". The explanation why you have to do that is also in the blog post.

RyanMcDonald commented 7 years ago

I'm not sure what I'm doing wrong, but I still can't get the source maps to be correct. Sentry continues to show the wrong source location for prod builds. Non-prod builds work fine though.

I made the changes shown in that PR, added the custom webpack config, followed all the instructions in the blog post, set ionic_generate_source_map to true, then ionic build --prod --release --noMinifyJs android, but still no luck. This is pretty important for me because it is making it more difficult to debug any errors that users of my app are currently seeing.

cesarlvielma commented 7 years ago

I was working on this issue by some days, I found an other extra change for @elevente solution, I disabled minifyJs and optimizeJs too. so replace this lines on @ionic/app-scripts/dist/util/config.js :

context.isProd || hasArg('--minifyJs')
by:
(context.isProd && !hasArg('--noMinifyJs')) || hasArg('--minifyJs')
and
context.isProd || hasArg('--optimizeJs')
by:
(context.isProd && !hasArg('--noMinifyJs')) || hasArg('--optimizeJs')

@RyanMcDonald , @AndreasGassmann , feel free to try that, I hope this help you.

nrcrabbe commented 7 years ago

Hi all, just wanted to bring our attention back to the original problem here: app-scripts places the absolute path of the sourcemap in the minified js file. In my (and others') case this breaks Sentry stacktraces. A relative URL is required.

Taking the minification task away from app-scripts and using a webpack plugin a la @elevente is a workaround, but @danbucholtz has made it clear that there are no current plans to include negative flags to the app-scripts CLI parameters. Plus doing it @danbucholtz 's way of specifying individual build flags does not set the ENV_VAR_IONIC_ENV to prod (I have no idea of the consequence) so it really isn't a like-for like.

So back to the original problem. @elevente you mentioned you could create a PR for this?

Thanks

elevente commented 7 years ago

@nrcrabbe the problem is that not only the sourceMappingUrl is wrong, but the source maps are incorrect also, so even if the Url would be fixed, source maps would still be invalid...

nrcrabbe commented 7 years ago

Oh okay, I interpreted that comment incorrectly then.

I did some brief testing after I set up a post build task to change the path from absolute to relative. The sourcemap pointed me to the opening line of the code block containing the error rather than the line of the error itself. I found it a little odd but put an error in a completely different module and it did the same thing. I just concluded that was how the stacktrace was reporting it. As I say, very brief testing... Is this what others have experienced or have I just experienced this by chance?

Anyway if this is a problem I still think it should be fixed rather than have ionic build a workaround. Is there a dedicated issue opened for the dodgy mapping? (Sorry I'm replying on my phone in bed, can't seem to search the issues register on the mobile interface).

fephil commented 7 years ago

@nrcrabbe I have the exact same situation as you where the sourcemap tells me the opening codeblock in sentry.

elevente commented 7 years ago

I had some bigger problems with my sourcemap, it was pointing to the wrong file... Sure, fixing would be the best, but they wrote that will probably not happen anytime soon, that's why I wanted at least a workaround. I don't know about any issue for this, sorry.

DavidStrausz commented 7 years ago

Switching to UglifyJs webpack plugin instead of an extra build step would fix the sourceMappingUrl and the wrong sourcemaps but I think Dan mentioned somewhere that this is to much overhead currently because they are focused on fixing or improving other things right now. Using a custom webpack config (with UglifyJs plugin) paired with a "custom" prod build (ionic cordova build --aot --optimizejs --minifycss) currently works quite good for me.

nrcrabbe commented 7 years ago

Yes I went down that path too thanks to the helpful comments here and on @elevente's PR. A downside I found while in development I had to comment out my custom webpack config where I added the plugin. Then when doing a prod build I'd forget to uncomment! Also that "custom" prod build doesn't set the ENV_VAR_IONIC_ENV to prod. I have no idea of the consequence of that.

So in summary I think it is great there is a workaround, but I want to keep the ball rolling to see if we can fix the original issue (plus another if the source mapping is broken too)

DavidStrausz commented 7 years ago

Yes you're right, thats the pain of this workaround! Hopefully they will fix this soon :-)

c0bra commented 6 years ago

So what's the latest on this? I'm having a terrible time getting sourcemaps to come out that will work with Sentry and so far I haven't found a solution from this thread.

kirillgroshkov commented 6 years ago

Struggling with this too. In a separate thread I found a small script to include sourceMappingUrl in every js file. But then Sentry is still not using it (or maybe cannot download them).

maefinho commented 6 years ago

Any updates regarding this issue?

Further can someone clarify to me what exact downsides the workaround has? I assume due to using not the standard minifying workflow and not setting the prod flag could lead into may be a worse performance of the app?