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

[Sveltekit] Strange behaviour of Source Maps #8294

Closed
3 tasks done
hrueger opened this issue Jun 6, 2023 · 24 comments
Closed
3 tasks done

[Sveltekit] Strange behaviour of Source Maps #8294

hrueger opened this issue Jun 6, 2023 · 24 comments

Comments

@hrueger
Copy link

hrueger commented Jun 6, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/sveltekit

SDK Version

7.54.0

Framework Version

@sveltejs/kit@1.20.1

Link to Sentry event

https://dein-ticket-shop.sentry.io/issues/4231736390/?project=4505312390610944&referrer=project-issue-stream

SDK Setup

Sentry.init({
    dsn: "[redacted]",
    tracesSampleRate: 1.0,
});

Steps to Reproduce

I just caught an error in my Sveltekit Project. However, the filepath is wrong:
grafik

However, source maps are uploaded and seem to be working partly?
grafik

Sentry was configured just as in the documentation, with the plugin in vite.config.js. No special settings / config, whatsoever.

Expected Result

I see the correct filename in sentry.

Actual Result

Wrong filename (see above)
I also can't connect GitHub, which is currently not possible due to the wrong filename:

grafik

(obviously there are no built files in my GitHub Repo).

Thanks for your help!

@lforst
Copy link
Member

lforst commented Jun 13, 2023

Hey, thanks for reporting this. Since the svelte-kit SDK (and the vite plugin) are still quite new we still have to figure out some stuff in regards to source maps.

The difficulty lies in svelte-kit seemingly having multiple build steps and keeping source maps and source files aligned throughout these build steps is a bit tricky. I believe we're in communication with Vercel and Rich Harris about this.

cc @Lms24

@hrueger
Copy link
Author

hrueger commented Jun 13, 2023

Sure, no worries 👍 It's not super-important to me, as the build output is actually quite readable and not too different from the source (at least for the *.server.ts files)

@Lms24
Copy link
Member

Lms24 commented Jun 20, 2023

Hey @hrueger 👋 just a few questions:

  • Which adapter are you using? Maybe we're incorrectly detecting the output path
  • Are you using a custom out directory (i.e. not .svelte-kit)?

@hrueger
Copy link
Author

hrueger commented Jun 20, 2023

Hi,
I'm using @sveltejs/adapter-node. No, no custom output directory. Here's my svelte.config.js:

import preprocess from 'svelte-preprocess';
import adapter from '@sveltejs/adapter-node';
import { vitePreprocess } from '@sveltejs/kit/vite';

const checkOrigin = process.env.NODE_ENV !== "development";

/** @type {import('@sveltejs/kit').Config} */
const config = {
	// Consult https://kit.svelte.dev/docs/integrations#preprocessors
	// for more information about preprocessors
	preprocess: [
		vitePreprocess(),
		preprocess({
			scss: {
				prependData: '@use "src/variables.scss" as *;'
			}
		})
	],

	kit: {
		adapter: adapter(),
		alias: {
			$myLib: '../_lib',
		},
		csrf: {
			checkOrigin
		}
	}
};

export default config;

So just the standard stuff, basically. I don't think it has something todo with the alias, since although prisma is declared inside the ../_lib folder, the file where the exception is thrown is not.

FYI I'm using a monorepo with multiple packages, but when I'm building, I'm doing that from inside the folder of the respective package, so that shouldn't be a problem either.

@github-actions
Copy link
Contributor

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 🥀

@hrueger
Copy link
Author

hrueger commented Jul 12, 2023

Still an issue

@Lms24
Copy link
Member

Lms24 commented Jul 12, 2023

Hey, sorry for letting this slip through!

I'll try to reproduce this so we can fix it. On first glance, it seems like the stack frame is rewritten incorrectly inside the SDK:
Your uploaded source map has ~/chunks/10-a6442050.js.map as a name, while your stack frame points to app:////app/server/chunks/10-a6442050.js. We made some modifications to stack frame rewriting while working on adding Vercel support (#8085) so maybe this caused something to break for node (although I tested this back then).

Meanwhile, can you confirm that the error happens inside a page.server.ts load function?

@hrueger
Copy link
Author

hrueger commented Jul 12, 2023

Hi @Lms24 no worries.

Meanwhile, can you confirm that the error happens inside a page.server.ts load function?

Yes, this error happens in a load function of a +page.server.ts.
However, it also happens for POST functions (RequestHandlers) of +server.ts files, see here for example.

@hrueger
Copy link
Author

hrueger commented Jul 12, 2023

Also, I noticed that with some sentry update, the UI changed a bit:

grafik

@Lms24
Copy link
Member

Lms24 commented Jul 12, 2023

Yes, this banner appears if source maps don't work and the stack trace is still bundled/minified. The message is very generic across all JS frameworks, so please ignore the Webpack/CLI part for now.

@Lms24
Copy link
Member

Lms24 commented Jul 12, 2023

@hrueger I tried reproducing this and so far I can't, meaning errors caught in server load functions or in server routes show up source-mapped for me. Please take a look at my repo (or feel free to fork it) and see where the configuration differs.

You said you configured the sentrySvelteKit plugin in your vite.config.ts file. Are you setting any special options?

I'm still sure that your server-side errors' stack frames are rewritten incorrectly but I can't yet figure out why.
During build, our plugin tries to detect which adapter you're using. It does that by looking up your package.json in your project root and checking for the adapter dependencies. Maybe something goes wrong here. Would you mind trying to override the auto-detection by setting the node adapter manually?

export default defineConfig({
  plugins: [
    sentrySvelteKit({
      adapter: 'node'
    }),
    sveltekit(),
  ],
});

Btw: The "Sentry not part of build pipeline" warning is buggy indeed. It shows up for my test errors although they're correctly source-mapped. If you encounter this as well, please ignore it, I already raised it internally with the respective team.

@hrueger
Copy link
Author

hrueger commented Jul 12, 2023

Hi @Lms24,
thanks for investigating.
You can find my vite.config.js file above (here). Where do you get the defineConfig function from?

I can't view the repository, maybe it's private?

Would you mind trying to override the auto-detection by setting the node adapter manually?

Sure, I'll do next time I'm deploying (probably tomorrow).

I wonder if it has to do with my monorepo setup. I have a couple of completely separate sveltekit apps in the packages folder but when building (using GitHub actions), I cwd to each folder before building. Does Sentry use the git repository root for something?
Here's the relevant step:

    - name: Build package
      run: yarn build
      working-directory: packages/${{ matrix.package }}
      env:
        SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
        SENTRY_ORG: dein-ticket-shop
        SENTRY_PROJECT: dts-${{ matrix.package }}

@Lms24
Copy link
Member

Lms24 commented Jul 13, 2023

You can find my vite.config.js file above (#8294 (comment)).

Sorry perhaps I'm missing something obvious but all I can see is your svelte.config.js 🤔

I can't view the repository, maybe it's private?

Whoops, sorry about that - I set it to public now

I wonder if it has to do with my monorepo setup. I have a couple of completely separate sveltekit apps in the packages folder but when building (using GitHub actions), I cwd to each folder before building

Hmm it's possible (monorepos always bring some challenges) but we usually look up things relative to process.cwd(). If overriding the adapter indeed fixes this, then chances are that it has something to do with the monorepo.

@Lms24
Copy link
Member

Lms24 commented Jul 13, 2023

Where do you get the defineConfig function from?

import { defineConfig } from 'vite';

but it shouldn't matter if you use it or not

@hrueger
Copy link
Author

hrueger commented Jul 13, 2023

Sorry perhaps I'm missing something obvious but all I can see is your svelte.config.js 🤔

Sorry, I mixed that up. Here's the vite.config.js:

import { sveltekit } from '@sveltejs/kit/vite';
import { sentrySvelteKit } from "@sentry/sveltekit";

/** @type {import('vite').UserConfig} */
const config = {
	plugins: [sentrySvelteKit(), sveltekit()],

	test: {
		include: ['src/**/*.{test,spec}.{js,ts}']
	},

	css: {
		preprocessorOptions: {
			scss: {
				additionalData: '@use "src/variables.scss" as *;'
			}
		}
	}
};

export default config;

@hrueger
Copy link
Author

hrueger commented Jul 14, 2023

Unfortunately, setting the adapter manually does not help. Here's a new error from today, but I don't see any difference regarding source maps: https://dein-ticket-shop.sentry.io/issues/4314817573/?project=4505312392904704&query=is%3Aunresolved&referrer=issue-stream&stream_index=0

@Lms24
Copy link
Member

Lms24 commented Jul 14, 2023

I had another thought. Can you describe a little what you're doing when deploying after building?

The reason I'm asking is that our plugin tries to find the build directory that's created by the Node adapter for the final output. It uploads the source maps from that directory. Any chance that you're changing the structure of that directory?
When rewriting the stack frames in the server-side SDK, we take build as a base path and remove everything before it.
Maybe that's how the stack frames get different paths and we need to account for it in some way.

Also, did you try running the production build locally within the sveltekit project and throwing a test error? Would be interesting to see if source maps work then. So:

  1. yarn build
  2. node ./build
  3. open app, throw a test error (preferrably within a +page.server.ts so that we can replicate)
  4. Check if error is source mapped in your project

@hrueger
Copy link
Author

hrueger commented Jul 14, 2023

Sure. So after yarn build the github action runs docker build -t ${{ secrets.DOCKER_USERNAME }}/dein-ticket-shop:${{ steps.version.outputs.tag }} . and pushes that.
Here's a dockerfile:

FROM node:18

RUN apt update && apt install tzdata -y
ENV TZ=Europe/Berlin

ENV BODY_SIZE_LIMIT=5242880
WORKDIR /app
COPY build .
COPY package.json .
COPY prisma .
ENV NODE_ENV production

RUN echo "nodeLinker: node-modules" > .yarnrc.yml && yarn set version berry && yarn plugin import workspace-tools && yarn workspaces focus --production && yarn setup

EXPOSE 3000

CMD npx prisma migrate deploy && node index.js

basically, it just copies build to /app.

However, I just found what could be the problem: I'm manually modifying js files after the build process. Somehow, lots of js files contain the following:

import 'string_decoder/';
import 'string_decoder';

That throws an error.
I have a fix script that just traverses all js files in the build dir and removes that trailing slash. That might cause the source maps to not match anymore?

It's hard to test, though, because I need to find and fix whats causing that trailing slash.

@Lms24
Copy link
Member

Lms24 commented Jul 14, 2023

basically, it just copies build to /app.

I think this might be it. I just reproduced this in my reproduction app I posted above:

  1. yarn build
  2. rename build to app
  3. node app

==> The error is not source mapped anymore.

It makes sense because our SDK (wrongfully in this case) assumes that the output directory of the adapter remains untouched. We need try to detect this better when rewriting the stack frame filenames or (if not possible) provide an option to override the directory name.

Meanwhile, you can probably fix this by modifying your svelte.config.js:

import adapter from '@sveltejs/adapter-node';
import { vitePreprocess } from '@sveltejs/kit/vite';

/** @type {import('@sveltejs/kit').Config} */
const config = {
	preprocess: vitePreprocess(),
	kit: {
		adapter: adapter({
			out: 'app' // <-- this will change the output directory from build to app. Our sentrySvelteKit plugin detects this automatically
		})
	}
};

and slightly adjusting your dockerfile:

ENV BODY_SIZE_LIMIT=5242880
WORKDIR /app
COPY app .
...

I have a fix script that just traverses all js files in the build dir and removes that trailing slash. That might cause the source maps to not match anymore?

I think a slight modification like this shouldn't cause too much harm but yes, changing JS after the build has potential to corrupt the source map linking. I'm not sure when or how the import is added but you might want to look into creating a custom vite plugin and using a tool like magic-string to rewrite the import and generate the changed source map.

@hrueger
Copy link
Author

hrueger commented Jul 14, 2023

That makes sense, thanks for the hint.
I have partial success with that. Please take a look at this issue: https://dein-ticket-shop.sentry.io/issues/4315187787/?project=4505312365838336&query=is%3Aunresolved&referrer=issue-stream&stream_index=0
The We've encountered 1 problem un-minifying your applications source code! warning is gone now, but the source map points to js files and not my ts source code?

@Lms24
Copy link
Member

Lms24 commented Jul 20, 2023

So we're one step further in this mystery, it seem 😅

I took a look at the uploaded source map of this issue and I can see that the sources array points to the _page.server.ts.js rather than to the actual source file. This is a know issue because the Node adapter does two things at build time that mess with source maps:

  1. it bundles the .svelte-kit output a second time. This means that the source maps of the node adapter output (in build or app) are only pointing to the already bundled js artifacts from the initial SvelteKit build.
  2. it copies files in the .svelte-kit directory, changes its directory hierarchy by doing so and hence destroys the relative paths in the sources array.

The SvelteKit team seems to be aware of both issues and they recently tried to fix 1 but had to revert due to some problems. Not sure if/when they're gonna attempt again.

Anyway, we actually use sorcery to address 1 internally in our plugin but it seems like this is not working in your case. sorcery is supposed to flatten the source map to directly link to the original source file instead of the intermediate output. I'm not yet sure why it's not doing that in your case though...

Since I'm still unable to reproduce this, can you turn on the debug option in sentrySvelteKit and look for some of these log messages:

[Source Maps Plugin] Looking up source maps in ...

[Source Maps Plugin] Flattening source maps

[Source Maps Plugin] error while flattening ...

I might have to ask you to add some console logs directly in your node_modules/@sentry/sveltekit/cjs/vite/sourceMaps.js around lines 150 to 190 to check what's happening in this loop. Really sorry that this is so hard to debug!

@hrueger
Copy link
Author

hrueger commented Jul 20, 2023

Hi, thanks for the next steps. I'll look into that on Sunday since I'm ooo till then.
I'll check if I can give you a stripped down version of my repo.

@hrueger
Copy link
Author

hrueger commented Jul 24, 2023

Hm, I stripped it down for you and suddenly it is working... very strange. I need to do that again and just remove bit by bit and see when it breaks. I'll keep you updated.

@getsantry
Copy link

getsantry bot commented Aug 17, 2023

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 remove the label Waiting for: Community, I will leave it alone ... forever!


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

@getsantry getsantry bot added the Stale label Aug 17, 2023
@getsantry getsantry bot closed this as completed Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants