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

Metadata injection breaks SvelteKit error pages #593

Closed
fkling opened this issue Aug 27, 2024 · 4 comments · Fixed by getsentry/sentry-cli#2132
Closed

Metadata injection breaks SvelteKit error pages #593

fkling opened this issue Aug 27, 2024 · 4 comments · Fixed by getsentry/sentry-cli#2132
Assignees

Comments

@fkling
Copy link

fkling commented Aug 27, 2024

Environment

"@sentry/vite-plugin": "^2.22.2",
"@sveltejs/kit": "^2.5.17",
"@sentry/sveltekit": "^8.26.0",

Steps to Reproduce

Added the following to our vite config:

sentryVitePlugin({
  applicationKey: 'key',
}),

Expected Result

Opening a page that would cause an +error.svelte page to be rendered works without problems in preview mode (vite preview).

Actual Result

The page doesn't render and the console shows the following error:

ReferenceError: can't access lexical declaration 'me' before initialization

This is part of the generated and minified code:

// [...]
e._sentryModuleMetadata[(new me).stack] = Object.assign({
}, e._sentryModuleMetadata[(new me).stack], {
  '_sentryBundlerPluginAppKey:key': !0
});
// [...]
class me extends j {
  constructor(e) {
    super (),
    N(this, e, ue, de, r, {
    })
  }
}
export {
  me as component
};

So at first glance it appears that Sentry is using the wrong class to get the stack trace from. Looking at the unminified module code we can see why:

// [...]
  _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign(
    {},
    _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack],
    { "_sentryBundlerPluginAppKey:key": true }
  );
// [...]
class Error extends SvelteComponent {
  constructor(options) {
    super();
    init(this, options, instance, create_fragment, safe_not_equal, {});
  }
}
export {
  Error as component
};

SvelteKit generates components named Error for +error.svelte pages. Sentry is injected after SvelteKit's code generation and expects Error to refer to the built-in value.

Interestingly it works when referencing Error directly in the component somehow because that forces SvelteKit to use a different name for the component:

// [...]
function instance($$self, $$props, $$invalidate) {
  let $page;
  component_subscribe($$self, page, ($$value) => $$invalidate(0, $page = $$value));
  console.log(">>>>>", Error);
  return [$page];
}
class Error_1 extends SvelteComponent {
  constructor(options) {
    super();
    init(this, options, instance, create_fragment, safe_not_equal, {});
  }
}
export {
  Error_1 as component
};

I filed this as a bug here because even though I think SvelteKit should not shadow built-in classes, Sentry should be aware of the conventions that SvelteKit uses or at the very least account for the fact that Error might be overwritten.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 27, 2024
@Lms24
Copy link
Member

Lms24 commented Aug 27, 2024

Hey @fkling thanks for writing in and big thank you for doing all the detective work!

We honestly had to laugh a bit because it's so obvious that this could happen but we never thought about it. We'll fix this in a minute by ensuring we always access Error from the global object.

I think SvelteKit should not shadow built-in classes

I generally agree but chances are this is happening somewhere else and, more importantly, it could also happen in user-code at any time. So it definitely makes sense for us to keep this in mind. I'll open a PR soon.

@Lms24
Copy link
Member

Lms24 commented Aug 27, 2024

On another note: Is there a reason you're using the vite plugin directly instead of sentrySvelteKit()?

@fkling
Copy link
Author

fkling commented Aug 27, 2024

@Lms24 Thank you for the fast response!

On another note: Is there a reason you're using the vite plugin directly instead of sentrySvelteKit()?

No particular reason. I've stumbled upon this setting via https://sentry.io/changelog/ignore-errors-that-dont-come-from-your-code/ and then https://docs.sentry.io/platforms/javascript/configuration/filtering/#using-thirdpartyerrorfilterintegration.

Now that you mention it I remember that I read about sentrySvelteKit too. I'll switch to that!

@Lms24
Copy link
Member

Lms24 commented Aug 27, 2024

Now that you mention it I remember that I read about sentrySvelteKit too. I'll switch to that

Third party error filtering should work in sentrySvelteKit but you'll need to pass the options via unstable_sentryVitePluginOptions to the plugin because we didn't add this feature yet to the API exposed from @sentry/sveltekit. Probably a todo for the future.

I opened #594 to fix the bug. Expect it to be released soon. Note, if you switch to @sentry/sveltekit you'll either need to manually update to the latest @sentry/vite-plugin version or wait until we bump it from within the SDK. Could take a couple of days.

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