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

Astro integration upload source map warning #9591

Closed
3 tasks done
youngboy opened this issue Nov 17, 2023 · 7 comments · Fixed by #9665
Closed
3 tasks done

Astro integration upload source map warning #9591

youngboy opened this issue Nov 17, 2023 · 7 comments · Fixed by #9665

Comments

@youngboy
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/astro

SDK Version

7.80.1

Framework Version

Astro 3.4.0

Link to Sentry event

No response

SDK Setup

    sentry({
      dsn: __DSN__,
      environment: process.env.VERCEL_ENV,
      release: `vivace@${version}`,
      debug: false,
      sourceMapsUploadOptions: {
        project: 'vivace',
        authToken: process.env.SENTRY_AUTH_TOKEN,
        telemetry: false
      }
    })

Steps to Reproduce

  1. build
  2. warning message occur
  3. dashboard releases pages shows Source Maps[0 artifacts]

error

Expected Result

can find source maps file in release detail

Actual Result

error message:

[sentry-vite-plugin] Warning: No release name provided. Will not inject release. Please set the release.name option to identify your release.

@Lms24
Copy link
Member

Lms24 commented Nov 17, 2023

Hi @youngboy thanks for writing in! The "no release name" warning should be safe to ignore because source maps no longer need to be associated with a release. (although we should do something to omit the warning or carry over the release from the release option)

A couple of questions to debug this:

  • Can you tell me, where in your project your built javascript is generated?
  • Can you see *.map.(m)js files next to them?
  • Which adapter (if any) are you using?
  • Does this happen locally when you build your project (when running npm run build or similar)?

@youngboy
Copy link
Author

@Lms24 Hi, thank you for your prompt reply.

I'm currently utilizing the Vercel adapter. During local builds, the process unexpectedly does not proceed to the sourcemap upload step.

However, the Vercel build logs indicate that despite some warnings, the sourcemap file is recognized and successfully uploaded. The issue arises in associating it with the release.

This can be verified on the dashboard page at "/settings/projects/vivace/source-maps/artifact-bundles/," as shown in this screenshot:

image

Additionally, in the release details page at "/releases/vivace%400.0.1/?project=4506240691798016," a similar situation is evident, as depicted in this screenshot:

image

@Lms24
Copy link
Member

Lms24 commented Nov 20, 2023

@youngboy You're using Sentry SaaS, correct? If yes, can you share a URL to an issue that should have source maps? This would help us debug this better. As I said, source maps don't have to be associated to the correct release anymore. We decoupled source maps and releases a while ago.

Also please let me know the answers to these two questions from my first reply:

  • Can you tell me, where in your project your built javascript is generated? (e.g. <projectRoot>/dist)
  • Can you see *.map.(m)js files next to the generated JS files?

@youngboy
Copy link
Author

youngboy commented Nov 20, 2023

the dir is in <projectRoot>/.vercel/output, and yes I can see sourcemap file next to js files

.vercel/output
├── config.json
├── functions
│   └── render.func
│       ├── app
│       ├── node_modules
│       └── package.json
└── static
    ├── _astro
    │   ├── BravuraText.d3d0c08e.svg
    │   ├── ViewTransitions.astro_astro_type_script_index_0_lang.5157f620.js
    │   ├── ViewTransitions.astro_astro_type_script_index_0_lang.5157f620.js.map
    │   ├── hoisted.12fde9f9.js
    │   ├── hoisted.12fde9f9.js.map
    │   ├── hoisted.c25269c9.js
    │   ├── hoisted.c25269c9.js.map
    │   ├── index.68b10ccf.css
    │   ├── page.dd9e33d9.js
    │   └── page.dd9e33d9.js.map

for vercel plugin, sourcemaps are in two dir ./dist/**/** and ./static/**/** see h ttps://github.com/withastro/astro/blob/main/packages/integrations/vercel/src/serverless/adapter.ts#L176-L177

I'll try to reproduce it with a minimal repo later.

@Lms24
Copy link
Member

Lms24 commented Nov 21, 2023

Thanks for the repro, I'll take a look!

for vercel plugin, sourcemaps are in two dir ./dist// and ./static// see h ttps://github.com/withastro/astro/blob/main/packages/integrations/vercel/src/serverless/adapter.ts#L176-L177

Oh I see, this is indeed something we didn't cover yet. We always assumed there's one output directory and within that, sub directories for client and server (I guess that would make too much sense lol). I have a feeling that we generally need to provide a user-configurable option for specifying the output directory. While I generally like the adapter pattern that more fullstack frameworks seem to be embracing, this severely messes up file output configs as they all do their own thing...

@Lms24
Copy link
Member

Lms24 commented Nov 21, 2023

Another thing I forgot: If you're deploying to Edge, the server SDK is probably not going to work correctly. We still need to add Edge support for Astro (which is a big challenge we only tackled in NextJS with mixed results so far).

edem-cyber pushed a commit to edem-cyber/sentry-javascript that referenced this issue Nov 30, 2023
…ntry#9668)

Adds the `assets` option to the Astro integration source maps
upload options.

It behaves just like the `assets` option of the Vite plugin, taking a
(array of) glob(s) and overriding the
default values. 
This came up in getsentry#9591 and I think it makes sense to give users with more
advanced/custom setups the ability to
override our defaults.
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.

2 participants