From 80894c40440094185676e12a033f36acce67ac51 Mon Sep 17 00:00:00 2001 From: Leopold Kristjansson Date: Wed, 28 Aug 2024 13:42:12 +0200 Subject: [PATCH 1/6] Fix config example in README.md for Nuxt (#13496) Config example fix. env.DSN was missing process prefix. --- packages/nuxt/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nuxt/README.md b/packages/nuxt/README.md index 1227aa5e2c82..9049ec7e6691 100644 --- a/packages/nuxt/README.md +++ b/packages/nuxt/README.md @@ -96,7 +96,7 @@ Add a `sentry.client.config.(js|ts)` file to the root of your project: import * as Sentry from '@sentry/nuxt'; Sentry.init({ - dsn: env.DSN, + dsn: process.env.DSN, }); ``` From 664d30532dd619803e5cec39d11c956c11ca1867 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 28 Aug 2024 13:42:36 +0200 Subject: [PATCH 2/6] test: Increase timeout for redis-cache test & docker-compose (#13499) Noticed e.g. here: https://github.com/getsentry/sentry-javascript/actions/runs/10594383971/job/29358138105 that this was timing out some times, so increasing the timeout a bit... --- .../node-integration-tests/suites/tracing/redis-cache/test.ts | 2 +- dev-packages/node-integration-tests/utils/runner.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts index 8d494986ab3b..2b93c4d772bc 100644 --- a/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/redis-cache/test.ts @@ -1,7 +1,7 @@ import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; // When running docker compose, we need a larger timeout, as this takes some time... -jest.setTimeout(75000); +jest.setTimeout(90000); describe('redis cache auto instrumentation', () => { afterAll(() => { diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index cb4ab58347e7..76c19074ed9c 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -124,7 +124,7 @@ async function runDockerCompose(options: DockerOptions): Promise { const timeout = setTimeout(() => { close(); reject(new Error('Timed out waiting for docker-compose')); - }, 60_000); + }, 75_000); function newData(data: Buffer): void { const text = data.toString('utf8'); From 3158c21198319d3fd8459a2fd415372449035fd1 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 28 Aug 2024 13:42:46 +0200 Subject: [PATCH 3/6] ci: Ensure cache save happens after install step (#13497) Noticed e.g. here: https://github.com/getsentry/sentry-javascript/actions/runs/10594383971/job/29359100222 that saving of the cache was not working. I guess this only works for the combined restore/save step, but here it expects that the cached data is there immediately (which makes sense!). --- .github/actions/install-playwright/action.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/actions/install-playwright/action.yml b/.github/actions/install-playwright/action.yml index 9de6e1a2b104..8fc0aeba7330 100644 --- a/.github/actions/install-playwright/action.yml +++ b/.github/actions/install-playwright/action.yml @@ -21,15 +21,6 @@ runs: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} - # Only store cache on develop branch - - name: Store cached playwright binaries - uses: actions/cache/save@v4 - if: github.event_name == 'push' && github.ref == 'refs/heads/develop' - with: - path: | - ~/.cache/ms-playwright - key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} - # We always install all browsers, if uncached - name: Install Playwright dependencies (uncached) run: npx playwright install chromium webkit firefox --with-deps @@ -40,3 +31,12 @@ runs: run: npx playwright install-deps ${{ inputs.browsers || 'chromium webkit firefox' }} if: steps.playwright-cache.outputs.cache-hit == 'true' shell: bash + + # Only store cache on develop branch + - name: Store cached playwright binaries + uses: actions/cache/save@v4 + if: github.event_name == 'push' && github.ref == 'refs/heads/develop' + with: + path: | + ~/.cache/ms-playwright + key: playwright-${{ runner.os }}-${{ steps.playwright-version.outputs.version }} From 9e56c74b2a945ea5c4c33d5fe5c1466e8844e1ad Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 28 Aug 2024 14:26:40 +0200 Subject: [PATCH 4/6] fix(node): Suppress tracing for transport request execution rather than transport creation (#13491) https://github.com/getsentry/sentry-javascript/issues/13466#issuecomment-2312856449 correctly points out that we are suppressing tracing for the transport creation instead of the transport execution. This PR wraps the code that is actually conducting the request with `suppressTracing` instead of the transport creation. Fixes https://github.com/getsentry/sentry-javascript/issues/13466 --- packages/browser/src/transports/fetch.ts | 1 + packages/cloudflare/src/transport.ts | 20 ++-- packages/deno/src/transports/index.ts | 20 ++-- packages/node/src/integrations/http.ts | 5 - packages/node/src/transports/http.ts | 108 ++++++++++--------- packages/vercel-edge/src/transports/index.ts | 20 ++-- 6 files changed, 89 insertions(+), 85 deletions(-) diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 52ba6d71154c..f9a7c258d4ff 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -47,6 +47,7 @@ export function makeFetchTransport( } try { + // TODO: This may need a `suppresTracing` call in the future when we switch the browser SDK to OTEL return nativeFetch(options.url, requestOptions).then(response => { pendingBodySize -= requestSize; pendingCount--; diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index fd26b217c367..4f1314d693a7 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { SentryError } from '@sentry/utils'; @@ -89,14 +89,16 @@ export function makeCloudflareTransport(options: CloudflareTransportOptions): Tr ...options.fetchOptions, }; - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } diff --git a/packages/deno/src/transports/index.ts b/packages/deno/src/transports/index.ts index c678688c2462..1b2b3c661af9 100644 --- a/packages/deno/src/transports/index.ts +++ b/packages/deno/src/transports/index.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { consoleSandbox, logger, rejectedSyncPromise } from '@sentry/utils'; @@ -37,14 +37,16 @@ export function makeFetchTransport(options: DenoTransportOptions): Transport { }; try { - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } catch (e) { return rejectedSyncPromise(e); diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 615506605c9b..0d5b2d4814d1 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -9,7 +9,6 @@ import { getCapturedScopesOnSpan, getCurrentScope, getIsolationScope, - isSentryRequestUrl, setCapturedScopesOnSpan, } from '@sentry/core'; import { getClient } from '@sentry/opentelemetry'; @@ -102,10 +101,6 @@ export const instrumentHttp = Object.assign( return false; } - if (isSentryRequestUrl(url, getClient())) { - return true; - } - const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { return true; diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 751e4f3b3f4d..c4f13c89ee1b 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -79,11 +79,8 @@ export function makeNodeTransport(options: NodeTransportOptions): Transport { ? (new HttpsProxyAgent(proxy) as http.Agent) : new nativeHttpModule.Agent({ keepAlive, maxSockets: 30, timeout: 2000 }); - // This ensures we do not generate any spans in OpenTelemetry for the transport - return suppressTracing(() => { - const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); - return createTransport(options, requestExecutor); - }); + const requestExecutor = createRequestExecutor(options, options.httpModule ?? nativeHttpModule, agent); + return createTransport(options, requestExecutor); } /** @@ -122,54 +119,59 @@ function createRequestExecutor( const { hostname, pathname, port, protocol, search } = new URL(options.url); return function makeRequest(request: TransportRequest): Promise { return new Promise((resolve, reject) => { - let body = streamFromBody(request.body); - - const headers: Record = { ...options.headers }; - - if (request.body.length > GZIP_THRESHOLD) { - headers['content-encoding'] = 'gzip'; - body = body.pipe(createGzip()); - } - - const req = httpModule.request( - { - method: 'POST', - agent, - headers, - hostname, - path: `${pathname}${search}`, - port, - protocol, - ca: options.caCerts, - }, - res => { - res.on('data', () => { - // Drain socket - }); - - res.on('end', () => { - // Drain socket - }); - - res.setEncoding('utf8'); - - // "Key-value pairs of header names and values. Header names are lower-cased." - // https://nodejs.org/api/http.html#http_message_headers - const retryAfterHeader = res.headers['retry-after'] ?? null; - const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null; - - resolve({ - statusCode: res.statusCode, - headers: { - 'retry-after': retryAfterHeader, - 'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) ? rateLimitsHeader[0] || null : rateLimitsHeader, - }, - }); - }, - ); - - req.on('error', reject); - body.pipe(req); + // This ensures we do not generate any spans in OpenTelemetry for the transport + suppressTracing(() => { + let body = streamFromBody(request.body); + + const headers: Record = { ...options.headers }; + + if (request.body.length > GZIP_THRESHOLD) { + headers['content-encoding'] = 'gzip'; + body = body.pipe(createGzip()); + } + + const req = httpModule.request( + { + method: 'POST', + agent, + headers, + hostname, + path: `${pathname}${search}`, + port, + protocol, + ca: options.caCerts, + }, + res => { + res.on('data', () => { + // Drain socket + }); + + res.on('end', () => { + // Drain socket + }); + + res.setEncoding('utf8'); + + // "Key-value pairs of header names and values. Header names are lower-cased." + // https://nodejs.org/api/http.html#http_message_headers + const retryAfterHeader = res.headers['retry-after'] ?? null; + const rateLimitsHeader = res.headers['x-sentry-rate-limits'] ?? null; + + resolve({ + statusCode: res.statusCode, + headers: { + 'retry-after': retryAfterHeader, + 'x-sentry-rate-limits': Array.isArray(rateLimitsHeader) + ? rateLimitsHeader[0] || null + : rateLimitsHeader, + }, + }); + }, + ); + + req.on('error', reject); + body.pipe(req); + }); }); }; } diff --git a/packages/vercel-edge/src/transports/index.ts b/packages/vercel-edge/src/transports/index.ts index 4e8a35ac7c39..b938647b4415 100644 --- a/packages/vercel-edge/src/transports/index.ts +++ b/packages/vercel-edge/src/transports/index.ts @@ -1,4 +1,4 @@ -import { createTransport } from '@sentry/core'; +import { createTransport, suppressTracing } from '@sentry/core'; import type { BaseTransportOptions, Transport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types'; import { SentryError } from '@sentry/utils'; @@ -89,14 +89,16 @@ export function makeEdgeTransport(options: VercelEdgeTransportOptions): Transpor ...options.fetchOptions, }; - return fetch(options.url, requestOptions).then(response => { - return { - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - }; + return suppressTracing(() => { + return fetch(options.url, requestOptions).then(response => { + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); }); } From 5c1548560f41c2d88060c60f980d737f9bb0f97f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 28 Aug 2024 14:36:30 +0200 Subject: [PATCH 5/6] test: Avoid race conditions with symlinks (#13498) Noticed here: https://github.com/getsentry/sentry-javascript/actions/runs/10594383971/job/29358133582 that this was sometimes failing. While looking into this, we actually did unnecessary work here - we had two levels of symlinks. Now we simply have a single symlink, and since we have unique dirs now we can skip checking for existing files etc. --- .../utils/generatePlugin.ts | 32 +++++++++---------- .../utils/staticAssets.ts | 20 +++--------- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/dev-packages/browser-integration-tests/utils/generatePlugin.ts b/dev-packages/browser-integration-tests/utils/generatePlugin.ts index 30939c40c955..9189ce63812f 100644 --- a/dev-packages/browser-integration-tests/utils/generatePlugin.ts +++ b/dev-packages/browser-integration-tests/utils/generatePlugin.ts @@ -4,7 +4,7 @@ import type { Package } from '@sentry/types'; import HtmlWebpackPlugin, { createHtmlTagObject } from 'html-webpack-plugin'; import type { Compiler } from 'webpack'; -import { addStaticAsset, addStaticAssetSymlink } from './staticAssets'; +import { addStaticAsset, symlinkAsset } from './staticAssets'; const LOADER_TEMPLATE = fs.readFileSync(path.join(__dirname, '../fixtures/loader.js'), 'utf-8'); const PACKAGES_DIR = path.join(__dirname, '..', '..', '..', 'packages'); @@ -214,7 +214,10 @@ class SentryScenarioGenerationPlugin { src: 'cdn.bundle.js', }); - addStaticAssetSymlink(this.localOutPath, path.resolve(PACKAGES_DIR, bundleName, bundlePath), 'cdn.bundle.js'); + symlinkAsset( + path.resolve(PACKAGES_DIR, bundleName, bundlePath), + path.join(this.localOutPath, 'cdn.bundle.js'), + ); if (useLoader) { const loaderConfig = LOADER_CONFIGS[bundleKey]; @@ -245,14 +248,13 @@ class SentryScenarioGenerationPlugin { const fileName = `${integration}.bundle.js`; // We add the files, but not a script tag - they are lazy-loaded - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve( PACKAGES_DIR, 'feedback', BUNDLE_PATHS['feedback']?.[integrationBundleKey]?.replace('[INTEGRATION_NAME]', integration) || '', ), - fileName, + path.join(this.localOutPath, fileName), ); }); } @@ -262,26 +264,23 @@ class SentryScenarioGenerationPlugin { if (baseIntegrationFileName) { this.requiredIntegrations.forEach(integration => { const fileName = `${integration}.bundle.js`; - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve( PACKAGES_DIR, 'browser', baseIntegrationFileName.replace('[INTEGRATION_NAME]', integration), ), - fileName, + path.join(this.localOutPath, fileName), ); if (integration === 'feedback') { - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-modal.js'), - 'feedback-modal.bundle.js', + path.join(this.localOutPath, 'feedback-modal.bundle.js'), ); - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'feedback', 'build/bundles/feedback-screenshot.js'), - 'feedback-screenshot.bundle.js', + path.join(this.localOutPath, 'feedback-screenshot.bundle.js'), ); } @@ -295,10 +294,9 @@ class SentryScenarioGenerationPlugin { const baseWasmFileName = BUNDLE_PATHS['wasm']?.[integrationBundleKey]; if (this.requiresWASMIntegration && baseWasmFileName) { - addStaticAssetSymlink( - this.localOutPath, + symlinkAsset( path.resolve(PACKAGES_DIR, 'wasm', baseWasmFileName), - 'wasm.bundle.js', + path.join(this.localOutPath, 'wasm.bundle.js'), ); const wasmObject = createHtmlTagObject('script', { diff --git a/dev-packages/browser-integration-tests/utils/staticAssets.ts b/dev-packages/browser-integration-tests/utils/staticAssets.ts index 447a3ad337f7..81c18eec1dcf 100644 --- a/dev-packages/browser-integration-tests/utils/staticAssets.ts +++ b/dev-packages/browser-integration-tests/utils/staticAssets.ts @@ -22,23 +22,11 @@ export function addStaticAsset(localOutPath: string, fileName: string, cb: () => symlinkAsset(newPath, path.join(localOutPath, fileName)); } -export function addStaticAssetSymlink(localOutPath: string, originalPath: string, fileName: string): void { - const newPath = path.join(STATIC_DIR, fileName); - - // Only copy files once - if (!fs.existsSync(newPath)) { - fs.symlinkSync(originalPath, newPath); - } - - symlinkAsset(newPath, path.join(localOutPath, fileName)); -} - -function symlinkAsset(originalPath: string, targetPath: string): void { +export function symlinkAsset(originalPath: string, targetPath: string): void { try { - fs.unlinkSync(targetPath); + fs.linkSync(originalPath, targetPath); } catch { - // ignore errors here + // ignore errors here, probably means the file already exists + // Since we always build into a new directory for each test, we can safely ignore this } - - fs.linkSync(originalPath, targetPath); } From 6479e88dca621a4e00502a98336ce1eefaef603f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 28 Aug 2024 15:10:31 +0200 Subject: [PATCH 6/6] ref: Add external contributor to CHANGELOG.md (#13500) This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #13496 Co-authored-by: mydea <2411343+mydea@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fa005af46c5..8232b230c17c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @leopoldkristjansson. Thank you for your contribution! + ## 8.27.0 ### Important Changes