From 24e6346680431f8794fb3289f55568a987180c10 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 11 Aug 2023 14:37:54 +0200 Subject: [PATCH 1/3] fix(sveltekit): Avoid invalidating data o route changes in `wrapServerLoadWithSentry` --- packages/sveltekit/src/server/load.ts | 7 ++++- packages/sveltekit/test/server/load.test.ts | 29 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 0ad28e1cb4eb..8c7b4dc6b8df 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -119,7 +119,12 @@ export function wrapServerLoadWithSentry any>(origSe addNonEnumerableProperty(event as unknown as Record, '__sentry_wrapped__', true); - const routeId = event.route && event.route.id; + // Accessing any member of `event.route` causes SvelteKit to invalidate the + // server `load` function's data on every route change. + // To work around this, we use `Object.getOwnPropertyDescriptor` which doesn't invoke the proxy. + // https://github.com/sveltejs/kit/blob/e133aba479fa9ba0e7f9e71512f5f937f0247e2c/packages/kit/src/runtime/server/page/load_data.js#L111C3-L124 + const routeId = Object.getOwnPropertyDescriptor(event.route, 'id')?.value as string | undefined; + // const routeId = event.route.id; const { dynamicSamplingContext, traceparentData, propagationContext } = getTracePropagationData(event); getCurrentHub().getScope().setPropagationContext(propagationContext); diff --git a/packages/sveltekit/test/server/load.test.ts b/packages/sveltekit/test/server/load.test.ts index fa57dc6b7c46..90980204cf68 100644 --- a/packages/sveltekit/test/server/load.test.ts +++ b/packages/sveltekit/test/server/load.test.ts @@ -400,4 +400,33 @@ describe('wrapServerLoadWithSentry calls trace', () => { expect(mockTrace).toHaveBeenCalledTimes(1); }); + + it("doesn't invoke the proxy set on `event.route`", async () => { + const event = getServerOnlyArgs(); + + // simulates SvelteKit adding a proxy to `event.route` + // https://github.com/sveltejs/kit/blob/e133aba479fa9ba0e7f9e71512f5f937f0247e2c/packages/kit/src/runtime/server/page/load_data.js#L111C3-L124 + const proxyFn = vi.fn((target: { id: string }, key: string | symbol): any => { + return target[key]; + }); + + event.route = new Proxy(event.route, { + get: proxyFn, + }); + + const wrappedLoad = wrapServerLoadWithSentry(serverLoad); + await wrappedLoad(event); + + expect(mockTrace).toHaveBeenCalledTimes(1); + expect(mockTrace).toHaveBeenCalledWith( + expect.objectContaining({ + op: 'function.sveltekit.server.load', + name: '/users/[id]', // <-- this shows that the route was still accessed + }), + expect.any(Function), + expect.any(Function), + ); + + expect(proxyFn).not.toHaveBeenCalled(); + }); }); From 13d3cef9390a14afbb437e162104525bacfb830f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 11 Aug 2023 15:49:05 +0200 Subject: [PATCH 2/3] handle event.route being undefined --- packages/sveltekit/src/server/load.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 8c7b4dc6b8df..3546c5bc8759 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -123,7 +123,7 @@ export function wrapServerLoadWithSentry any>(origSe // server `load` function's data on every route change. // To work around this, we use `Object.getOwnPropertyDescriptor` which doesn't invoke the proxy. // https://github.com/sveltejs/kit/blob/e133aba479fa9ba0e7f9e71512f5f937f0247e2c/packages/kit/src/runtime/server/page/load_data.js#L111C3-L124 - const routeId = Object.getOwnPropertyDescriptor(event.route, 'id')?.value as string | undefined; + const routeId = event.route && (Object.getOwnPropertyDescriptor(event.route, 'id')?.value as string | undefined); // const routeId = event.route.id; const { dynamicSamplingContext, traceparentData, propagationContext } = getTracePropagationData(event); From 721c1dea5a8b54b7f3e6253f6966c44625a7f933 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 11 Aug 2023 15:55:20 +0200 Subject: [PATCH 3/3] Update packages/sveltekit/src/server/load.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Kamil Ogórek --- packages/sveltekit/src/server/load.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/sveltekit/src/server/load.ts b/packages/sveltekit/src/server/load.ts index 3546c5bc8759..04d0137062c6 100644 --- a/packages/sveltekit/src/server/load.ts +++ b/packages/sveltekit/src/server/load.ts @@ -124,7 +124,6 @@ export function wrapServerLoadWithSentry any>(origSe // To work around this, we use `Object.getOwnPropertyDescriptor` which doesn't invoke the proxy. // https://github.com/sveltejs/kit/blob/e133aba479fa9ba0e7f9e71512f5f937f0247e2c/packages/kit/src/runtime/server/page/load_data.js#L111C3-L124 const routeId = event.route && (Object.getOwnPropertyDescriptor(event.route, 'id')?.value as string | undefined); - // const routeId = event.route.id; const { dynamicSamplingContext, traceparentData, propagationContext } = getTracePropagationData(event); getCurrentHub().getScope().setPropagationContext(propagationContext);