Skip to content

Commit

Permalink
ref(sveltekit): Clean up sub-request check (#15251)
Browse files Browse the repository at this point in the history
Removes a no longer necessary fallback check that we only needed in
SvelteKit 1.26.0 or older. For Kit 2.x, we can rely on the
`event.isSubRequest` flag to identify sub vs. actual requests in our
request handler.

fixes #15244
  • Loading branch information
Lms24 authored Feb 4, 2025
1 parent bb7237a commit 76854e8
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 56 deletions.
9 changes: 1 addition & 8 deletions packages/sveltekit/src/server/handle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
continueTrace,
getActiveSpan,
getCurrentScope,
getDefaultIsolationScope,
getIsolationScope,
Expand Down Expand Up @@ -100,19 +99,13 @@ export function sentryHandle(handlerOptions?: SentryHandleOptions): Handle {
};

const sentryRequestHandler: Handle = input => {
// event.isSubRequest was added in SvelteKit 1.21.0 and we can use it to check
// if we should create a new execution context or not.
// In case of a same-origin `fetch` call within a server`load` function,
// SvelteKit will actually just re-enter the `handle` function and set `isSubRequest`
// to `true` so that no additional network call is made.
// We want the `http.server` span of that nested call to be a child span of the
// currently active span instead of a new root span to correctly reflect this
// behavior.
// As a fallback for Kit < 1.21.0, we check if there is an active span only if there's none,
// we create a new execution context.
const isSubRequest = typeof input.event.isSubRequest === 'boolean' ? input.event.isSubRequest : !!getActiveSpan();

if (isSubRequest) {
if (input.event.isSubRequest) {
return instrumentHandle(input, options);
}

Expand Down
49 changes: 1 addition & 48 deletions packages/sveltekit/test/server/handle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,53 +149,6 @@ describe('sentryHandle', () => {
expect(spans).toHaveLength(1);
});

it('[kit>=1.21.0] creates a child span for nested server calls (i.e. if there is an active span)', async () => {
let _span: Span | undefined = undefined;
let txnCount = 0;
client.on('spanEnd', span => {
if (span === getRootSpan(span)) {
_span = span;
++txnCount;
}
});

try {
await sentryHandle()({
event: mockEvent(),
resolve: async _ => {
// simulating a nested load call:
await sentryHandle()({
event: mockEvent({ route: { id: 'api/users/details/[id]', isSubRequest: true } }),
resolve: resolve(type, isError),
});
return mockResponse;
},
});
} catch (e) {
//
}

expect(txnCount).toEqual(1);
expect(_span!).toBeDefined();

expect(spanToJSON(_span!).description).toEqual('GET /users/[id]');
expect(spanToJSON(_span!).op).toEqual('http.server');
expect(spanToJSON(_span!).status).toEqual(isError ? 'internal_error' : 'ok');
expect(spanToJSON(_span!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toEqual('route');

expect(spanToJSON(_span!).timestamp).toBeDefined();

const spans = getSpanDescendants(_span!).map(spanToJSON);

expect(spans).toHaveLength(2);
expect(spans).toEqual(
expect.arrayContaining([
expect.objectContaining({ op: 'http.server', description: 'GET /users/[id]' }),
expect.objectContaining({ op: 'http.server', description: 'GET api/users/details/[id]' }),
]),
);
});

it('creates a child span for nested server calls (i.e. if there is an active span)', async () => {
let _span: Span | undefined = undefined;
let txnCount = 0;
Expand All @@ -212,7 +165,7 @@ describe('sentryHandle', () => {
resolve: async _ => {
// simulating a nested load call:
await sentryHandle()({
event: mockEvent({ route: { id: 'api/users/details/[id]' } }),
event: mockEvent({ route: { id: 'api/users/details/[id]', isSubRequest: true } }),
resolve: resolve(type, isError),
});
return mockResponse;
Expand Down

0 comments on commit 76854e8

Please sign in to comment.