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

Remix sourcemaps don't have debugId injected when publicPath is set #9666

Closed
3 tasks done
brettdh opened this issue Nov 27, 2023 · 7 comments · Fixed by #9773
Closed
3 tasks done

Remix sourcemaps don't have debugId injected when publicPath is set #9666

brettdh opened this issue Nov 27, 2023 · 7 comments · Fixed by #9773

Comments

@brettdh
Copy link

brettdh commented Nov 27, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/remix

SDK Version

7.81.1

Framework Version

Remix 2.1.0

Link to Sentry event

n/a

SDK Setup

Omitting; this bug is sufficiently described with just the results of sentry-cli sourcemaps inject. (It hasn't been significantly modified from the results of following the setup guide, though.)

Steps to Reproduce

  1. Configure a Remix app using create-remix
  2. Make sure the remix.config.js includes a URL value for publicPath
    • Mine points to a public S3 bucket
  3. Follow the Sentry docs for setting up the Remix SDK
    • This includes creating a Sentry account and project, to be used as the --org and --project opts in a later step
  4. Run npx remix build --sourcemap
  5. Run npx sentry-cli sourcemaps inject --org <org> --project <project> --deleteAfterUpload=false
  6. Inspect any of the generated .js.map files in public/build/

Expected Result

For any given .js file in the Remix build output, both that source file and its .js.map file should contain a comment like this:

//# debugId=<uuid>

Actual Result

Each .js file contains the //# debugId=<uuid> comment, but none of the .js.map files do.

@lforst
Copy link
Member

lforst commented Nov 28, 2023

Hi, the sentry-cli sourcemaps inject does its best to find the source maps for the bundled files in the command's scope. If source maps are emitted into a location where it is impossible for the CLI to correlate a bundled files and their source maps, it's not gonna find it.

One way of dealing with this is having the source files and the bundled files colocated in the output. Another way would be to emit bundled js files with sourceMapURL comments with relative paths on the bottom pointing to their source maps.

@brettdh
Copy link
Author

brettdh commented Nov 28, 2023

@lforst in my case, the source map files are in the same directory as their corresponding source files, so I think it should be feasible for sentry-cli to find them.

I was unsure about where to file this issue; since it is somewhat Remix-specific, I opened it here, thinking that perhaps sentry-upload-sourcemaps should provide some option to sentry-cli sourcemaps inject that tells it a prefix to strip when looking up a source map file location from sourceMappingUrl. Perhaps sentry-cli sourcemaps inject should already be able to find source maps that are co-located with their source files, as a fallback even if sourceMappingUrl points to something that can't be found. However, this has the issue that, if I have already uploaded source maps to static hosting in a past deploy, they might be out of date when sentry-cli sourcemaps inject runs. So I'm not sure what the best approach to a fix would be.

@lforst
Copy link
Member

lforst commented Nov 28, 2023

@brettdh I just forwarded this to the team operating on sentry-cli. If it doesn't, sentry-cli should definitely look for co-located files. This is how we do things in our bundler plugins and seems to work reasonably well.

@brettdh
Copy link
Author

brettdh commented Nov 30, 2023

I opened getsentry/sentry-cli#1850 which should resolve this issue.

@lforst
Copy link
Member

lforst commented Dec 1, 2023

Thanks for the contribution. Once the CLI is released we'll have to bump the CLI version in the SDK too.

@brettdh
Copy link
Author

brettdh commented Dec 6, 2023

CLI is released 🥳

@Lms24
Copy link
Member

Lms24 commented Dec 7, 2023

Hi @brettdh thanks for fixing this! I opened #9773 to bump the CLI in the Remix SDK. It'll ship in the next release (probably next week).

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

Successfully merging a pull request may close this issue.

4 participants