Skip to content

Commit

Permalink
fix(v8/svelte): Guard component tracking beforeUpdate call (#15262)
Browse files Browse the repository at this point in the history
Add a guard to the Svelte SDK's component update tracking feature
to catch an error thrown by Svelte 5 in Runes mode. In Runes mode,
components must not call the old `beforeUpdate` or `afterUpdate`
lifecycle hooks.
  • Loading branch information
Lms24 authored Feb 3, 2025
1 parent d7ec8a9 commit d459577
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 34 deletions.
29 changes: 11 additions & 18 deletions packages/svelte/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,23 @@ export function withSentryConfig(
const mergedOptions = {
...DEFAULT_SENTRY_OPTIONS,
...sentryOptions,
componentTracking: {
...DEFAULT_SENTRY_OPTIONS.componentTracking,
...(sentryOptions && sentryOptions.componentTracking),
},
};

const originalPreprocessors = getOriginalPreprocessorArray(originalConfig);

// Map is insertion-order-preserving. It's important to add preprocessors
// to this map in the right order we want to see them being executed.
// see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map
const sentryPreprocessors = new Map<string, SentryPreprocessorGroup>();

const shouldTrackComponents = mergedOptions.componentTracking && mergedOptions.componentTracking.trackComponents;
if (shouldTrackComponents) {
const firstPassPreproc: SentryPreprocessorGroup = componentTrackingPreprocessor(mergedOptions.componentTracking);
sentryPreprocessors.set(firstPassPreproc.sentryId || '', firstPassPreproc);
// Bail if users already added the preprocessor
if (originalPreprocessors.find((p: PreprocessorGroup) => !!(p as SentryPreprocessorGroup).sentryId)) {
return originalConfig;
}

// We prioritize user-added preprocessors, so we don't insert sentry processors if they
// have already been added by users.
originalPreprocessors.forEach((p: SentryPreprocessorGroup) => {
if (p.sentryId) {
sentryPreprocessors.delete(p.sentryId);
}
});

const mergedPreprocessors = [...sentryPreprocessors.values(), ...originalPreprocessors];
const mergedPreprocessors = [...originalPreprocessors];
if (mergedOptions.componentTracking.trackComponents) {
mergedPreprocessors.unshift(componentTrackingPreprocessor(mergedOptions.componentTracking));
}

return {
...originalConfig,
Expand Down
5 changes: 0 additions & 5 deletions packages/svelte/src/constants.ts

This file was deleted.

17 changes: 11 additions & 6 deletions packages/svelte/src/performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/browser';
import type { Span } from '@sentry/core';
import { afterUpdate, beforeUpdate, onMount } from 'svelte';

import { startInactiveSpan } from '@sentry/core';
import { DEFAULT_COMPONENT_NAME, UI_SVELTE_INIT, UI_SVELTE_UPDATE } from './constants';
import { logger, startInactiveSpan } from '@sentry/core';
import type { TrackComponentOptions } from './types';

const defaultTrackComponentOptions: {
Expand All @@ -29,21 +28,27 @@ export function trackComponent(options?: TrackComponentOptions): void {

const customComponentName = mergedOptions.componentName;

const componentName = `<${customComponentName || DEFAULT_COMPONENT_NAME}>`;
const componentName = `<${customComponentName || 'Svelte Component'}>`;

if (mergedOptions.trackInit) {
recordInitSpan(componentName);
}

if (mergedOptions.trackUpdates) {
recordUpdateSpans(componentName);
try {
recordUpdateSpans(componentName);
} catch {
logger.warn(
"Cannot track component updates. This is likely because you're using Svelte 5 in Runes mode. Set `trackUpdates: false` in `withSentryConfig` or `trackComponent` to disable this warning.",
);
}
}
}

function recordInitSpan(componentName: string): void {
const initSpan = startInactiveSpan({
onlyIfParent: true,
op: UI_SVELTE_INIT,
op: 'ui.svelte.init',
name: componentName,
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
});
Expand All @@ -58,7 +63,7 @@ function recordUpdateSpans(componentName: string): void {
beforeUpdate(() => {
updateSpan = startInactiveSpan({
onlyIfParent: true,
op: UI_SVELTE_UPDATE,
op: 'ui.svelte.update',
name: componentName,
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.svelte' },
});
Expand Down
2 changes: 1 addition & 1 deletion packages/svelte/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('withSentryConfig', () => {

const wrappedConfig = withSentryConfig(originalConfig);

expect(wrappedConfig).toEqual({ ...originalConfig, preprocess: [sentryPreproc] });
expect(wrappedConfig).toEqual({ ...originalConfig });
});

it('handles multiple wraps correctly by only adding our preprocessors once', () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/svelte/test/performance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { getClient, getCurrentScope, getIsolationScope, init, startSpan } from '

import type { TransactionEvent } from '@sentry/core';

// @ts-expect-error svelte import
import DummyComponent from './components/Dummy.svelte';

const PUBLIC_DSN = 'https://username@domain/123';
Expand All @@ -32,7 +31,7 @@ describe('Sentry.trackComponent()', () => {

init({
dsn: PUBLIC_DSN,
enableTracing: true,
tracesSampleRate: 1.0,
beforeSendTransaction,
});
});
Expand Down Expand Up @@ -220,7 +219,7 @@ describe('Sentry.trackComponent()', () => {
expect(transaction.spans![1]?.description).toEqual('<CustomComponentName>');
});

it("doesn't do anything, if there's no ongoing transaction", async () => {
it("doesn't do anything, if there's no ongoing parent span", async () => {
render(DummyComponent, {
props: { options: { componentName: 'CustomComponentName' } },
});
Expand All @@ -230,7 +229,7 @@ describe('Sentry.trackComponent()', () => {
expect(transactions).toHaveLength(0);
});

it("doesn't record update spans, if there's no ongoing root span at that time", async () => {
it("doesn't record update spans, if there's no ongoing parent span at that time", async () => {
const component = startSpan({ name: 'outer' }, span => {
expect(span).toBeDefined();

Expand Down

0 comments on commit d459577

Please sign in to comment.