Skip to content

Sentry does not add sentry-header due to Webpack import re-ordering #4439

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

Closed
boukeversteegh opened this issue Jan 21, 2022 · 3 comments
Closed
Labels
Meta: Help Wanted Package: vue Issues related to the Sentry Vue SDK

Comments

@boukeversteegh
Copy link

boukeversteegh commented Jan 21, 2022

Description

Sentry documentation suggests to initialize Sentry "as early as possible", and the example does it after all application imports:

import Vue from "vue";
import Router from "vue-router";
import * as Sentry from "@sentry/vue";
import { Integrations } from "@sentry/tracing";

Vue.use(Router);

const router = new Router();

// Sentry is initialized after imports
Sentry.init(...)

This approach will cause Sentry to be initialized too late in certain (perhaps common) scenarios, and causes the sentry-header not to be sent when using Api clients that read window.fetch during its initialization.

Because this is unexpected behavior that is hard to diagnose, and the solution is also non-obvious, I would propose to add caution remarks in the Documentation, or to alter the documented code snippet.

Root Cause: Sentry should be initialized before imports that use window.fetch

Application imports will commonly end up at the top. Either by convention, or by force when using webpack. Vue JS does this.

A common scenario is importing an API module that initializes an API client by passing window.fetch into the constructor.

// api.ts
export default const apiClient = new ApiClient('https://api.example.com', window.fetch);
// main.ts
import Vue from 'vue'
import { ApiModule} from '@/api' // this module reads `window.fetch` and won't add any sentry headers.
import * as Sentry from '@sentry/vue'
import { Integrations } from '@sentry/tracing'

var router = ...

// It looks like we're initializing Sentry as early as possible, but we're not.
Sentry.init(...)

Vue.use(AuthModule)
// ... rest of initialization

In this scenario, the initialization is too late.

Moving the logic before the import doesn't work

import Vue from 'vue'

// Trying to initialize before loading the ApiModule.
// Webpack (or babel?) will (secretly) move the block of code AFTER the imports!
Sentry.init(...)

import { ApiModule } from '@/api'
import * as Sentry from '@sentry/vue'
import { Integrations } from '@sentry/tracing'

Webpack will forcefully move the initialization logic after the import statements.

  • Best case: This will happen while running npm run serve, and the source file is updated by eslint. The change is obvious.
  • Worst case: This re-ordering happens during npm run build, and the order is only changed in the dist output. Then it's completely mysterious why the init logic is out of order.

Solution

  • Moving the sentry initialization logic to a separate file, and importing it at the top of the main application file.
  • Unfortunately, because in the case of Vue, Sentry relies on the Router, and the main file also relies on the Router, the router definition has to be extracted in another file as well.

This works because the import order is maintained (at least in my project), whereas code statements are always moved after imports.

// main.ts
import "./sentry"
import Vue from "vue";
import { router } from "./router"
Vue.use(router);
// rest of vue initialization
// router.ts
import Vue from "vue";
export const router = new Router({
  // ...
});
// sentry.ts
import * as Sentry from "@sentry/vue";
import { Integrations } from "@sentry/tracing";
import { router } from "./router"

Sentry.init(...)

The solution is not ideal because it makes integrating Sentry seem more complex than it is, and requires changing the structure of the application files.

Better solutions are welcome of course.

In any case, I hope that Sentry users who are trying to figure out why Sentry Headers are not being added to their fetch request, will find this issue and can solve it without opening yet another ticket 😉

@AbhiPrasad
Copy link
Member

Hey thanks for writing in!

I actually think that this is something we can improve with the Vue package - similar concerns came up with #4427

We should probably just use the generic history for routing so that we can hook onto stuff as early as possible, and then have a custom method to add the router when appropriate:

// as soon as possible
Sentry.init(...)

Sentry.addRouter(router);

This means there might be some race conditions where the routes are not parameterized properly, but those are such rare error cases that is probably fine.

Will backlog, but PRs welcome to help address this (both here and in our docs)

@boukeversteegh
Copy link
Author

@AbhiPrasad thanks for your reply and backlogging this.

It would be very dev-friendly if it is possible to solve it in the SDK. There would be a way to keep the API the same.

By making the import of the sentry source (or a specific @sentry/vue/init script) do the pre-initialization, i.e. decorating the fetch object.

// Let this import immediately intercept window.fetch
import * as Sentry from "@sentry/vue";
import Vue from "vue";

export const router = new Router({
  // ...
});

// Now we're hooking Vue into pre-initialized sentry
Sentry.init(...)

The user can control when sentry decorates Fetch by moving it higher or lower.

For maximum flexibility, two scripts could be available, one that requires manual "pre init" and another one that does it immediately.

@HazAT
Copy link
Member

HazAT commented Jan 25, 2023

Closing - PRs welcome :)

@HazAT HazAT closed this as completed Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Help Wanted Package: vue Issues related to the Sentry Vue SDK
Projects
None yet
Development

No branches or pull requests

3 participants