From ce986ca3079d10d500a56137b8a445316b0bd216 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Thu, 30 Nov 2023 08:51:11 -0500 Subject: [PATCH 1/8] build: enable biome linter (#9692) Enables Biome linter and moves several rules to Biome. Running `time yarn lint` ### Before ``` yarn lint 221.52s user 19.65s system 783% cpu 30.797 total ``` ### After ``` yarn lint 212.49s user 19.99s system 772% cpu 30.756 total ``` --- .eslintrc.js | 12 ++++++++++++ .gitignore | 1 + .vscode/settings.json | 8 ++++++-- biome.json | 18 +++++++++++++++++- packages/bun/scripts/install-bun.js | 2 +- .../manual/webpack-async-context/npm-build.js | 2 +- 6 files changed, 38 insertions(+), 5 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 6682e4b537d6..5a217a09fd75 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,11 +23,23 @@ module.exports = { 'types/**', ], overrides: [ + { + files: ['*'], + rules: { + // Disabled because it's included with Biome's linter + 'no-control-regex': 'off', + }, + }, { files: ['*.ts', '*.tsx', '*.d.ts'], parserOptions: { project: ['tsconfig.json'], }, + rules: { + // Disabled because it's included with Biome's linter + '@typescript-eslint/no-unused-vars': 'off', + '@typescript-eslint/no-loss-of-precision': 'off', + }, }, { files: ['test/**/*.ts', 'test/**/*.tsx'], diff --git a/.gitignore b/.gitignore index 07ba84c00e9e..813a38ad89d2 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,7 @@ local.log ._* .Spotlight-V100 .Trashes +.nx .rpt2_cache diff --git a/.vscode/settings.json b/.vscode/settings.json index 96bd2dfb42b9..d60ded5fea96 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -23,11 +23,15 @@ "yaml.schemas": { "https://json.schemastore.org/github-workflow.json": ".github/workflows/**.yml" }, - "eslint.packageManager": "yarn", "eslint.workingDirectories": [ { "mode": "auto" } ], - "deno.enablePaths": ["packages/deno/test"] + "deno.enablePaths": ["packages/deno/test"], + "editor.codeActionsOnSave": { + "source.organizeImports.biome": true, + "quickfix.biome": true + }, + "editor.defaultFormatter": "biomejs.biome" } diff --git a/biome.json b/biome.json index 4a6857b2325c..8e8155f8d6d8 100644 --- a/biome.json +++ b/biome.json @@ -9,7 +9,20 @@ "enabled": true }, "linter": { - "enabled": false + "enabled": true, + "rules": { + "recommended": false, + "correctness": { + "all": false, + "noUnusedVariables": "error", + "noPrecisionLoss": "error" + }, + "suspicious": { + "all": false, + "noControlCharactersInRegex": "error" + } + }, + "ignore": [".vscode/*", "**/*.json"] }, "files": { "ignoreUnknown": true @@ -37,6 +50,9 @@ "arrowParentheses": "asNeeded", "trailingComma": "all", "lineEnding": "lf" + }, + "parser": { + "unsafeParameterDecoratorsEnabled": true } }, "json": { diff --git a/packages/bun/scripts/install-bun.js b/packages/bun/scripts/install-bun.js index 33e798b0a6d6..a0ecfad6083c 100644 --- a/packages/bun/scripts/install-bun.js +++ b/packages/bun/scripts/install-bun.js @@ -1,7 +1,7 @@ /* eslint-disable no-console */ if (process.env.CI) { // This script is not needed in CI we install bun via GH actions - return; + process.exit(0); } const { exec } = require('child_process'); const https = require('https'); diff --git a/packages/node/test/manual/webpack-async-context/npm-build.js b/packages/node/test/manual/webpack-async-context/npm-build.js index 289cdd683dd2..eac357b10f36 100644 --- a/packages/node/test/manual/webpack-async-context/npm-build.js +++ b/packages/node/test/manual/webpack-async-context/npm-build.js @@ -4,7 +4,7 @@ const { execSync } = require('child_process'); // Webpack test does not work in Node 18 and above. if (Number(process.versions.node.split('.')[0]) >= 18) { - return; + process.exit(0); } // biome-ignore format: Follow-up for prettier From b7fdb7d8f85a24437ad74ba205dfdb3370cfb4c9 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 30 Nov 2023 11:59:09 -0330 Subject: [PATCH 2/8] ref(feedback): Change form `box-shadow` to use CSS var (#9630) This makes it consistent with the buttons box shadow, so they both have the same box shadow by default. --- packages/feedback/src/widget/Dialog.css.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/feedback/src/widget/Dialog.css.ts b/packages/feedback/src/widget/Dialog.css.ts index c02a20de43f4..b9cd5a499c13 100644 --- a/packages/feedback/src/widget/Dialog.css.ts +++ b/packages/feedback/src/widget/Dialog.css.ts @@ -47,9 +47,7 @@ export function createDialogStyles(d: Document): HTMLStyleElement { max-height: calc(100% - 2rem); display: flex; flex-direction: column; - box-shadow: - 0 0 0 1px rgba(0, 0, 0, 0.05), - 0 4px 16px rgba(0, 0, 0, 0.2); + box-shadow: var(--box-shadow); transition: transform 0.2s ease-in-out; transform: translate(0, 0) scale(1); } From 08308664394fb9eabda9f334f4cf97048fa0a4d9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 30 Nov 2023 16:31:12 +0100 Subject: [PATCH 3/8] feat(crons): Add interface for heartbeat checkin (#9706) --- packages/core/src/server-runtime-client.ts | 4 +-- packages/node/test/client.test.ts | 32 ++++++++++++++++++++++ packages/types/src/checkin.ts | 9 +++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 398a4224ef6d..4165aec8fc46 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -147,7 +147,7 @@ export class ServerRuntimeClient< * to create a monitor automatically when sending a check in. */ public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig, scope?: Scope): string { - const id = checkIn.status !== 'in_progress' && checkIn.checkInId ? checkIn.checkInId : uuid4(); + const id = 'checkInId' in checkIn && checkIn.checkInId ? checkIn.checkInId : uuid4(); if (!this._isEnabled()) { DEBUG_BUILD && logger.warn('SDK not enabled, will not capture checkin.'); return id; @@ -164,7 +164,7 @@ export class ServerRuntimeClient< environment, }; - if (checkIn.status !== 'in_progress') { + if ('duration' in checkIn) { serializedCheckIn.duration = checkIn.duration; } diff --git a/packages/node/test/client.test.ts b/packages/node/test/client.test.ts index ab302c8e1f08..d868fb6eddb1 100644 --- a/packages/node/test/client.test.ts +++ b/packages/node/test/client.test.ts @@ -354,6 +354,38 @@ describe('NodeClient', () => { ]); }); + it('sends a checkIn envelope for heartbeat checkIns', () => { + const options = getDefaultNodeClientOptions({ + dsn: PUBLIC_DSN, + serverName: 'server', + release: '1.0.0', + environment: 'dev', + }); + client = new NodeClient(options); + + // @ts-expect-error accessing private method + const sendEnvelopeSpy = jest.spyOn(client, '_sendEnvelope'); + + const id = client.captureCheckIn({ monitorSlug: 'heartbeat-monitor', status: 'ok' }); + + expect(sendEnvelopeSpy).toHaveBeenCalledTimes(1); + expect(sendEnvelopeSpy).toHaveBeenCalledWith([ + expect.any(Object), + [ + [ + expect.any(Object), + { + check_in_id: id, + monitor_slug: 'heartbeat-monitor', + status: 'ok', + release: '1.0.0', + environment: 'dev', + }, + ], + ], + ]); + }); + it('does not send a checkIn envelope if disabled', () => { const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, serverName: 'bar', enabled: false }); client = new NodeClient(options); diff --git a/packages/types/src/checkin.ts b/packages/types/src/checkin.ts index 786285554495..992c1781164e 100644 --- a/packages/types/src/checkin.ts +++ b/packages/types/src/checkin.ts @@ -43,6 +43,13 @@ export interface SerializedCheckIn { }; } +export interface HeartbeatCheckIn { + // The distinct slug of the monitor. + monitorSlug: SerializedCheckIn['monitor_slug']; + // The status of the check-in. + status: 'ok' | 'error'; +} + export interface InProgressCheckIn { // The distinct slug of the monitor. monitorSlug: SerializedCheckIn['monitor_slug']; @@ -61,7 +68,7 @@ export interface FinishedCheckIn { duration?: SerializedCheckIn['duration']; } -export type CheckIn = InProgressCheckIn | FinishedCheckIn; +export type CheckIn = HeartbeatCheckIn | InProgressCheckIn | FinishedCheckIn; type SerializedMonitorConfig = NonNullable; From f8cebde5884e576fab59b4eb0c17703aba4fd14a Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Fri, 1 Dec 2023 12:59:14 -0330 Subject: [PATCH 4/8] feat(feedback): Include Feedback package in browser SDK (#9586) Add feedback to craft publish list, and export `Feedback` integration from browser SDK Depends on - [x] https://github.com/getsentry/sentry-javascript/pull/9588 - [x] https://github.com/getsentry/sentry-javascript/pull/9604 Relates to: https://github.com/getsentry/team-replay/issues/273 --- .craft.yml | 4 ++ .size-limit.js | 30 +++++++-- packages/browser/package.json | 1 + packages/browser/rollup.bundle.config.js | 20 +++++- packages/browser/src/index.bundle.feedback.ts | 14 ++++ packages/browser/src/index.bundle.replay.ts | 4 +- .../index.bundle.tracing.replay.feedback.ts | 18 +++++ .../src/index.bundle.tracing.replay.ts | 3 +- packages/browser/src/index.bundle.tracing.ts | 4 +- packages/browser/src/index.bundle.ts | 4 +- packages/browser/src/index.ts | 2 + .../test/unit/index.bundle.feedback.test.ts | 25 +++++++ .../test/unit/index.bundle.replay.test.ts | 4 +- .../browser/test/unit/index.bundle.test.ts | 8 ++- ...dex.bundle.tracing.replay.feedback.test.ts | 25 +++++++ .../unit/index.bundle.tracing.replay.test.ts | 3 + .../test/unit/index.bundle.tracing.test.ts | 4 +- packages/feedback/package.json | 4 +- packages/feedback/src/integration.ts | 2 +- packages/integration-shims/src/Feedback.ts | 67 +++++++++++++++++++ packages/integration-shims/src/index.ts | 1 + packages/tracing-internal/tsconfig.types.json | 1 + yarn.lock | 9 +++ 23 files changed, 238 insertions(+), 19 deletions(-) create mode 100644 packages/browser/src/index.bundle.feedback.ts create mode 100644 packages/browser/src/index.bundle.tracing.replay.feedback.ts create mode 100644 packages/browser/test/unit/index.bundle.feedback.test.ts create mode 100644 packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts create mode 100644 packages/integration-shims/src/Feedback.ts diff --git a/.craft.yml b/.craft.yml index 46f270120a91..1a4b07c18874 100644 --- a/.craft.yml +++ b/.craft.yml @@ -28,6 +28,10 @@ targets: - name: npm id: '@sentry/opentelemetry' includeNames: /^sentry-opentelemetry-\d.*\.tgz$/ + ## 1.7 Feedback package (browser only) + - name: npm + id: '@sentry-internal/feedback' + includeNames: /^sentry-internal-feedback-\d.*\.tgz$/ ## 2. Browser & Node SDKs - name: npm diff --git a/.size-limit.js b/.size-limit.js index 36c4212adea6..599f59e07170 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -1,18 +1,25 @@ module.exports = [ // Main browser webpack builds + { + name: '@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped)', + path: 'packages/browser/build/npm/esm/index.js', + import: '{ init, Replay, BrowserTracing, Feedback }', + gzip: true, + limit: '90 KB', + }, { name: '@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped)', path: 'packages/browser/build/npm/esm/index.js', import: '{ init, Replay, BrowserTracing }', gzip: true, - limit: '90 KB', + limit: '75 KB', }, { name: '@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped)', path: 'packages/browser/build/npm/esm/index.js', import: '{ init, Replay, BrowserTracing }', gzip: true, - limit: '90 KB', + limit: '75 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); config.plugins.push( @@ -33,6 +40,13 @@ module.exports = [ gzip: true, limit: '35 KB', }, + { + name: '@sentry/browser (incl. Feedback) - Webpack (gzipped)', + path: 'packages/browser/build/npm/esm/index.js', + import: '{ init, Feedback }', + gzip: true, + limit: '50 KB', + }, { name: '@sentry/browser - Webpack (gzipped)', path: 'packages/browser/build/npm/esm/index.js', @@ -42,11 +56,17 @@ module.exports = [ }, // Browser CDN bundles (ES6) + { + name: '@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped)', + path: 'packages/browser/build/bundles/bundle.tracing.replay.feedback.min.js', + gzip: true, + limit: '75 KB', + }, { name: '@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped)', path: 'packages/browser/build/bundles/bundle.tracing.replay.min.js', gzip: true, - limit: '90 KB', + limit: '75 KB', }, { name: '@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped)', @@ -96,7 +116,7 @@ module.exports = [ path: 'packages/react/build/esm/index.js', import: '{ init, BrowserTracing, Replay }', gzip: true, - limit: '90 KB', + limit: '75 KB', }, { name: '@sentry/react - Webpack (gzipped)', @@ -126,6 +146,6 @@ module.exports = [ path: 'packages/feedback/build/npm/esm/index.js', import: '{ Feedback }', gzip: true, - limit: '35 KB', + limit: '25 KB', }, ]; diff --git a/packages/browser/package.json b/packages/browser/package.json index cecdc30dda35..28b06cafa9a1 100644 --- a/packages/browser/package.json +++ b/packages/browser/package.json @@ -23,6 +23,7 @@ "access": "public" }, "dependencies": { + "@sentry-internal/feedback": "0.0.1-alpha.17", "@sentry-internal/tracing": "7.84.0", "@sentry/core": "7.84.0", "@sentry/replay": "7.84.0", diff --git a/packages/browser/rollup.bundle.config.js b/packages/browser/rollup.bundle.config.js index 3bb87e0c5e7b..9ebe2bd8b5aa 100644 --- a/packages/browser/rollup.bundle.config.js +++ b/packages/browser/rollup.bundle.config.js @@ -29,7 +29,7 @@ targets.forEach(jsVersion => { }); if (targets.includes('es6')) { - // Replay bundles only available for es6 + // Replay/Feedback bundles only available for es6 const replayBaseBundleConfig = makeBaseBundleConfig({ bundleType: 'standalone', entrypoints: ['src/index.bundle.replay.ts'], @@ -38,6 +38,14 @@ if (targets.includes('es6')) { outputFileBase: () => 'bundles/bundle.replay', }); + const feedbackBaseBundleConfig = makeBaseBundleConfig({ + bundleType: 'standalone', + entrypoints: ['src/index.bundle.feedback.ts'], + jsVersion: 'es6', + licenseTitle: '@sentry/browser & @sentry/feedback', + outputFileBase: () => 'bundles/bundle.feedback', + }); + const tracingReplayBaseBundleConfig = makeBaseBundleConfig({ bundleType: 'standalone', entrypoints: ['src/index.bundle.tracing.replay.ts'], @@ -46,9 +54,19 @@ if (targets.includes('es6')) { outputFileBase: () => 'bundles/bundle.tracing.replay', }); + const tracingReplayFeedbackBaseBundleConfig = makeBaseBundleConfig({ + bundleType: 'standalone', + entrypoints: ['src/index.bundle.tracing.replay.feedback.ts'], + jsVersion: 'es6', + licenseTitle: '@sentry/browser & @sentry/tracing & @sentry/replay & @sentry/feedback', + outputFileBase: () => 'bundles/bundle.tracing.replay.feedback', + }); + builds.push( ...makeBundleConfigVariants(replayBaseBundleConfig), + ...makeBundleConfigVariants(feedbackBaseBundleConfig), ...makeBundleConfigVariants(tracingReplayBaseBundleConfig), + ...makeBundleConfigVariants(tracingReplayFeedbackBaseBundleConfig), ); } diff --git a/packages/browser/src/index.bundle.feedback.ts b/packages/browser/src/index.bundle.feedback.ts new file mode 100644 index 000000000000..7aab6485d254 --- /dev/null +++ b/packages/browser/src/index.bundle.feedback.ts @@ -0,0 +1,14 @@ +// This is exported so the loader does not fail when switching off Replay/Tracing +import { Feedback } from '@sentry-internal/feedback'; +import { BrowserTracing, Replay, addTracingExtensions } from '@sentry-internal/integration-shims'; + +import * as Sentry from './index.bundle.base'; + +// TODO (v8): Remove this as it was only needed for backwards compatibility +Sentry.Integrations.Replay = Replay; + +Sentry.Integrations.BrowserTracing = BrowserTracing; + +export * from './index.bundle.base'; +export { BrowserTracing, addTracingExtensions, Replay, Feedback }; +// Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.bundle.replay.ts b/packages/browser/src/index.bundle.replay.ts index d6fdaf26ceb0..0b556bc71b8e 100644 --- a/packages/browser/src/index.bundle.replay.ts +++ b/packages/browser/src/index.bundle.replay.ts @@ -1,5 +1,5 @@ // This is exported so the loader does not fail when switching off Replay/Tracing -import { BrowserTracing, addTracingExtensions } from '@sentry-internal/integration-shims'; +import { BrowserTracing, Feedback, addTracingExtensions } from '@sentry-internal/integration-shims'; import { Replay } from '@sentry/replay'; import * as Sentry from './index.bundle.base'; @@ -10,5 +10,5 @@ Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; -export { BrowserTracing, addTracingExtensions, Replay }; +export { BrowserTracing, addTracingExtensions, Replay, Feedback }; // Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.bundle.tracing.replay.feedback.ts b/packages/browser/src/index.bundle.tracing.replay.feedback.ts new file mode 100644 index 000000000000..5558703f7703 --- /dev/null +++ b/packages/browser/src/index.bundle.tracing.replay.feedback.ts @@ -0,0 +1,18 @@ +import { Feedback } from '@sentry-internal/feedback'; +import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; +import { Replay } from '@sentry/replay'; + +import * as Sentry from './index.bundle.base'; + +// TODO (v8): Remove this as it was only needed for backwards compatibility +// We want replay to be available under Sentry.Replay, to be consistent +// with the NPM package version. +Sentry.Integrations.Replay = Replay; + +Sentry.Integrations.BrowserTracing = BrowserTracing; + +// We are patching the global object with our hub extension methods +addExtensionMethods(); + +export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; +export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.tracing.replay.ts b/packages/browser/src/index.bundle.tracing.replay.ts index ee9100e53a8d..0e7aa1ec19bc 100644 --- a/packages/browser/src/index.bundle.tracing.replay.ts +++ b/packages/browser/src/index.bundle.tracing.replay.ts @@ -1,3 +1,4 @@ +import { Feedback } from '@sentry-internal/integration-shims'; import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import { Replay } from '@sentry/replay'; @@ -13,5 +14,5 @@ Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods addExtensionMethods(); -export { Replay, BrowserTracing, Span, addExtensionMethods }; +export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.tracing.ts b/packages/browser/src/index.bundle.tracing.ts index 167df17380b5..22b33962e860 100644 --- a/packages/browser/src/index.bundle.tracing.ts +++ b/packages/browser/src/index.bundle.tracing.ts @@ -1,5 +1,5 @@ // This is exported so the loader does not fail when switching off Replay -import { Replay } from '@sentry-internal/integration-shims'; +import { Feedback, Replay } from '@sentry-internal/integration-shims'; import { BrowserTracing, Span, addExtensionMethods } from '@sentry-internal/tracing'; import * as Sentry from './index.bundle.base'; @@ -14,5 +14,5 @@ Sentry.Integrations.BrowserTracing = BrowserTracing; // We are patching the global object with our hub extension methods addExtensionMethods(); -export { Replay, BrowserTracing, Span, addExtensionMethods }; +export { Feedback, Replay, BrowserTracing, Span, addExtensionMethods }; export * from './index.bundle.base'; diff --git a/packages/browser/src/index.bundle.ts b/packages/browser/src/index.bundle.ts index 4bb272b5cc49..a2541ceadda1 100644 --- a/packages/browser/src/index.bundle.ts +++ b/packages/browser/src/index.bundle.ts @@ -1,5 +1,5 @@ // This is exported so the loader does not fail when switching off Replay/Tracing -import { BrowserTracing, Replay, addTracingExtensions } from '@sentry-internal/integration-shims'; +import { BrowserTracing, Feedback, Replay, addTracingExtensions } from '@sentry-internal/integration-shims'; import * as Sentry from './index.bundle.base'; @@ -9,5 +9,5 @@ Sentry.Integrations.Replay = Replay; Sentry.Integrations.BrowserTracing = BrowserTracing; export * from './index.bundle.base'; -export { BrowserTracing, addTracingExtensions, Replay }; +export { BrowserTracing, addTracingExtensions, Replay, Feedback }; // Note: We do not export a shim for `Span` here, as that is quite complex and would blow up the bundle diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 75286d3da7df..e357c05f14f1 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -33,6 +33,8 @@ export type { ReplaySpanFrameEvent, } from '@sentry/replay'; +export { Feedback } from '@sentry-internal/feedback'; + export { BrowserTracing, defaultRequestInstrumentationOptions, diff --git a/packages/browser/test/unit/index.bundle.feedback.test.ts b/packages/browser/test/unit/index.bundle.feedback.test.ts new file mode 100644 index 000000000000..4c5f26e5f313 --- /dev/null +++ b/packages/browser/test/unit/index.bundle.feedback.test.ts @@ -0,0 +1,25 @@ +import { BrowserTracing as BrowserTracingShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; +import { Feedback } from '@sentry/browser'; + +import * as TracingReplayBundle from '../../src/index.bundle.feedback'; + +describe('index.bundle.feedback', () => { + it('has correct exports', () => { + Object.keys(TracingReplayBundle.Integrations).forEach(key => { + // Skip BrowserTracing because it doesn't have a static id field. + if (key === 'BrowserTracing') { + return; + } + + expect((TracingReplayBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); + }); + + expect(TracingReplayBundle.Integrations.Replay).toBe(ReplayShim); + expect(TracingReplayBundle.Replay).toBe(ReplayShim); + + expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); + expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracingShim); + + expect(TracingReplayBundle.Feedback).toBe(Feedback); + }); +}); diff --git a/packages/browser/test/unit/index.bundle.replay.test.ts b/packages/browser/test/unit/index.bundle.replay.test.ts index 760232ec57cd..b14eb70f6330 100644 --- a/packages/browser/test/unit/index.bundle.replay.test.ts +++ b/packages/browser/test/unit/index.bundle.replay.test.ts @@ -1,4 +1,4 @@ -import { BrowserTracing as BrowserTracingShim } from '@sentry-internal/integration-shims'; +import { BrowserTracing as BrowserTracingShim, Feedback as FeedbackShim } from '@sentry-internal/integration-shims'; import { Replay } from '@sentry/browser'; import * as TracingReplayBundle from '../../src/index.bundle.replay'; @@ -19,5 +19,7 @@ describe('index.bundle.replay', () => { expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracingShim); + + expect(TracingReplayBundle.Feedback).toBe(FeedbackShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.test.ts b/packages/browser/test/unit/index.bundle.test.ts index 7bb866dabaad..f61987732858 100644 --- a/packages/browser/test/unit/index.bundle.test.ts +++ b/packages/browser/test/unit/index.bundle.test.ts @@ -1,4 +1,8 @@ -import { BrowserTracing as BrowserTracingShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; +import { + BrowserTracing as BrowserTracingShim, + Feedback as FeedbackShim, + Replay as ReplayShim, +} from '@sentry-internal/integration-shims'; import * as TracingBundle from '../../src/index.bundle'; @@ -18,5 +22,7 @@ describe('index.bundle', () => { expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracingShim); expect(TracingBundle.BrowserTracing).toBe(BrowserTracingShim); + + expect(TracingBundle.Feedback).toBe(FeedbackShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts b/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts new file mode 100644 index 000000000000..0bd50453da43 --- /dev/null +++ b/packages/browser/test/unit/index.bundle.tracing.replay.feedback.test.ts @@ -0,0 +1,25 @@ +import { BrowserTracing } from '@sentry-internal/tracing'; +import { Feedback, Replay } from '@sentry/browser'; + +import * as TracingReplayFeedbackBundle from '../../src/index.bundle.tracing.replay.feedback'; + +describe('index.bundle.tracing.replay.feedback', () => { + it('has correct exports', () => { + Object.keys(TracingReplayFeedbackBundle.Integrations).forEach(key => { + // Skip BrowserTracing because it doesn't have a static id field. + if (key === 'BrowserTracing') { + return; + } + + expect((TracingReplayFeedbackBundle.Integrations[key] as any).id).toStrictEqual(expect.any(String)); + }); + + expect(TracingReplayFeedbackBundle.Integrations.Replay).toBe(Replay); + expect(TracingReplayFeedbackBundle.Replay).toBe(Replay); + + expect(TracingReplayFeedbackBundle.Integrations.BrowserTracing).toBe(BrowserTracing); + expect(TracingReplayFeedbackBundle.BrowserTracing).toBe(BrowserTracing); + + expect(TracingReplayFeedbackBundle.Feedback).toBe(Feedback); + }); +}); diff --git a/packages/browser/test/unit/index.bundle.tracing.replay.test.ts b/packages/browser/test/unit/index.bundle.tracing.replay.test.ts index ac951e24445d..4c8af2130a3b 100644 --- a/packages/browser/test/unit/index.bundle.tracing.replay.test.ts +++ b/packages/browser/test/unit/index.bundle.tracing.replay.test.ts @@ -1,3 +1,4 @@ +import { Feedback as FeedbackShim } from '@sentry-internal/integration-shims'; import { BrowserTracing } from '@sentry-internal/tracing'; import { Replay } from '@sentry/browser'; @@ -19,5 +20,7 @@ describe('index.bundle.tracing.replay', () => { expect(TracingReplayBundle.Integrations.BrowserTracing).toBe(BrowserTracing); expect(TracingReplayBundle.BrowserTracing).toBe(BrowserTracing); + + expect(TracingReplayBundle.Feedback).toBe(FeedbackShim); }); }); diff --git a/packages/browser/test/unit/index.bundle.tracing.test.ts b/packages/browser/test/unit/index.bundle.tracing.test.ts index 1f92a02858ac..b439dece8c6f 100644 --- a/packages/browser/test/unit/index.bundle.tracing.test.ts +++ b/packages/browser/test/unit/index.bundle.tracing.test.ts @@ -1,4 +1,4 @@ -import { Replay as ReplayShim } from '@sentry-internal/integration-shims'; +import { Feedback as FeedbackShim, Replay as ReplayShim } from '@sentry-internal/integration-shims'; import { BrowserTracing } from '@sentry-internal/tracing'; import * as TracingBundle from '../../src/index.bundle.tracing'; @@ -19,5 +19,7 @@ describe('index.bundle.tracing', () => { expect(TracingBundle.Integrations.BrowserTracing).toBe(BrowserTracing); expect(TracingBundle.BrowserTracing).toBe(BrowserTracing); + + expect(TracingBundle.Feedback).toBe(FeedbackShim); }); }); diff --git a/packages/feedback/package.json b/packages/feedback/package.json index 1233653f7994..41d562bf8c82 100644 --- a/packages/feedback/package.json +++ b/packages/feedback/package.json @@ -1,6 +1,6 @@ { "name": "@sentry-internal/feedback", - "version": "7.84.0", + "version": "0.0.1-alpha.17", "description": "Sentry SDK integration for user feedback", "repository": "git://github.com/getsentry/sentry-javascript.git", "homepage": "https://github.com/getsentry/sentry-javascript/tree/master/packages/feedback", @@ -42,7 +42,7 @@ "build:types:watch": "tsc -p tsconfig.types.json --watch", "build:tarball": "ts-node ../../scripts/prepack.ts --bundles && npm pack ./build/npm", "circularDepCheck": "madge --circular src/index.ts", - "clean": "rimraf build sentry-replay-*.tgz", + "clean": "rimraf build sentry-feedback-*.tgz", "fix": "eslint . --format stylish --fix", "lint": "eslint . --format stylish", "test": "jest", diff --git a/packages/feedback/src/integration.ts b/packages/feedback/src/integration.ts index 6b1e0fb1b2c4..639540e6eaa3 100644 --- a/packages/feedback/src/integration.ts +++ b/packages/feedback/src/integration.ts @@ -155,7 +155,7 @@ export class Feedback implements Integration { } /** - * Setup and initialize replay container + * Setup and initialize feedback container */ public setupOnce(): void { if (!isBrowser()) { diff --git a/packages/integration-shims/src/Feedback.ts b/packages/integration-shims/src/Feedback.ts new file mode 100644 index 000000000000..3f41a15a8231 --- /dev/null +++ b/packages/integration-shims/src/Feedback.ts @@ -0,0 +1,67 @@ +import type { Integration } from '@sentry/types'; + +/** + * This is a shim for the Feedback integration. + * It is needed in order for the CDN bundles to continue working when users add/remove feedback + * from it, without changing their config. This is necessary for the loader mechanism. + */ +class FeedbackShim implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'Feedback'; + + /** + * @inheritDoc + */ + public name: string; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public constructor(_options: any) { + this.name = FeedbackShim.id; + + // eslint-disable-next-line no-console + console.error('You are using new Feedback() even though this bundle does not include Feedback.'); + } + + /** jsdoc */ + public setupOnce(): void { + // noop + } + + /** jsdoc */ + public openDialog(): void { + // noop + } + + /** jsdoc */ + public closeDialog(): void { + // noop + } + + /** jsdoc */ + public attachTo(): void { + // noop + } + + /** jsdoc */ + public createWidget(): void { + // noop + } + + /** jsdoc */ + public removeWidget(): void { + // noop + } + + /** jsdoc */ + public getWidget(): void { + // noop + } + /** jsdoc */ + public remove(): void { + // noop + } +} + +export { FeedbackShim as Feedback }; diff --git a/packages/integration-shims/src/index.ts b/packages/integration-shims/src/index.ts index 1a11515053a8..410a4a31d9c8 100644 --- a/packages/integration-shims/src/index.ts +++ b/packages/integration-shims/src/index.ts @@ -1,2 +1,3 @@ +export { Feedback } from './Feedback'; export { Replay } from './Replay'; export { BrowserTracing, addTracingExtensions } from './BrowserTracing'; diff --git a/packages/tracing-internal/tsconfig.types.json b/packages/tracing-internal/tsconfig.types.json index 94190d6f0f73..775c9b91fe20 100644 --- a/packages/tracing-internal/tsconfig.types.json +++ b/packages/tracing-internal/tsconfig.types.json @@ -6,6 +6,7 @@ // level. "exclude": [ "src/index.bundle.ts", + "src/index.bundle.feedback.ts", "src/index.bundle.replay.ts" ], "compilerOptions": { diff --git a/yarn.lock b/yarn.lock index 7f5cfc15d4f9..10db5d1c22c6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5048,6 +5048,15 @@ semver "7.3.2" semver-intersect "1.4.0" +"@sentry-internal/feedback@0.0.1-alpha.17": + version "0.0.1-alpha.17" + resolved "https://registry.yarnpkg.com/@sentry-internal/feedback/-/feedback-0.0.1-alpha.17.tgz#98e7477152111efe18febc07e7d6064d32340711" + integrity sha512-PCYiA0tQU8NWOVeZf8eRxptkI5JMro+9jQT1pLNyRG9vhzYYXurepalUgeY8qbMREdaFk5p0N9DmkJHDiOW1mg== + dependencies: + "@sentry/core" "7.84.0" + "@sentry/types" "7.84.0" + "@sentry/utils" "7.84.0" + "@sentry-internal/rrdom@2.2.0": version "2.2.0" resolved "https://registry.yarnpkg.com/@sentry-internal/rrdom/-/rrdom-2.2.0.tgz#edc292e55263e1e2541a9ed282ff998aae272a0a" From 6a382a96c46015af519d94d43d34584adfe83312 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 4 Dec 2023 15:59:48 +0100 Subject: [PATCH 5/8] fix(replay): Capture JSON XHR response bodies (#9623) This is stupid, now that I figured it out. Basically, if you set `xhr.responseType = 'json'`, it will force `xhr.response` to be a POJO - which we can't parse right now. We now handle this case specifically. This also adds a new `UNPARSEABLE_BODY_TYPE` meta warning if we are not getting a body because it is not matching any of the known/parsed types. Closes https://github.com/getsentry/sentry-javascript/issues/9339 --- .../xhr/captureResponseBody/test.ts | 87 +++++++ .../src/coreHandlers/util/networkUtils.ts | 6 +- .../replay/src/coreHandlers/util/xhrUtils.ts | 59 ++++- packages/replay/src/types/request.ts | 7 +- .../unit/coreHandlers/util/fetchUtils.test.ts | 226 +++++++++--------- .../coreHandlers/util/networkUtils.test.ts | 36 +++ .../unit/coreHandlers/util/xhrUtils.test.ts | 38 +++ 7 files changed, 342 insertions(+), 117 deletions(-) create mode 100644 packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts diff --git a/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts b/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts index 46e20da391cc..12ef0b2a6068 100644 --- a/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts +++ b/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts @@ -178,6 +178,93 @@ sentryTest('captures JSON response body', async ({ getLocalTestPath, page, brows ]); }); +sentryTest('captures JSON response body when responseType=json', async ({ getLocalTestPath, page, browserName }) => { + // These are a bit flaky on non-chromium browsers + if (shouldSkipReplayTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + await page.route('**/foo', route => { + return route.fulfill({ + status: 200, + body: JSON.stringify({ res: 'this' }), + headers: { + 'Content-Length': '', + }, + }); + }); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const requestPromise = waitForErrorRequest(page); + const replayRequestPromise1 = waitForReplayRequest(page, 0); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + void page.evaluate(() => { + /* eslint-disable */ + const xhr = new XMLHttpRequest(); + + xhr.open('POST', 'http://localhost:7654/foo'); + // Setting this to json ensures that xhr.response returns a POJO + xhr.responseType = 'json'; + xhr.send(); + + xhr.addEventListener('readystatechange', function () { + if (xhr.readyState === 4) { + // @ts-expect-error Sentry is a global + setTimeout(() => Sentry.captureException('test error', 0)); + } + }); + /* eslint-enable */ + }); + + const request = await requestPromise; + const eventData = envelopeRequestParser(request); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'xhr', + type: 'http', + data: { + method: 'POST', + response_body_size: 14, + status_code: 200, + url: 'http://localhost:7654/foo', + }, + }); + + const replayReq1 = await replayRequestPromise1; + const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1); + expect(performanceSpans1.filter(span => span.op === 'resource.xhr')).toEqual([ + { + data: { + method: 'POST', + statusCode: 200, + response: { + size: 14, + headers: {}, + body: { res: 'this' }, + }, + }, + description: 'http://localhost:7654/foo', + endTimestamp: expect.any(Number), + op: 'resource.xhr', + startTimestamp: expect.any(Number), + }, + ]); +}); + sentryTest('captures non-text response body', async ({ getLocalTestPath, page, browserName }) => { // These are a bit flaky on non-chromium browsers if (shouldSkipReplayTest() || browserName !== 'chromium') { diff --git a/packages/replay/src/coreHandlers/util/networkUtils.ts b/packages/replay/src/coreHandlers/util/networkUtils.ts index 5037d5a6769c..29d9684456ca 100644 --- a/packages/replay/src/coreHandlers/util/networkUtils.ts +++ b/packages/replay/src/coreHandlers/util/networkUtils.ts @@ -75,6 +75,10 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa if (body instanceof FormData) { return [_serializeFormData(body)]; } + + if (!body) { + return [undefined]; + } } catch { DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; @@ -82,7 +86,7 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); - return [undefined]; + return [undefined, 'UNPARSEABLE_BODY_TYPE']; } /** Merge a warning into an existing network request/response. */ diff --git a/packages/replay/src/coreHandlers/util/xhrUtils.ts b/packages/replay/src/coreHandlers/util/xhrUtils.ts index fa2345dc04bd..b422d3bd1e81 100644 --- a/packages/replay/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay/src/coreHandlers/util/xhrUtils.ts @@ -61,7 +61,7 @@ export function enrichXhrBreadcrumb( const reqSize = getBodySize(input, options.textEncoder); const resSize = xhr.getResponseHeader('content-length') ? parseContentLengthHeader(xhr.getResponseHeader('content-length')) - : getBodySize(xhr.response, options.textEncoder); + : _getBodySize(xhr.response, xhr.responseType, options.textEncoder); if (reqSize !== undefined) { breadcrumb.data.request_body_size = reqSize; @@ -154,8 +154,7 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM // Try to manually parse the response body, if responseText fails try { - const response = xhr.response; - return getBodyString(response); + return _parseXhrResponse(xhr.response, xhr.responseType); } catch (e) { errors.push(e); } @@ -164,3 +163,57 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM return [undefined]; } + +/** + * Get the string representation of the XHR response. + * Based on MDN, these are the possible types of the response: + * string + * ArrayBuffer + * Blob + * Document + * POJO + * + * Exported only for tests. + */ +export function _parseXhrResponse( + body: XMLHttpRequest['response'], + responseType: XMLHttpRequest['responseType'], +): [string | undefined, NetworkMetaWarning?] { + try { + if (typeof body === 'string') { + return [body]; + } + + if (body instanceof Document) { + return [body.body.outerHTML]; + } + + if (responseType === 'json' && body && typeof body === 'object') { + return [JSON.stringify(body)]; + } + + if (!body) { + return [undefined]; + } + } catch { + DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); + return [undefined, 'BODY_PARSE_ERROR']; + } + + DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); + + return [undefined, 'UNPARSEABLE_BODY_TYPE']; +} + +function _getBodySize( + body: XMLHttpRequest['response'], + responseType: XMLHttpRequest['responseType'], + textEncoder: TextEncoder | TextEncoderInternal, +): number | undefined { + try { + const bodyStr = responseType === 'json' && body && typeof body === 'object' ? JSON.stringify(body) : body; + return getBodySize(bodyStr, textEncoder); + } catch { + return undefined; + } +} diff --git a/packages/replay/src/types/request.ts b/packages/replay/src/types/request.ts index 03067596fb36..60c25a55ce44 100644 --- a/packages/replay/src/types/request.ts +++ b/packages/replay/src/types/request.ts @@ -3,7 +3,12 @@ type JsonArray = unknown[]; export type NetworkBody = JsonObject | JsonArray | string; -export type NetworkMetaWarning = 'MAYBE_JSON_TRUNCATED' | 'TEXT_TRUNCATED' | 'URL_SKIPPED' | 'BODY_PARSE_ERROR'; +export type NetworkMetaWarning = + | 'MAYBE_JSON_TRUNCATED' + | 'TEXT_TRUNCATED' + | 'URL_SKIPPED' + | 'BODY_PARSE_ERROR' + | 'UNPARSEABLE_BODY_TYPE'; interface NetworkMeta { warnings?: NetworkMetaWarning[]; diff --git a/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts index 82a6b66eeb7d..123ec85fe887 100644 --- a/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts +++ b/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts @@ -2,136 +2,138 @@ import { TextEncoder } from 'util'; import { _getResponseInfo } from '../../../../src/coreHandlers/util/fetchUtils'; -describe('_getResponseInfo', () => { - it('works with captureDetails: false', async () => { - const res = await _getResponseInfo( - false, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - undefined, - undefined, - ); +describe('Unit | coreHandlers | util | fetchUtils', () => { + describe('_getResponseInfo', () => { + it('works with captureDetails: false', async () => { + const res = await _getResponseInfo( + false, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + undefined, + undefined, + ); - expect(res).toEqual(undefined); - }); + expect(res).toEqual(undefined); + }); - it('works with captureDetails: false & responseBodySize', async () => { - const res = await _getResponseInfo( - false, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - undefined, - 123, - ); + it('works with captureDetails: false & responseBodySize', async () => { + const res = await _getResponseInfo( + false, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + undefined, + 123, + ); - expect(res).toEqual({ - headers: {}, - size: 123, - _meta: { - warnings: ['URL_SKIPPED'], - }, + expect(res).toEqual({ + headers: {}, + size: 123, + _meta: { + warnings: ['URL_SKIPPED'], + }, + }); }); - }); - it('works with text body', async () => { - const response = { - headers: { - has: () => { - return false; - }, - get: () => { - return undefined; + it('works with text body', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - }, - clone: () => response, - text: () => Promise.resolve('text body'), - } as unknown as Response; + clone: () => response, + text: () => Promise.resolve('text body'), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - headers: {}, - size: 9, - body: 'text body', + expect(res).toEqual({ + headers: {}, + size: 9, + body: 'text body', + }); }); - }); - it('works with body that fails', async () => { - const response = { - headers: { - has: () => { - return false; + it('works with body that fails', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - get: () => { - return undefined; - }, - }, - clone: () => response, - text: () => Promise.reject('cannot read'), - } as unknown as Response; + clone: () => response, + text: () => Promise.reject('cannot read'), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - _meta: { warnings: ['BODY_PARSE_ERROR'] }, - headers: {}, - size: undefined, + expect(res).toEqual({ + _meta: { warnings: ['BODY_PARSE_ERROR'] }, + headers: {}, + size: undefined, + }); }); - }); - it('works with body that times out', async () => { - const response = { - headers: { - has: () => { - return false; - }, - get: () => { - return undefined; + it('works with body that times out', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - }, - clone: () => response, - text: () => new Promise(resolve => setTimeout(() => resolve('text body'), 1000)), - } as unknown as Response; + clone: () => response, + text: () => new Promise(resolve => setTimeout(() => resolve('text body'), 1000)), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - _meta: { warnings: ['BODY_PARSE_ERROR'] }, - headers: {}, - size: undefined, + expect(res).toEqual({ + _meta: { warnings: ['BODY_PARSE_ERROR'] }, + headers: {}, + size: undefined, + }); }); }); }); diff --git a/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts index f00acad6384d..fccbf24a0a23 100644 --- a/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts +++ b/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts @@ -4,6 +4,7 @@ import { NETWORK_BODY_MAX_SIZE } from '../../../../src/constants'; import { buildNetworkRequestOrResponse, getBodySize, + getBodyString, getFullUrl, parseContentLengthHeader, } from '../../../../src/coreHandlers/util/networkUtils'; @@ -248,4 +249,39 @@ describe('Unit | coreHandlers | util | networkUtils', () => { expect(actual).toBe(expected); }); }); + + describe('getBodyString', () => { + it('works with a string', () => { + const actual = getBodyString('abc'); + expect(actual).toEqual(['abc']); + }); + + it('works with URLSearchParams', () => { + const body = new URLSearchParams(); + body.append('name', 'Anne'); + body.append('age', '32'); + const actual = getBodyString(body); + expect(actual).toEqual(['name=Anne&age=32']); + }); + + it('works with FormData', () => { + const body = new FormData(); + body.append('name', 'Anne'); + body.append('age', '32'); + const actual = getBodyString(body); + expect(actual).toEqual(['name=Anne&age=32']); + }); + + it('works with empty data', () => { + const body = undefined; + const actual = getBodyString(body); + expect(actual).toEqual([undefined]); + }); + + it('works with other type of data', () => { + const body = {}; + const actual = getBodyString(body); + expect(actual).toEqual([undefined, 'UNPARSEABLE_BODY_TYPE']); + }); + }); }); diff --git a/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts new file mode 100644 index 000000000000..d39b473f317f --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts @@ -0,0 +1,38 @@ +import { _parseXhrResponse } from '../../../../src/coreHandlers/util/xhrUtils'; + +describe('Unit | coreHandlers | util | xhrUtils', () => { + describe('_parseXhrResponse', () => { + it('works with a string', () => { + const actual = _parseXhrResponse('abc', ''); + expect(actual).toEqual(['abc']); + }); + + it('works with a Document', () => { + const body = document.implementation.createHTMLDocument(); + const bodyEl = document.createElement('body'); + bodyEl.innerHTML = '
abc
'; + body.body = bodyEl; + + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual(['
abc
']); + }); + + it('works with empty data', () => { + const body = undefined; + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual([undefined]); + }); + + it('works with other type of data', () => { + const body = {}; + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual([undefined, 'UNPARSEABLE_BODY_TYPE']); + }); + + it('works with JSON data', () => { + const body = {}; + const actual = _parseXhrResponse(body, 'json'); + expect(actual).toEqual(['{}']); + }); + }); +}); From abbc57fdc60b7467dffca30c108c36ba14e2c3d2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 4 Dec 2023 16:00:10 +0100 Subject: [PATCH 6/8] feat(core): Add `addEventProcessor` method (#9554) And deprecate `addGlobalEventProcessor()` and `getGlobalEventProcessors()`. In v8, all event processors will be on the client only, streamlining this a bit and preventing global "pollution". Closes https://github.com/getsentry/sentry-javascript/issues/9082 --- MIGRATION.md | 8 +++++++ packages/astro/src/index.server.ts | 2 ++ packages/browser/src/exports.ts | 2 ++ packages/browser/src/integrations/dedupe.ts | 2 +- packages/bun/src/index.ts | 2 ++ packages/core/src/baseclient.ts | 15 ++++++++++++ packages/core/src/eventProcessors.ts | 4 +++- packages/core/src/index.ts | 7 ++++-- packages/core/src/integration.ts | 1 + .../core/src/integrations/inboundfilters.ts | 2 +- packages/core/src/scope.ts | 7 +++++- packages/core/src/utils/prepareEvent.ts | 10 +++++++- packages/deno/src/index.ts | 2 ++ packages/e2e-tests/README.md | 2 +- .../create-next-app/sentry.client.config.ts | 2 +- .../create-next-app/sentry.server.config.ts | 2 +- .../create-remix-app-v2/app/entry.client.tsx | 2 +- .../create-remix-app/app/entry.client.tsx | 2 +- .../node-express-app/src/app.ts | 2 +- .../react-create-hash-router/src/index.tsx | 2 +- .../react-router-6-use-routes/src/index.tsx | 2 +- .../src/index.tsx | 2 +- .../standard-frontend-react/src/index.tsx | 2 +- packages/ember/tests/test-helper.ts | 2 +- packages/hub/src/index.ts | 3 ++- packages/hub/test/scope.test.ts | 1 + packages/integrations/src/contextlines.ts | 2 +- packages/integrations/src/dedupe.ts | 2 +- packages/integrations/src/extraerrordata.ts | 2 +- packages/integrations/src/rewriteframes.ts | 2 +- packages/integrations/src/sessiontiming.ts | 2 +- packages/integrations/src/transaction.ts | 2 +- packages/node-experimental/src/index.ts | 2 ++ packages/node/src/anr/index.ts | 4 ++-- packages/node/src/index.ts | 2 ++ .../opentelemetry-node/src/spanprocessor.ts | 4 ++-- .../test/spanprocessor.test.ts | 17 ++++++++++++- packages/react/src/redux.ts | 4 ++-- packages/react/test/redux.test.ts | 24 +++++++++---------- packages/remix/src/index.server.ts | 2 ++ .../src/coreHandlers/handleGlobalEvent.ts | 2 +- .../replay/src/util/addGlobalListeners.ts | 4 ++-- packages/serverless/src/index.ts | 2 ++ packages/svelte/src/sdk.ts | 4 ++-- packages/svelte/test/sdk.test.ts | 8 +++---- packages/sveltekit/src/server/index.ts | 2 ++ packages/vercel-edge/src/index.ts | 2 ++ packages/vue/src/integration.ts | 2 +- packages/wasm/src/index.ts | 2 +- 49 files changed, 134 insertions(+), 55 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 33a6c0f6ea09..78f5f16d3002 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -8,6 +8,14 @@ npx @sentry/migr8@latest This will let you select which updates to run, and automatically update your code. Make sure to still review all code changes! +## Deprecate `addGlobalEventProcessor` in favor of `addEventProcessor` + +Instead of using `addGlobalEventProcessor`, you should use `addEventProcessor` which does not add the event processor globally, but to the current client. + +For the vast majority of cases, the behavior of these should be the same. Only in the case where you have multiple clients will this differ - but you'll likely want to add event processors per-client then anyhow, not globally. + +In v8, we will remove the global event processors overall, as that allows us to avoid keeping global state that is not necessary. + ## Deprecate `extractTraceParentData` export from `@sentry/core` & downstream packages Instead, import this directly from `@sentry/utils`. diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 6bc9db94d4b1..a0c18c0cb6e7 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -8,7 +8,9 @@ import { handleRequest } from './server/middleware'; // Hence, we export everything from the Node SDK explicitly: export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, captureException, captureEvent, diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index 18b9320eafbd..0804dbe49ac0 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -21,7 +21,9 @@ export type { BrowserOptions } from './client'; export type { ReportDialogOptions } from './helpers'; export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, addIntegration, captureException, diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts index 74d22d5a462c..2d4b51f58767 100644 --- a/packages/browser/src/integrations/dedupe.ts +++ b/packages/browser/src/integrations/dedupe.ts @@ -25,7 +25,7 @@ export class Dedupe implements Integration { } /** @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/bun/src/index.ts b/packages/bun/src/index.ts index baae8ff4fa32..302d36fb0b62 100644 --- a/packages/bun/src/index.ts +++ b/packages/bun/src/index.ts @@ -24,7 +24,9 @@ export type { TransactionNamingScheme } from '@sentry/node'; export type { BunOptions } from './types'; export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, addIntegration, captureException, diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3d97e2b056a6..0ea80e229c58 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -46,6 +46,7 @@ import { import { getEnvelopeEndpointWithUrlEncodedAuth } from './api'; import { DEBUG_BUILD } from './debug-build'; import { createEventEnvelope, createSessionEnvelope } from './envelope'; +import { getCurrentHub } from './hub'; import type { IntegrationIndex } from './integration'; import { setupIntegration, setupIntegrations } from './integration'; import type { Scope } from './scope'; @@ -834,3 +835,17 @@ function isErrorEvent(event: Event): event is ErrorEvent { function isTransactionEvent(event: Event): event is TransactionEvent { return event.type === 'transaction'; } + +/** + * Add an event processor to the current client. + * This event processor will run for all events processed by this client. + */ +export function addEventProcessor(callback: EventProcessor): void { + const client = getCurrentHub().getClient(); + + if (!client || !client.addEventProcessor) { + return; + } + + client.addEventProcessor(callback); +} diff --git a/packages/core/src/eventProcessors.ts b/packages/core/src/eventProcessors.ts index 2300afa7d8c0..0f5e0f0202fa 100644 --- a/packages/core/src/eventProcessors.ts +++ b/packages/core/src/eventProcessors.ts @@ -5,6 +5,7 @@ import { DEBUG_BUILD } from './debug-build'; /** * Returns the global event processors. + * @deprecated Global event processors will be removed in v8. */ export function getGlobalEventProcessors(): EventProcessor[] { return getGlobalSingleton('globalEventProcessors', () => []); @@ -12,9 +13,10 @@ export function getGlobalEventProcessors(): EventProcessor[] { /** * Add a EventProcessor to be kept globally. - * @param callback EventProcessor to add + * @deprecated Use `addEventProcessor` instead. Global event processors will be removed in v8. */ export function addGlobalEventProcessor(callback: EventProcessor): void { + // eslint-disable-next-line deprecation/deprecation getGlobalEventProcessors().push(callback); } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 0a1daae81450..07871633f16c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -41,9 +41,12 @@ export { export { makeSession, closeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; export { Scope } from './scope'; -export { addGlobalEventProcessor } from './eventProcessors'; +export { + // eslint-disable-next-line deprecation/deprecation + addGlobalEventProcessor, +} from './eventProcessors'; export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from './api'; -export { BaseClient } from './baseclient'; +export { BaseClient, addEventProcessor } from './baseclient'; export { ServerRuntimeClient } from './server-runtime-client'; export { initAndBind } from './sdk'; export { createTransport } from './transports/base'; diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 2f21339a0842..7be22a316e53 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -105,6 +105,7 @@ export function setupIntegration(client: Client, integration: Integration, integ // `setupOnce` is only called the first time if (installedIntegrations.indexOf(integration.name) === -1) { + // eslint-disable-next-line deprecation/deprecation integration.setupOnce(addGlobalEventProcessor, getCurrentHub); installedIntegrations.push(integration.name); } diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index c1fcadb1076b..9d348a4b4d23 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -50,7 +50,7 @@ export class InboundFilters implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 48e1701ab3cb..266087dd6090 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -523,7 +523,12 @@ export class Scope implements ScopeInterface { // TODO (v8): Update this order to be: Global > Client > Scope return notifyEventProcessors( - [...(additionalEventProcessors || []), ...getGlobalEventProcessors(), ...this._eventProcessors], + [ + ...(additionalEventProcessors || []), + // eslint-disable-next-line deprecation/deprecation + ...getGlobalEventProcessors(), + ...this._eventProcessors, + ], event, hint, ); diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index cf263e9366ee..6e25350202b4 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -110,7 +110,15 @@ export function prepareEvent( } else { // Apply client & global event processors even if there is no scope // TODO (v8): Update the order to be Global > Client - result = notifyEventProcessors([...clientEventProcessors, ...getGlobalEventProcessors()], prepared, hint); + result = notifyEventProcessors( + [ + ...clientEventProcessors, + // eslint-disable-next-line deprecation/deprecation + ...getGlobalEventProcessors(), + ], + prepared, + hint, + ); } return result.then(evt => { diff --git a/packages/deno/src/index.ts b/packages/deno/src/index.ts index 093ed25380de..846ad000bcaf 100644 --- a/packages/deno/src/index.ts +++ b/packages/deno/src/index.ts @@ -23,7 +23,9 @@ export type { AddRequestDataToEventOptions } from '@sentry/utils'; export type { DenoOptions } from './types'; export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, captureException, captureEvent, diff --git a/packages/e2e-tests/README.md b/packages/e2e-tests/README.md index 541257eb2410..76fa6370d154 100644 --- a/packages/e2e-tests/README.md +++ b/packages/e2e-tests/README.md @@ -107,7 +107,7 @@ A standardized frontend test application has the following features: be done with an event processor: ```ts - Sentry.addGlobalEventProcessor(event => { + Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts b/packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts index 760843e01739..1318bf32e5ac 100644 --- a/packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts +++ b/packages/e2e-tests/test-applications/create-next-app/sentry.client.config.ts @@ -16,7 +16,7 @@ Sentry.init({ // that it will also get attached to your source maps }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/create-next-app/sentry.server.config.ts b/packages/e2e-tests/test-applications/create-next-app/sentry.server.config.ts index 8bd8ad21f0a6..4f8004c021b3 100644 --- a/packages/e2e-tests/test-applications/create-next-app/sentry.server.config.ts +++ b/packages/e2e-tests/test-applications/create-next-app/sentry.server.config.ts @@ -23,7 +23,7 @@ Sentry.init({ integrations: [new Sentry.Integrations.LocalVariables()], }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { global.transactionIds = global.transactionIds || []; if (event.type === 'transaction') { diff --git a/packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx b/packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx index c0134c08c451..15cb6513a917 100644 --- a/packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx +++ b/packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.client.tsx @@ -24,7 +24,7 @@ Sentry.init({ replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur. }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/create-remix-app/app/entry.client.tsx b/packages/e2e-tests/test-applications/create-remix-app/app/entry.client.tsx index c0134c08c451..15cb6513a917 100644 --- a/packages/e2e-tests/test-applications/create-remix-app/app/entry.client.tsx +++ b/packages/e2e-tests/test-applications/create-remix-app/app/entry.client.tsx @@ -24,7 +24,7 @@ Sentry.init({ replaysOnErrorSampleRate: 1.0, // If you're not already sampling the entire session, change the sample rate to 100% when sampling sessions where errors occur. }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/node-express-app/src/app.ts b/packages/e2e-tests/test-applications/node-express-app/src/app.ts index b0321e535bfa..e9de96631259 100644 --- a/packages/e2e-tests/test-applications/node-express-app/src/app.ts +++ b/packages/e2e-tests/test-applications/node-express-app/src/app.ts @@ -89,7 +89,7 @@ app.listen(port, () => { console.log(`Example app listening on port ${port}`); }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { global.transactionIds = global.transactionIds || []; if (event.type === 'transaction') { diff --git a/packages/e2e-tests/test-applications/react-create-hash-router/src/index.tsx b/packages/e2e-tests/test-applications/react-create-hash-router/src/index.tsx index e23cb81d2c38..35db65cf3160 100644 --- a/packages/e2e-tests/test-applications/react-create-hash-router/src/index.tsx +++ b/packages/e2e-tests/test-applications/react-create-hash-router/src/index.tsx @@ -47,7 +47,7 @@ Object.defineProperty(window, 'sentryReplayId', { }, }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/react-router-6-use-routes/src/index.tsx b/packages/e2e-tests/test-applications/react-router-6-use-routes/src/index.tsx index 4c4fcc5a7d4f..2f8587db9859 100644 --- a/packages/e2e-tests/test-applications/react-router-6-use-routes/src/index.tsx +++ b/packages/e2e-tests/test-applications/react-router-6-use-routes/src/index.tsx @@ -45,7 +45,7 @@ Object.defineProperty(window, 'sentryReplayId', { }, }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/standard-frontend-react-tracing-import/src/index.tsx b/packages/e2e-tests/test-applications/standard-frontend-react-tracing-import/src/index.tsx index 0a0babe9df84..9a26b58079e1 100644 --- a/packages/e2e-tests/test-applications/standard-frontend-react-tracing-import/src/index.tsx +++ b/packages/e2e-tests/test-applications/standard-frontend-react-tracing-import/src/index.tsx @@ -34,7 +34,7 @@ Sentry.init({ release: 'e2e-test', }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/e2e-tests/test-applications/standard-frontend-react/src/index.tsx b/packages/e2e-tests/test-applications/standard-frontend-react/src/index.tsx index dbf5717beee3..660c3827f583 100644 --- a/packages/e2e-tests/test-applications/standard-frontend-react/src/index.tsx +++ b/packages/e2e-tests/test-applications/standard-frontend-react/src/index.tsx @@ -46,7 +46,7 @@ Object.defineProperty(window, 'sentryReplayId', { }, }); -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if ( event.type === 'transaction' && (event.contexts?.trace?.op === 'pageload' || event.contexts?.trace?.op === 'navigation') diff --git a/packages/ember/tests/test-helper.ts b/packages/ember/tests/test-helper.ts index 9c04e37d68b5..e01f3ab50eba 100644 --- a/packages/ember/tests/test-helper.ts +++ b/packages/ember/tests/test-helper.ts @@ -13,7 +13,7 @@ declare global { } } -Sentry.addGlobalEventProcessor(event => { +Sentry.addEventProcessor(event => { if (isTesting()) { if (!window._sentryTestEvents) { window._sentryTestEvents = []; diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index ed804ac191a2..7797b1d0e7d5 100644 --- a/packages/hub/src/index.ts +++ b/packages/hub/src/index.ts @@ -46,7 +46,8 @@ export const getCurrentHub = getCurrentHubCore; /** * @deprecated This export has moved to @sentry/core. The @sentry/hub package will be removed in v8. */ -export const addGlobalEventProcessor = addGlobalEventProcessorCore; + +export const addGlobalEventProcessor = addGlobalEventProcessorCore; // eslint-disable-line deprecation/deprecation /** * @deprecated This export has moved to @sentry/core. The @sentry/hub package will be removed in v8. diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index c64040f2b6ca..a039a839dcc6 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -644,6 +644,7 @@ describe('Scope', () => { const localScope = new Scope(); localScope.setExtra('a', 'b'); + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor((processedEvent: Event) => { processedEvent.dist = '1'; return processedEvent; diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts index a83df95077e4..d716ca2fbb5f 100644 --- a/packages/integrations/src/contextlines.ts +++ b/packages/integrations/src/contextlines.ts @@ -44,7 +44,7 @@ export class ContextLines implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts index 06ff147a76cb..464758d20dfc 100644 --- a/packages/integrations/src/dedupe.ts +++ b/packages/integrations/src/dedupe.ts @@ -25,7 +25,7 @@ export class Dedupe implements Integration { } /** @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/integrations/src/extraerrordata.ts b/packages/integrations/src/extraerrordata.ts index a5222fb84833..1c1b46e58c22 100644 --- a/packages/integrations/src/extraerrordata.ts +++ b/packages/integrations/src/extraerrordata.ts @@ -38,7 +38,7 @@ export class ExtraErrorData implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/integrations/src/rewriteframes.ts b/packages/integrations/src/rewriteframes.ts index 67f146e650bd..bcb6eeca56f2 100644 --- a/packages/integrations/src/rewriteframes.ts +++ b/packages/integrations/src/rewriteframes.ts @@ -43,7 +43,7 @@ export class RewriteFrames implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/integrations/src/sessiontiming.ts b/packages/integrations/src/sessiontiming.ts index 016d71f336e3..6b85b6ff9d56 100644 --- a/packages/integrations/src/sessiontiming.ts +++ b/packages/integrations/src/sessiontiming.ts @@ -23,7 +23,7 @@ export class SessionTiming implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/integrations/src/transaction.ts b/packages/integrations/src/transaction.ts index bc36857f6538..28bb90b0f91b 100644 --- a/packages/integrations/src/transaction.ts +++ b/packages/integrations/src/transaction.ts @@ -19,7 +19,7 @@ export class Transaction implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { // noop } diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index e14572b14b6a..e80e9ff4dc0b 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -24,7 +24,9 @@ export { extractRequestData, deepReadDirSync, getModuleFromFilename, + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, captureException, captureEvent, diff --git a/packages/node/src/anr/index.ts b/packages/node/src/anr/index.ts index b7a85273f4db..84a3ebe52920 100644 --- a/packages/node/src/anr/index.ts +++ b/packages/node/src/anr/index.ts @@ -3,7 +3,7 @@ import { getClient, makeSession, updateSession } from '@sentry/core'; import type { Event, Session, StackFrame } from '@sentry/types'; import { logger, watchdogTimer } from '@sentry/utils'; -import { addGlobalEventProcessor, captureEvent, flush, getCurrentHub } from '..'; +import { addEventProcessor, captureEvent, flush, getCurrentHub } from '..'; import { captureStackTrace } from './debugger'; const DEFAULT_INTERVAL = 50; @@ -191,7 +191,7 @@ function handleChildProcess(options: Options): void { }); } - addGlobalEventProcessor(event => { + addEventProcessor(event => { // Strip sdkProcessingMetadata from all child process events to remove trace info delete event.sdkProcessingMetadata; event.tags = { diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 0a361f02f5d7..28fa7e6e603d 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -23,7 +23,9 @@ export type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sen export type { NodeOptions } from './types'; export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, addIntegration, captureException, diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts index ed8a2e84d5f4..772bd861cc2a 100644 --- a/packages/opentelemetry-node/src/spanprocessor.ts +++ b/packages/opentelemetry-node/src/spanprocessor.ts @@ -2,7 +2,7 @@ import type { Context } from '@opentelemetry/api'; import { SpanKind, context, trace } from '@opentelemetry/api'; import { suppressTracing } from '@opentelemetry/core'; import type { Span as OtelSpan, SpanProcessor as OtelSpanProcessor } from '@opentelemetry/sdk-trace-base'; -import { Transaction, addGlobalEventProcessor, addTracingExtensions, getClient, getCurrentHub } from '@sentry/core'; +import { Transaction, addEventProcessor, addTracingExtensions, getClient, getCurrentHub } from '@sentry/core'; import type { DynamicSamplingContext, Span as SentrySpan, TraceparentData, TransactionContext } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -22,7 +22,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor { public constructor() { addTracingExtensions(); - addGlobalEventProcessor(event => { + addEventProcessor(event => { const otelSpan = trace && trace.getActiveSpan && (trace.getActiveSpan() as OtelSpan | undefined); if (!otelSpan) { return event; diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 992a05dda0c6..5361055755ff 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -863,10 +863,15 @@ describe('SentrySpanProcessor', () => { }); }); - it('associates an error to a transaction', () => { + it('associates an error to a transaction', async () => { let sentryEvent: any; let otelSpan: any; + // Clear provider & setup a new one + // As we need a custom client + await provider.forceFlush(); + await provider.shutdown(); + client = new NodeClient({ ...DEFAULT_NODE_CLIENT_OPTIONS, beforeSend: event => { @@ -877,6 +882,16 @@ describe('SentrySpanProcessor', () => { hub = new Hub(client); makeMain(hub); + // Need to register the spanprocessor again + spanProcessor = new SentrySpanProcessor(); + provider = new NodeTracerProvider({ + resource: new Resource({ + [SemanticResourceAttributes.SERVICE_NAME]: 'test-service', + }), + }); + provider.addSpanProcessor(spanProcessor); + provider.register(); + const tracer = provider.getTracer('default'); tracer.startActiveSpan('GET /users', parentOtelSpan => { diff --git a/packages/react/src/redux.ts b/packages/react/src/redux.ts index 360e6d7b1382..b81622f8797c 100644 --- a/packages/react/src/redux.ts +++ b/packages/react/src/redux.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { addGlobalEventProcessor, configureScope, getClient } from '@sentry/browser'; +import { addEventProcessor, configureScope, getClient } from '@sentry/browser'; import type { Scope } from '@sentry/types'; import { addNonEnumerableProperty } from '@sentry/utils'; @@ -97,7 +97,7 @@ function createReduxEnhancer(enhancerOptions?: Partial): return (next: StoreEnhancerStoreCreator): StoreEnhancerStoreCreator => (reducer: Reducer, initialState?: PreloadedState) => { options.attachReduxState && - addGlobalEventProcessor((event, hint) => { + addEventProcessor((event, hint) => { try { // @ts-expect-error try catch to reduce bundle size if (event.type === undefined && event.contexts.state.state.type === 'redux') { diff --git a/packages/react/test/redux.test.ts b/packages/react/test/redux.test.ts index f8260a1dc278..61b908a1d5fe 100644 --- a/packages/react/test/redux.test.ts +++ b/packages/react/test/redux.test.ts @@ -14,15 +14,15 @@ jest.mock('@sentry/browser', () => ({ addBreadcrumb: mockAddBreadcrumb, setContext: mockSetContext, }), - addGlobalEventProcessor: jest.fn(), + addEventProcessor: jest.fn(), })); -const mockAddGlobalEventProcessor = Sentry.addGlobalEventProcessor as jest.Mock; +const mockAddEventProcessor = Sentry.addEventProcessor as jest.Mock; afterEach(() => { mockAddBreadcrumb.mockReset(); mockSetContext.mockReset(); - mockAddGlobalEventProcessor.mockReset(); + mockAddEventProcessor.mockReset(); }); describe('createReduxEnhancer', () => { @@ -258,9 +258,9 @@ describe('createReduxEnhancer', () => { Redux.createStore((state = initialState) => state, enhancer); - expect(mockAddGlobalEventProcessor).toHaveBeenCalledTimes(1); + expect(mockAddEventProcessor).toHaveBeenCalledTimes(1); - const callbackFunction = mockAddGlobalEventProcessor.mock.calls[0][0]; + const callbackFunction = mockAddEventProcessor.mock.calls[0][0]; const mockEvent = { contexts: { @@ -307,7 +307,7 @@ describe('createReduxEnhancer', () => { Redux.createStore((state = initialState) => state, enhancer); - expect(mockAddGlobalEventProcessor).toHaveBeenCalledTimes(0); + expect(mockAddEventProcessor).toHaveBeenCalledTimes(0); }); it('does not attach when state.type is not redux', () => { @@ -319,9 +319,9 @@ describe('createReduxEnhancer', () => { Redux.createStore((state = initialState) => state, enhancer); - expect(mockAddGlobalEventProcessor).toHaveBeenCalledTimes(1); + expect(mockAddEventProcessor).toHaveBeenCalledTimes(1); - const callbackFunction = mockAddGlobalEventProcessor.mock.calls[0][0]; + const callbackFunction = mockAddEventProcessor.mock.calls[0][0]; const mockEvent = { contexts: { @@ -354,9 +354,9 @@ describe('createReduxEnhancer', () => { Redux.createStore((state = initialState) => state, enhancer); - expect(mockAddGlobalEventProcessor).toHaveBeenCalledTimes(1); + expect(mockAddEventProcessor).toHaveBeenCalledTimes(1); - const callbackFunction = mockAddGlobalEventProcessor.mock.calls[0][0]; + const callbackFunction = mockAddEventProcessor.mock.calls[0][0]; const mockEvent = { contexts: { @@ -386,9 +386,9 @@ describe('createReduxEnhancer', () => { Redux.createStore((state = initialState) => state, enhancer); - expect(mockAddGlobalEventProcessor).toHaveBeenCalledTimes(1); + expect(mockAddEventProcessor).toHaveBeenCalledTimes(1); - const callbackFunction = mockAddGlobalEventProcessor.mock.calls[0][0]; + const callbackFunction = mockAddEventProcessor.mock.calls[0][0]; const mockEvent = { type: 'not_redux', diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index a68e417924e9..8a85ed16913f 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -10,7 +10,9 @@ import type { RemixOptions } from './utils/remixOptions'; // We need to explicitly export @sentry/node as they end up under `default` in ESM builds // See: https://github.com/getsentry/sentry-javascript/issues/8474 export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, captureCheckIn, withMonitor, diff --git a/packages/replay/src/coreHandlers/handleGlobalEvent.ts b/packages/replay/src/coreHandlers/handleGlobalEvent.ts index 16cc9d9f8750..3dbe1498f987 100644 --- a/packages/replay/src/coreHandlers/handleGlobalEvent.ts +++ b/packages/replay/src/coreHandlers/handleGlobalEvent.ts @@ -10,7 +10,7 @@ import { addFeedbackBreadcrumb } from './util/addFeedbackBreadcrumb'; import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent'; /** - * Returns a listener to be added to `addGlobalEventProcessor(listener)`. + * Returns a listener to be added to `addEventProcessor(listener)`. */ export function handleGlobalEventListener( replay: ReplayContainer, diff --git a/packages/replay/src/util/addGlobalListeners.ts b/packages/replay/src/util/addGlobalListeners.ts index fb116438003e..828731bf814f 100644 --- a/packages/replay/src/util/addGlobalListeners.ts +++ b/packages/replay/src/util/addGlobalListeners.ts @@ -1,5 +1,5 @@ import type { BaseClient } from '@sentry/core'; -import { addGlobalEventProcessor, getClient, getCurrentHub } from '@sentry/core'; +import { addEventProcessor, getClient, getCurrentHub } from '@sentry/core'; import type { Client, DynamicSamplingContext } from '@sentry/types'; import { addClickKeypressInstrumentationHandler, addHistoryInstrumentationHandler } from '@sentry/utils'; @@ -30,7 +30,7 @@ export function addGlobalListeners(replay: ReplayContainer): void { if (client && client.addEventProcessor) { client.addEventProcessor(eventProcessor); } else { - addGlobalEventProcessor(eventProcessor); + addEventProcessor(eventProcessor); } // If a custom client has no hooks yet, we continue to use the "old" implementation diff --git a/packages/serverless/src/index.ts b/packages/serverless/src/index.ts index 403e989e6a96..2ac8f77274a5 100644 --- a/packages/serverless/src/index.ts +++ b/packages/serverless/src/index.ts @@ -14,7 +14,9 @@ export { SDK_VERSION, Scope, addBreadcrumb, + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addIntegration, autoDiscoverNodePerformanceMonitoringIntegrations, captureEvent, diff --git a/packages/svelte/src/sdk.ts b/packages/svelte/src/sdk.ts index 6caef4817af9..fb6cb575cdcd 100644 --- a/packages/svelte/src/sdk.ts +++ b/packages/svelte/src/sdk.ts @@ -1,5 +1,5 @@ import type { BrowserOptions } from '@sentry/browser'; -import { SDK_VERSION, addGlobalEventProcessor, init as browserInit } from '@sentry/browser'; +import { SDK_VERSION, addEventProcessor, init as browserInit } from '@sentry/browser'; import type { EventProcessor, SdkMetadata } from '@sentry/types'; import { getDomElement } from '@sentry/utils'; /** @@ -50,7 +50,7 @@ export function detectAndReportSvelteKit(): void { }; svelteKitProcessor.id = 'svelteKitProcessor'; - addGlobalEventProcessor(svelteKitProcessor); + addEventProcessor(svelteKitProcessor); } /** diff --git a/packages/svelte/test/sdk.test.ts b/packages/svelte/test/sdk.test.ts index ea3e3e383eba..6144e065c643 100644 --- a/packages/svelte/test/sdk.test.ts +++ b/packages/svelte/test/sdk.test.ts @@ -7,8 +7,8 @@ import { detectAndReportSvelteKit, init as svelteInit, isSvelteKitApp } from '.. let passedEventProcessor: EventProcessor | undefined; const browserInit = jest.spyOn(SentryBrowser, 'init'); -const addGlobalEventProcessor = jest - .spyOn(SentryBrowser, 'addGlobalEventProcessor') +const addEventProcessor = jest + .spyOn(SentryBrowser, 'addEventProcessor') .mockImplementation((eventProcessor: EventProcessor) => { passedEventProcessor = eventProcessor; return () => {}; @@ -79,10 +79,10 @@ describe('detectAndReportSvelteKit()', () => { passedEventProcessor = undefined; }); - it('registers a global event processor', async () => { + it('registers an event processor', async () => { detectAndReportSvelteKit(); - expect(addGlobalEventProcessor).toHaveBeenCalledTimes(1); + expect(addEventProcessor).toHaveBeenCalledTimes(1); expect(passedEventProcessor?.id).toEqual('svelteKitProcessor'); }); diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts index efaae15cbf02..f584759da054 100644 --- a/packages/sveltekit/src/server/index.ts +++ b/packages/sveltekit/src/server/index.ts @@ -4,7 +4,9 @@ // on the top - level namespace. // Hence, we export everything from the Node SDK explicitly: export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, addIntegration, captureException, diff --git a/packages/vercel-edge/src/index.ts b/packages/vercel-edge/src/index.ts index bd1ce1ba6526..3a13f97beb03 100644 --- a/packages/vercel-edge/src/index.ts +++ b/packages/vercel-edge/src/index.ts @@ -23,7 +23,9 @@ export type { AddRequestDataToEventOptions } from '@sentry/utils'; export type { VercelEdgeOptions } from './types'; export { + // eslint-disable-next-line deprecation/deprecation addGlobalEventProcessor, + addEventProcessor, addBreadcrumb, addIntegration, captureException, diff --git a/packages/vue/src/integration.ts b/packages/vue/src/integration.ts index 90e02014aaac..e34c3493d1e0 100644 --- a/packages/vue/src/integration.ts +++ b/packages/vue/src/integration.ts @@ -40,7 +40,7 @@ export class VueIntegration implements Integration { } /** @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, getCurrentHub: () => Hub): void { + public setupOnce(_addGlobalEventProcessor: unknown, getCurrentHub: () => Hub): void { this._setupIntegration(getCurrentHub()); } diff --git a/packages/wasm/src/index.ts b/packages/wasm/src/index.ts index a45b18ac4e96..49487167e346 100644 --- a/packages/wasm/src/index.ts +++ b/packages/wasm/src/index.ts @@ -49,7 +49,7 @@ export class Wasm implements Integration { /** * @inheritDoc */ - public setupOnce(_addGlobaleventProcessor: unknown, _getCurrentHub: unknown): void { + public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void { patchWebAssembly(); } From bfc63cb9c1b81f8e410f6e40dbef01fc52462b81 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 4 Dec 2023 17:28:43 +0100 Subject: [PATCH 7/8] fix(astro): Isolate request instrumentation middleware (#9709) Add scope isolation to our sever-side request instrumentation, similarly to the request handler in Sveltekit. --- packages/astro/src/server/middleware.ts | 201 ++++++++++-------- packages/astro/test/server/middleware.test.ts | 74 +++++-- packages/core/src/tracing/trace.ts | 1 - 3 files changed, 167 insertions(+), 109 deletions(-) diff --git a/packages/astro/src/server/middleware.ts b/packages/astro/src/server/middleware.ts index fc63639691ea..8ff33382d916 100644 --- a/packages/astro/src/server/middleware.ts +++ b/packages/astro/src/server/middleware.ts @@ -1,4 +1,11 @@ -import { captureException, configureScope, getCurrentHub, startSpan } from '@sentry/node'; +import { + captureException, + configureScope, + continueTrace, + getCurrentHub, + runWithAsyncContext, + startSpan, +} from '@sentry/node'; import type { Hub, Span } from '@sentry/types'; import { addNonEnumerableProperty, @@ -56,107 +63,125 @@ type AstroLocalsWithSentry = Record & { __sentry_wrapped__?: boolean; }; -export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = ( - options = { trackClientIp: false, trackHeaders: false }, -) => { +export const handleRequest: (options?: MiddlewareOptions) => MiddlewareResponseHandler = options => { + const handlerOptions = { trackClientIp: false, trackHeaders: false, ...options }; + return async (ctx, next) => { - // Make sure we don't accidentally double wrap (e.g. user added middleware and integration auto added it) - const locals = ctx.locals as AstroLocalsWithSentry; - if (locals && locals.__sentry_wrapped__) { - return next(); + // if there is an active span, we know that this handle call is nested and hence + // we don't create a new domain for it. If we created one, nested server calls would + // create new transactions instead of adding a child span to the currently active span. + if (getCurrentHub().getScope().getSpan()) { + return instrumentRequest(ctx, next, handlerOptions); } - addNonEnumerableProperty(locals, '__sentry_wrapped__', true); + return runWithAsyncContext(() => { + return instrumentRequest(ctx, next, handlerOptions); + }); + }; +}; - const method = ctx.request.method; - const headers = ctx.request.headers; +async function instrumentRequest( + ctx: Parameters[0], + next: Parameters[1], + options: MiddlewareOptions, +): Promise { + // Make sure we don't accidentally double wrap (e.g. user added middleware and integration auto added it) + const locals = ctx.locals as AstroLocalsWithSentry; + if (locals && locals.__sentry_wrapped__) { + return next(); + } + addNonEnumerableProperty(locals, '__sentry_wrapped__', true); - const { dynamicSamplingContext, traceparentData, propagationContext } = tracingContextFromHeaders( - headers.get('sentry-trace') || undefined, - headers.get('baggage'), - ); + const { method, headers } = ctx.request; - const allHeaders: Record = {}; + const traceCtx = continueTrace({ + sentryTrace: headers.get('sentry-trace') || undefined, + baggage: headers.get('baggage'), + }); + + const allHeaders: Record = {}; + + if (options.trackHeaders) { headers.forEach((value, key) => { allHeaders[key] = value; }); + } + if (options.trackClientIp) { configureScope(scope => { - scope.setPropagationContext(propagationContext); - - if (options.trackClientIp) { - scope.setUser({ ip_address: ctx.clientAddress }); - } + scope.setUser({ ip_address: ctx.clientAddress }); }); + } - try { - // storing res in a variable instead of directly returning is necessary to - // invoke the catch block if next() throws - const res = await startSpan( - { - name: `${method} ${interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params)}`, - op: 'http.server', - origin: 'auto.http.astro', - status: 'ok', - ...traceparentData, - metadata: { - source: 'route', - dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, - }, - data: { - method, - url: stripUrlQueryAndFragment(ctx.url.href), - ...(ctx.url.search && { 'http.query': ctx.url.search }), - ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }), - ...(options.trackHeaders && { headers: allHeaders }), - }, + try { + // storing res in a variable instead of directly returning is necessary to + // invoke the catch block if next() throws + const res = await startSpan( + { + ...traceCtx, + name: `${method} ${interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params)}`, + op: 'http.server', + origin: 'auto.http.astro', + status: 'ok', + metadata: { + ...traceCtx?.metadata, + source: 'route', }, - async span => { - const originalResponse = await next(); - - if (span && originalResponse.status) { - span.setHttpStatus(originalResponse.status); - } - - const hub = getCurrentHub(); - const client = hub.getClient(); - const contentType = originalResponse.headers.get('content-type'); - - const isPageloadRequest = contentType && contentType.startsWith('text/html'); - if (!isPageloadRequest || !client) { - return originalResponse; - } - - // Type case necessary b/c the body's ReadableStream type doesn't include - // the async iterator that is actually available in Node - // We later on use the async iterator to read the body chunks - // see https://github.com/microsoft/TypeScript/issues/39051 - const originalBody = originalResponse.body as NodeJS.ReadableStream | null; - if (!originalBody) { - return originalResponse; - } - - const newResponseStream = new ReadableStream({ - start: async controller => { - for await (const chunk of originalBody) { - const html = typeof chunk === 'string' ? chunk : new TextDecoder().decode(chunk); - const modifiedHtml = addMetaTagToHead(html, hub, span); - controller.enqueue(new TextEncoder().encode(modifiedHtml)); - } - controller.close(); - }, - }); - - return new Response(newResponseStream, originalResponse); + data: { + method, + url: stripUrlQueryAndFragment(ctx.url.href), + ...(ctx.url.search && { 'http.query': ctx.url.search }), + ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }), + ...(options.trackHeaders && { headers: allHeaders }), }, - ); - return res; - } catch (e) { - sendErrorToSentry(e); - throw e; - } - // TODO: flush if serveless (first extract function) - }; -}; + }, + async span => { + const originalResponse = await next(); + + if (span && originalResponse.status) { + span.setHttpStatus(originalResponse.status); + } + + const hub = getCurrentHub(); + const client = hub.getClient(); + const contentType = originalResponse.headers.get('content-type'); + + const isPageloadRequest = contentType && contentType.startsWith('text/html'); + if (!isPageloadRequest || !client) { + return originalResponse; + } + + // Type case necessary b/c the body's ReadableStream type doesn't include + // the async iterator that is actually available in Node + // We later on use the async iterator to read the body chunks + // see https://github.com/microsoft/TypeScript/issues/39051 + const originalBody = originalResponse.body as NodeJS.ReadableStream | null; + if (!originalBody) { + return originalResponse; + } + + const decoder = new TextDecoder(); + + const newResponseStream = new ReadableStream({ + start: async controller => { + for await (const chunk of originalBody) { + const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk); + const modifiedHtml = addMetaTagToHead(html, hub, span); + controller.enqueue(new TextEncoder().encode(modifiedHtml)); + } + controller.close(); + }, + }); + + return new Response(newResponseStream, originalResponse); + }, + ); + return res; + } catch (e) { + sendErrorToSentry(e); + throw e; + } + // TODO: flush if serverless (first extract function) +} /** * This function optimistically assumes that the HTML coming in chunks will not be split diff --git a/packages/astro/test/server/middleware.test.ts b/packages/astro/test/server/middleware.test.ts index 39146cdaa1d7..19ee0ef1b5c6 100644 --- a/packages/astro/test/server/middleware.test.ts +++ b/packages/astro/test/server/middleware.test.ts @@ -12,6 +12,18 @@ vi.mock('../../src/server/meta', () => ({ describe('sentryMiddleware', () => { const startSpanSpy = vi.spyOn(SentryNode, 'startSpan'); + + const getSpanMock = vi.fn(() => {}); + // @ts-expect-error only returning a partial hub here + vi.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => { + return { + getScope: () => ({ + getSpan: getSpanMock, + }), + getClient: () => ({}), + }; + }); + const nextResult = Promise.resolve(new Response(null, { status: 200, headers: new Headers() })); afterEach(() => { @@ -86,10 +98,6 @@ describe('sentryMiddleware', () => { }); it('attaches tracing headers', async () => { - const scope = { setUser: vi.fn(), setPropagationContext: vi.fn() }; - // @ts-expect-error, only passing a partial Scope object - const configureScopeSpy = vi.spyOn(SentryNode, 'configureScope').mockImplementation(cb => cb(scope)); - const middleware = handleRequest(); const ctx = { request: { @@ -108,17 +116,6 @@ describe('sentryMiddleware', () => { // @ts-expect-error, a partial ctx object is fine here await middleware(ctx, next); - expect(configureScopeSpy).toHaveBeenCalledTimes(1); - expect(scope.setPropagationContext).toHaveBeenCalledWith({ - dsc: { - release: '1.0.0', - }, - parentSpanId: '1234567890123456', - sampled: true, - spanId: expect.any(String), - traceId: '12345678901234567890123456789012', - }); - expect(startSpanSpy).toHaveBeenCalledWith( expect.objectContaining({ metadata: { @@ -174,11 +171,6 @@ describe('sentryMiddleware', () => { }); it('injects tracing tags into the HTML of a pageload response', async () => { - vi.spyOn(SentryNode, 'getCurrentHub').mockImplementation(() => ({ - // @ts-expect-error this is fine - getClient: () => ({}), - })); - const middleware = handleRequest(); const ctx = { @@ -261,6 +253,48 @@ describe('sentryMiddleware', () => { expect(html).toBe(originalHtml); }); + + describe('async context isolation', () => { + const runWithAsyncContextSpy = vi.spyOn(SentryNode, 'runWithAsyncContext'); + afterEach(() => { + vi.clearAllMocks(); + runWithAsyncContextSpy.mockRestore(); + }); + + it('starts a new async context if no span is active', async () => { + getSpanMock.mockReturnValueOnce(undefined); + const handler = handleRequest(); + const ctx = {}; + const next = vi.fn(); + + try { + // @ts-expect-error, a partial ctx object is fine here + await handler(ctx, next); + } catch { + // this is fine, it's not required to pass in this test + } + + expect(runWithAsyncContextSpy).toHaveBeenCalledTimes(1); + }); + + it("doesn't start a new async context if a span is active", async () => { + // @ts-expect-error, a empty span is fine here + getSpanMock.mockReturnValueOnce({}); + + const handler = handleRequest(); + const ctx = {}; + const next = vi.fn(); + + try { + // @ts-expect-error, a partial ctx object is fine here + await handler(ctx, next); + } catch { + // this is fine, it's not required to pass in this test + } + + expect(runWithAsyncContextSpy).not.toHaveBeenCalled(); + }); + }); }); describe('interpolateRouteFromUrlAndParams', () => { diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 99d706f0585e..667bedaaef6c 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -226,7 +226,6 @@ export function continueTrace( * These values can be obtained from incoming request headers, * or in the browser from `` and `` HTML tags. * - * It also takes an optional `request` option, which if provided will also be added to the scope & transaction metadata. * The callback receives a transactionContext that may be used for `startTransaction` or `startSpan`. */ export function continueTrace( From 1fd0b614e783820abd1df20394fae6bd54e1cc31 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 4 Dec 2023 16:45:07 +0100 Subject: [PATCH 8/8] meta(changelog): Update changelog for 7.85.0 --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67ef36fba32e..815650e0d8c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +## 7.85.0 + +- feat(core): Add `addEventProcessor` method (#9554) +- feat(crons): Add interface for heartbeat checkin (#9706) +- feat(feedback): Include Feedback package in browser SDK (#9586) +- fix(astro): Isolate request instrumentation in middleware (#9709) +- fix(replay): Capture JSON XHR response bodies (#9623) +- ref(feedback): Change form `box-shadow` to use CSS var (#9630) + ## 7.84.0 ### Important Changes