sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

[nextjs] Original source code visible in browser dev tools

Open kelvinndmo opened this issue 3 years ago • 6 comments

  • [x] Review the documentation: https://docs.sentry.io/
  • [x] Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
  • [x] Use the latest release: https://github.com/getsentry/sentry-javascript/releases
  • [ ] Provide a link to the affected event from your Sentry account

Package + Version

  • [ ] @sentry/browser
  • [ ] @sentry/node
  • [ ] raven-js
  • [ ] raven-node (raven for node)
  • [x] other:

Version:

6.17.3

Description

Describe your issue in detail, ideally, you have a reproducible demo that you can show.

I have used sentry/nextjs to upload the source maps to sentry, however, i can be able to see my source code on production, attached here is a screenshot, if i remove sentry from the next config file, the folder doesn't exist anymore, i have tried all options but haven't succeeded, any help will be appreciated.

Screenshot 2022-02-02 at 10 46 12

kelvinndmo avatar Feb 02 '22 14:02 kelvinndmo

Hi, @kelvinndmo.

What you want is a combo of hidden-source-map (which removes the sourceMappingURL comment at the bottom of your bundles, so Chrome won't look for maps) and not deploying the maps (so that even if one went looking, they wouldn't be there). You can turn hidden-source-maps on in your sentry config). As for not hosting sourcemaps, it depends on your platform, but likely the easiest way is just to delete the files after your app builds but before it deploys (or, if you can control what gets deployed, only deploy the .js files).

Cheers!

lobsterkatie avatar Feb 03 '22 01:02 lobsterkatie

I created a webpack plugin to achieve this with nextjs, it will build the sourcemaps, upload them to sentry and then remove them so they won't be there at the end of the build and won't be uploaded to your production server https://github.com/AlexBeauchemin/webpack-delete-sourcemaps-plugin

AlexBeauchemin avatar Mar 07 '22 20:03 AlexBeauchemin

Thanks for sharing! BTW, your readme says

For nextjs config using withSentryConfig, we can't set the devtool value to hidden-source-map as it's overriden by the sentry config wrapper

but that's no longer true: https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#use-hidden-source-map.

lobsterkatie avatar Mar 30 '22 02:03 lobsterkatie

Let's revisit this conversation and figure out if there's something we can do to improve DX.

  1. We need sourcmaps in Sentry
  2. We seemingly default them on (ok thats fine, we need them)
  3. If we are the responsible party for turning them on, what can we do to also restore the prior experience after we get what we need? e.g. delete them before app is published?

dcramer avatar Aug 19 '22 03:08 dcramer

We talked about this as a team, and we're going to deal with this in stages. There are a few different considerations here:

  • Default value of devtool (source-map or hiddden-source-map): Controls the existence of the sourceMappingURL comment at the bottom of bundles, affects whether or not chrome devtools looks for and auto-loads sourcemaps and sourcemapped code. Doesn't affect the existence of sourcemaps. (IOW, if you publish everything, they'll still be there if you know the correct URL.)
  • Existence of/potential deletion of sourcemaps: They can't be published if they don't exist because we delete them after uploading. That said, should we be in the business of deleting people's files?
  • Reason for sourcemaps' existence: Would they be there if not for us?
  • Order of config manipulation: We tell people to do module.exports = withSentryConfig(...) in their next.config.js, which means we're the last to touch the webpack config. If we are, it's easy to tell if we're the ones who set devtool. But can we guarantee that we're last?

Our first stage will be to assume we're last, check the incoming config, and if we're the ones to set devtool, set it to hidden-source-map by default. This is technically a breaking change, so we'll have to find the best way(s) to communicate this to users.

Our second stage will be to consider deleting sourcemaps after we upload them, again assuming that we're the last to change the webpack config.

Our third stage, if we decide to delete source maps, will be to actually parse next.config.js or run it in a sandbox or find some other way to determine if we actually are the last ones to modify the config and, depending on our approach, possibly figure out what happens after us if we're not the last.

We are going into a weeklong code freeze starting Monday (and will not make any releases before then), but will begin work on this today and pick it back up after after we return.

lobsterkatie avatar Aug 19 '22 16:08 lobsterkatie

Update:

A slightly modified version of the first stage mentioned above has now been PR-ed.

  • Existing users who have not set the hideSourceMaps option one way or the other will get a warning, during both prod and dev build, letting them know that sourcemaps may make original source code visible in browser dev tools, and letting them know how to prevent that.
  • New users setting up their app either by using the wizard or by following the nextjs with-sentry example will have hideSourceMaps explicitly set to true in their boilerplate next.config.js.
  • In the next major (version 8 of the SDK), we will switch the default for hideSourceMaps to true, a fact reflected in the aforementioned warning message, in a comment above the aforementioned spots where we're setting it explicitly, and in the docs.

Further update: All of the above are merged and released. I'm going to move this to the backlog for potential future work on stages 2 and 3 mentioned above. Hopefully this stage 1 implementation solves the immediate problem, though.

Last update (I swear!): In order to prepare for a possible future in which we delete source maps, I've filed a feature request in the sentry-cli repo, for a flag which could be passed to upload-sourcemaps and which would cause the cli to delete any maps it finds after it's done uploading.

lobsterkatie avatar Aug 30 '22 04:08 lobsterkatie

We can think about blocking requests for source maps with Next.js rewrites automatically.

lforst avatar Oct 30 '23 10:10 lforst