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

[nextjs] support assetPrefix option #4174

Closed
2 tasks done
klapec opened this issue Nov 22, 2021 · 12 comments · Fixed by #6388
Closed
2 tasks done

[nextjs] support assetPrefix option #4174

klapec opened this issue Nov 22, 2021 · 12 comments · Fixed by #6388
Assignees
Labels

Comments

@klapec
Copy link

klapec commented Nov 22, 2021

Package + Version

  • @sentry/cli: 1.71.0
  • @sentry/nextjs: 6.15.0

Description

I have my Next.js (12.0.4) app served with an assetPrefix option in the next.config.js set to point to a custom CDN location.

Unfortunately, my on-premise Sentry instance seems to be not matching the sourcemaps that are being uploaded every time I do a production app build with the exception trace filename.

Here's the filename that I see in the issue's exception trace filename: /multi-prod/B991483/react/_next/static/chunks/pages/_app-40564a69361d2d0a.js

Here's the filename that I seen in the project's Source Maps > Archive page: ~/_next/static/chunks/pages/_app-40564a69361d2d0a.js (and a corresponding .js.map file: ~/_next/static/chunks/pages/_app-40564a69361d2d0a.js.map)

The actual script is served from https://cdnprod.example.com/multi-prod/B991483/react/_next/static/chunks/pages/_app-40564a69361d2d0a.js

And the script has a following sourceMappingURL comment: //# sourceMappingURL=_app-40564a69361d2d0a.js.map.

Is this all correct? The issue here is that every issue has problems with "Source code was not found".

@nikolay-govorov
Copy link

I am experiencing such difficulties - the application is used assetPrefix, which is not taken into account when downloading the release.

I noticed that the toolkit is already checking the basePath. Do you think it is possible to take into account assetPrefix in a similar way?

@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@klapec
Copy link
Author

klapec commented Jan 5, 2022

This is still relevant as Sentry is unaware of my source maps - which makes debugging quite difficult.

@nikolay-govorov
Copy link

This is still relevant for me, I can't use this package, I have to maintain my own configuration. Is there any way I can help in researching the problem?

@colbyjohnson-care
Copy link

colbyjohnson-care commented Jan 7, 2022

I have to maintain my own configuration. Is there any way I can help in researching the problem?

This is relevant for us as well. @nikolay-govorov any tips you have to begin on a placeholder configuration to get this working?

@smeubank
Copy link
Member

Hi all

we are taking this into our backlog to review, we are aware of the issue and appreciate the need to keep the ticket open.

@lobsterkatie lobsterkatie changed the title [nextjs] source maps aren't picked up if app scripts are served from a CDN [nextjs] support assetPrefix option Jan 13, 2022
@lukasvice
Copy link

lukasvice commented Mar 21, 2022

Is there any progress or ETA for this improvement? Thanks!

This is my workaround in the next.config.js:

const sentryNextConfig = withSentryConfig(nextConfig)

// Workaround: fix urlPrefix in Sentry source map config if assetPrefix is used in the Next config
const newWebpackFunction = (incomingConfig, buildContext) => {
  let newConfig = { ...incomingConfig };

  // If user has custom webpack config, run it so we have actual values to work with
  if ('webpack' in sentryNextConfig && typeof sentryNextConfig.webpack === 'function') {
    newConfig = sentryNextConfig.webpack(newConfig, buildContext);
  }

  // Find the SentryCliPlugin options and prepend the assetPrefix to the urlPrefix
  const sentryCliPlugin = newConfig.plugins?.find((p) => typeof p === 'object' && p.constructor?.name === 'SentryCliPlugin')
  sentryCliPlugin?.options?.include?.forEach(option => {
    if (option.urlPrefix) {
      option.urlPrefix = option.urlPrefix.replace('~', `~${sentryNextConfig.assetPrefix}`)
    }
  })

  return newConfig
}

nextConfig = {
  ...sentryNextConfig,
  webpack: newWebpackFunction
}

@jarvelov
Copy link

Thank you @lukasvice for the workaround. It works perfectly!

Hoping to see built in support for assetPrefix in Sentry someday.

@panghaoyuan
Copy link

panghaoyuan commented Nov 17, 2022

i have same qustion. I used a not very good way to solve this problem

@smeubank

I hope my issue will help you resolve this issue as soon as possible. (my issues)(#5072)

i think assetPrefix can perfect resolve this issue. such as

const urlPrefix = (userNextConfig.basePath || userNextConfig.assetPrefix) ? 
`~${userNextConfig.basePath || ''}${userNextConfig.assetPrefix || "}/_next` : 
'~/_next'; 

const urlPrefix = userNextConfig.basePath ? `~${userNextConfig.basePath}/_next` : '~/_next';

@tomas-c
Copy link

tomas-c commented Nov 20, 2022

The workaround from @panghaoyuan causes files to be uploaded without server/chunks/ or static/chunks/ in the path.

I suspect it's because while the workaround overwrites urlPrefix that is passed into the CLI tool, it does not overwrite urlPrefix used in serverIncludes and clientIncludes here:

? [{ paths: [`${distDirAbsPath}/serverless/`], urlPrefix: `${urlPrefix}/serverless` }]

lobsterkatie added a commit that referenced this issue Dec 2, 2022
This adds support support for the nextjs `assetPrefix` option to the nextjs SDK. Currently, if users set this option, it changes the filepaths in their client-side stacktraces in a way not reflected in their release artifact names, causing sourcemaps not to work. This fixes that by using `RewriteFrames` to modify the stacktrace filepaths so that they'll match the release artifacts.

Notes:

- Initial work on this was done by @tomas-c in #6241. (Thanks, @tomas-c!) The approach taken there was to change the way the client-side release artifacts are named, rather than to change the filenames in the stacktrace as is done here. After discussions with @lforst, I decided to take this approach because it's more consistent with what we already do on the server side. There, we use `RewriteFrames`, and we mutate the filepaths to start with `app:///_next`. This mirrors that for the client side.

- In the process of working on this, I discovered that we currently have a bug, caused by the way we handle the `basePath` option when naming release artifacts, including it at the beginning of all artifact names. Unfortunately, it's not included in stacktrace filenames, so this is a mistake. 

  On the server side, this makes the stacktrace filenames and the artifact filenames not match, meaning sourcemaps for people using that option are currently broken for all sever-side errors. (The reason this hasn't been more obvious is that in the node SDK, we harvest context lines at capture time, rather than relying on sourcemapping to provide them. Also, server-side built code is transpiled but not bundled or mangled, making even un-sourcemapped code much easier to read than it is on the browser side of things.)

  On the browser side, it doesn't break sourcemaps, but only because it turns out that nextjs copies `basePath` over into `assetPrefix` if a user provides the former but not the latter. As mentioned, `basePath` doesn't appear in stacktrace filepaths, but `assetPrefix` does, which is what led us to think was working.

  To fix this, this PR stops including `basePath` in artifact filenames. As a result, on both the server-side and client-side, all artifact filenames are now of the form `~/_next/...`, rather than some looking like that but others looking like `~/basePathValue/_next/...`.

- Part of the work here was figuring out how `distDir`, `basePath`, and `assetPrefix` interact, and where they show up in stacktrace filepaths. Just so it's written down somewhere, the answer is:
  - `basePath` - Never shows up in stacktrace filepaths, except insofar as it's copied into `assetPrefix` if it's set and `assetPrefix` isn't, in which case `assetPrefix` acts like it's path-only.
  - `distDir` - Server-side stacktrace filepaths are of the form `<pathToProjectDir>/<distDir>/server/...` (or `<pathToProjectDir>/<distDir>/serverless/...`). Example: `/path/on/host/machine/someDistDir/server/...`.
  - `assetPrefix` - If `assetPrefix` is a full url, client-side stacktrace filepaths are of the form `<assetPrefixHost>/<assetPrefixPath>/_next/static/...`. If `assetPrefix` just a path, stacktrace filepaths are of the form `<host>/<assetPrefixPath>/_next/static/...`. Examples: http://some.assetPrefix.host/some/assetPrefix/path/_next/...` and `http://some.normal.domain/some/assetPrefix/path/_next/...`.

Fixes #4174.
@lobsterkatie
Copy link
Member

Hey, all.

This should be fixed now, and will go out in the next release (some time next week).

@lucas0530
Copy link

is that solved?

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