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

fix(sveltekit): Check for cached requests in client-side fetch instrumentation #8391

Merged
merged 4 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/sveltekit/src/client/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type { LoadEvent } from '@sveltejs/kit';

import type { SentryWrappedFlag } from '../common/utils';
import { isRedirect } from '../common/utils';
import { isRequestCached } from './vendor/lookUpCache';

type PatchedLoadEvent = LoadEvent & Partial<SentryWrappedFlag>;

Expand Down Expand Up @@ -153,6 +154,11 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
return new Proxy(originalFetch, {
apply: (wrappingTarget, thisArg, args: Parameters<LoadEvent['fetch']>) => {
const [input, init] = args;

if (isRequestCached(input, init)) {
return wrappingTarget.apply(thisArg, args);
}
Comment on lines +158 to +160
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it. All this vendored code, just for this check 👌

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is our life now 🥲


const { url: rawUrl, method } = parseFetchArgs(args);

// TODO: extract this to a util function (and use it in breadcrumbs integration as well)
Expand Down Expand Up @@ -196,6 +202,7 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch

patchedInit.headers = headers;
}

let fetchPromise: Promise<Response>;

const patchedFetchArgs = [input, patchedInit];
Expand Down
57 changes: 57 additions & 0 deletions packages/sveltekit/src/client/vendor/buildSelector.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */

// Vendored from https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js
// with types only changes.

// The MIT License (MIT)

// Copyright (c) 2020 [these people](https://github.com/sveltejs/kit/graphs/contributors)

// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files(the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import { hash } from './hash';

/**
* Build the cache key for a given request
* @param {URL | RequestInfo} resource
* @param {RequestInit} [opts]
*/
export function build_selector(resource: URL | RequestInfo, opts: RequestInit | undefined): string {
const url = JSON.stringify(resource instanceof Request ? resource.url : resource);

let selector = `script[data-sveltekit-fetched][data-url=${url}]`;

if (opts?.headers || opts?.body) {
/** @type {import('types').StrictBody[]} */
const values = [];

if (opts.headers) {
// @ts-ignore - TS complains but this is a 1:1 copy of the original code and apparently it works
values.push([...new Headers(opts.headers)].join(','));
}

if (opts.body && (typeof opts.body === 'string' || ArrayBuffer.isView(opts.body))) {
values.push(opts.body);
}

selector += `[data-hash="${hash(...values)}"]`;
}

return selector;
}
51 changes: 51 additions & 0 deletions packages/sveltekit/src/client/vendor/hash.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/* eslint-disable no-bitwise */

// Vendored from https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/hash.js
// with types only changes.

// The MIT License (MIT)

// Copyright (c) 2020 [these people](https://github.com/sveltejs/kit/graphs/contributors)

// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files(the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import type { StrictBody } from '@sveltejs/kit/types/internal';

/**
* Hash using djb2
* @param {import('types').StrictBody[]} values
*/
export function hash(...values: StrictBody[]): string {
let hash = 5381;

for (const value of values) {
if (typeof value === 'string') {
let i = value.length;
while (i) hash = (hash * 33) ^ value.charCodeAt(--i);
} else if (ArrayBuffer.isView(value)) {
const buffer = new Uint8Array(value.buffer, value.byteOffset, value.byteLength);
let i = buffer.length;
while (i) hash = (hash * 33) ^ buffer[--i];
} else {
throw new TypeError('value must be a string or TypedArray');
}
}

return (hash >>> 0).toString(36);
}
79 changes: 79 additions & 0 deletions packages/sveltekit/src/client/vendor/lookUpCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* eslint-disable no-bitwise */

// Parts of this code are taken from https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js
// Attribution given directly in the function code below

// The MIT License (MIT)

// Copyright (c) 2020 [these people](https://github.com/sveltejs/kit/graphs/contributors)

// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files(the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:

// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.

// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import { WINDOW } from '@sentry/svelte';
import { getDomElement } from '@sentry/utils';

import { build_selector } from './buildSelector';

/**
* Checks if a request is cached by looking for a script tag with the same selector as the constructed selector of the request.
*
* This function is a combination of the cache lookups in sveltekit's internal client-side fetch functions
* - initial_fetch (used during hydration) https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js#L76
* - subsequent_fetch (used afterwards) https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js#L98
*
* Parts of this function's logic is taken from SvelteKit source code.
* These lines are annotated with attribution in comments above them.
*
* @param input first fetch param
* @param init second fetch param
* @returns true if a cache hit was encountered, false otherwise
*/
export function isRequestCached(input: URL | RequestInfo, init: RequestInit | undefined): boolean {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function isn't 100% copied but a good part of its logic is. Therefore I think this should also live in the vendor subdir.

// build_selector call copied from https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js#L77
const selector = build_selector(input, init);

const script = getDomElement<HTMLScriptElement>(selector);

if (!script) {
return false;
}

// If the script has a data-ttl attribute, we check if we're still in the TTL window:
try {
// ttl retrieval taken from https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js#L83-L84
const ttl = Number(script.getAttribute('data-ttl')) * 1000;

if (isNaN(ttl)) {
return false;
}

if (ttl) {
// cache hit determination taken from: https://github.com/sveltejs/kit/blob/1c1ddd5e34fce28e6f89299d6c59162bed087589/packages/kit/src/runtime/client/fetcher.js#L105-L106
return (
WINDOW.performance.now() < ttl &&
['default', 'force-cache', 'only-if-cached', undefined].includes(init && init.cache)
);
}
} catch {
return false;
}

// Otherwise, we check if the script has a content and return true in that case
return !!script.textContent;
}
7 changes: 6 additions & 1 deletion packages/sveltekit/test/client/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ vi.mock('@sentry/svelte', async () => {
};
});

vi.mock('../../src/client/vendor/lookUpCache', () => {
return {
isRequestCached: () => false,
};
});

const mockTrace = vi.fn();

const mockedBrowserTracing = {
Expand Down Expand Up @@ -433,7 +439,6 @@ describe('wrapLoadWithSentry', () => {
['is undefined', undefined],
["doesn't have a `getClientById` method", {}],
])("doesn't instrument fetch if the client %s", async (_, client) => {
// @ts-expect-error: we're mocking the client
mockedGetClient.mockImplementationOnce(() => client);

async function load(_event: Parameters<Load>[0]): Promise<ReturnType<Load>> {
Expand Down
45 changes: 45 additions & 0 deletions packages/sveltekit/test/client/vendor/lookUpCache.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { JSDOM } from 'jsdom';
import { vi } from 'vitest';

import { isRequestCached } from '../../../src/client/vendor/lookUpCache';

globalThis.document = new JSDOM().window.document;

vi.useFakeTimers().setSystemTime(new Date('2023-06-22'));
vi.spyOn(performance, 'now').mockReturnValue(1000);

describe('isRequestCached', () => {
it('should return true if a script tag with the same selector as the constructed request selector is found', () => {
globalThis.document.body.innerHTML =
'<script type="application/json" data-sveltekit-fetched data-url="/api/todos/1">{"status":200}</script>';

expect(isRequestCached('/api/todos/1', undefined)).toBe(true);
});

it('should return false if a script with the same selector as the constructed request selector is not found', () => {
globalThis.document.body.innerHTML = '';

expect(isRequestCached('/api/todos/1', undefined)).toBe(false);
});

it('should return true if a script with the same selector as the constructed request selector is found and its TTL is valid', () => {
globalThis.document.body.innerHTML =
'<script type="application/json" data-sveltekit-fetched data-url="/api/todos/1" data-ttl="10">{"status":200}</script>';

expect(isRequestCached('/api/todos/1', undefined)).toBe(true);
});

it('should return false if a script with the same selector as the constructed request selector is found and its TTL is expired', () => {
globalThis.document.body.innerHTML =
'<script type="application/json" data-sveltekit-fetched data-url="/api/todos/1" data-ttl="1">{"status":200}</script>';

expect(isRequestCached('/api/todos/1', undefined)).toBe(false);
});

it("should return false if the TTL is set but can't be parsed as a number", () => {
globalThis.document.body.innerHTML =
'<script type="application/json" data-sveltekit-fetched data-url="/api/todos/1" data-ttl="notANumber">{"status":200}</script>';

expect(isRequestCached('/api/todos/1', undefined)).toBe(false);
});
});
5 changes: 5 additions & 0 deletions packages/sveltekit/test/vitest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,8 @@ export function setup() {
};
});
}

if (!globalThis.fetch) {
// @ts-ignore - Needed for vitest to work with SvelteKit fetch instrumentation
globalThis.Request = class Request {};
}