From 7846b242fc139ebf0f18db1c9fdc3cf7c4e6815b Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 6 Dec 2024 11:27:48 +0100 Subject: [PATCH 01/38] ci(v9): Ensure CI runs on v8 & v9 branches (#14604) In order for us to have size-limit comparison etc, we need to ensure CI runs on v8 & v9 branches too. --- .github/workflows/build.yml | 10 ++++++---- .github/workflows/enforce-license-compliance.yml | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5df700a2947e..c26be7e5ec96 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,6 +4,8 @@ on: branches: - develop - master + - v9 + - v8 - release/** pull_request: merge_group: @@ -105,7 +107,7 @@ jobs: outputs: commit_label: '${{ env.COMMIT_SHA }}: ${{ env.COMMIT_MESSAGE }}' # Note: These next three have to be checked as strings ('true'/'false')! - is_develop: ${{ github.ref == 'refs/heads/develop' }} + is_base_branch: ${{ github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/v9' || github.ref == 'refs/heads/v8'}} is_release: ${{ startsWith(github.ref, 'refs/heads/release/') }} changed_profiling_node: ${{ steps.changed.outputs.profiling_node == 'true' }} changed_ci: ${{ steps.changed.outputs.workflow == 'true' }} @@ -126,7 +128,7 @@ jobs: timeout-minutes: 15 if: | needs.job_get_metadata.outputs.changed_any_code == 'true' || - needs.job_get_metadata.outputs.is_develop == 'true' || + needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' || (needs.job_get_metadata.outputs.is_gitflow_sync == 'false' && needs.job_get_metadata.outputs.has_gitflow_label == 'false') steps: @@ -171,7 +173,7 @@ jobs: key: nx-Linux-${{ github.ref }}-${{ env.HEAD_COMMIT || github.sha }} # On develop branch, we want to _store_ the cache (so it can be used by other branches), but never _restore_ from it restore-keys: - ${{needs.job_get_metadata.outputs.is_develop == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} + ${{needs.job_get_metadata.outputs.is_base_branch == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} - name: Build packages # Set the CODECOV_TOKEN for Bundle Analysis @@ -219,7 +221,7 @@ jobs: timeout-minutes: 15 runs-on: ubuntu-20.04 if: - github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_develop == 'true' || + github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' steps: - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) diff --git a/.github/workflows/enforce-license-compliance.yml b/.github/workflows/enforce-license-compliance.yml index 8f63b6ca063b..776f8135178d 100644 --- a/.github/workflows/enforce-license-compliance.yml +++ b/.github/workflows/enforce-license-compliance.yml @@ -2,9 +2,18 @@ name: "CI: Enforce License Compliance" on: push: - branches: [master, develop, release/*] + branches: + - develop + - master + - v9 + - v8 + - release/** pull_request: - branches: [master, develop] + branches: + - develop + - master + - v9 + - v8 jobs: enforce-license-compliance: From d1ee44491fb33aae76ff8082805983ecadc463dd Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:06:10 +0100 Subject: [PATCH 02/38] feat(vue/nuxt)!: No longer create `"update"` spans for component tracking by default (#14602) Resolves https://github.com/getsentry/sentry-javascript/issues/12851 --- docs/migration/draft-v9-migration-guide.md | 22 ++++++++++++++++++++++ packages/vue/src/constants.ts | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index d3b31b4f2570..3faab01d450e 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -102,6 +102,28 @@ }); ``` +## `@sentry/nuxt` and `@sentry/vue` + +- When component tracking is enabled, "update" spans are no longer created by default. + Add an `"update"` item to the `tracingOptions.hooks` option via the `vueIntegration()` to restore this behavior. + + ```ts + Sentry.init({ + integrations: [ + Sentry.vueIntegration({ + tracingOptions: { + trackComponents: true, + hooks: [ + 'mount', + 'update', // <-- + 'unmount', + ], + }, + }), + ], + }); + ``` + ## `@sentry/astro` - Deprecated passing `dsn`, `release`, `environment`, `sampleRate`, `tracesSampleRate`, `replaysSessionSampleRate` to the integration. Use the runtime-specific `Sentry.init()` calls for passing these options instead. diff --git a/packages/vue/src/constants.ts b/packages/vue/src/constants.ts index e254d988c40c..50aa82f77885 100644 --- a/packages/vue/src/constants.ts +++ b/packages/vue/src/constants.ts @@ -1,3 +1,3 @@ import type { Operation } from './types'; -export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount', 'update']; +export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount']; From 26119bf1a3cced7b73b369d94f0fb874c11abd74 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:43:05 +0100 Subject: [PATCH 03/38] feat(nextjs)!: Remove `experimental_captureRequestError` (#14607) Fixes https://github.com/getsentry/sentry-javascript/issues/14302 --- .../e2e-tests/test-applications/nextjs-t3/package.json | 6 +++--- .../test-applications/nextjs-t3/src/trpc/server.ts | 7 +++---- packages/nextjs/src/common/captureRequestError.ts | 8 -------- packages/nextjs/src/common/index.ts | 3 +-- packages/nextjs/src/index.types.ts | 3 +-- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json index d5c3a9d20f0d..b85fec6a4897 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json @@ -21,9 +21,9 @@ "@trpc/react-query": "^11.0.0-rc.446", "@trpc/server": "^11.0.0-rc.446", "geist": "^1.3.0", - "next": "^14.2.4", - "react": "^18.3.1", - "react-dom": "^18.3.1", + "next": "14.2.4", + "react": "18.3.1", + "react-dom": "18.3.1", "server-only": "^0.0.1", "superjson": "^2.2.1", "zod": "^3.23.3" diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts index b6cb13a70781..7760873eb51d 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts @@ -2,7 +2,6 @@ import 'server-only'; import { createHydrationHelpers } from '@trpc/react-query/rsc'; import { headers } from 'next/headers'; -import { cache } from 'react'; import { type AppRouter, createCaller } from '~/server/api/root'; import { createTRPCContext } from '~/server/api/trpc'; @@ -12,16 +11,16 @@ import { createQueryClient } from './query-client'; * This wraps the `createTRPCContext` helper and provides the required context for the tRPC API when * handling a tRPC call from a React Server Component. */ -const createContext = cache(() => { +const createContext = () => { const heads = new Headers(headers()); heads.set('x-trpc-source', 'rsc'); return createTRPCContext({ headers: heads, }); -}); +}; -const getQueryClient = cache(createQueryClient); +const getQueryClient = createQueryClient; const caller = createCaller(createContext); export const { trpc: api, HydrateClient } = createHydrationHelpers(caller, getQueryClient); diff --git a/packages/nextjs/src/common/captureRequestError.ts b/packages/nextjs/src/common/captureRequestError.ts index 6de33ad11a8e..26fdaab4953b 100644 --- a/packages/nextjs/src/common/captureRequestError.ts +++ b/packages/nextjs/src/common/captureRequestError.ts @@ -41,11 +41,3 @@ export function captureRequestError(error: unknown, request: RequestInfo, errorC }); }); } - -/** - * Reports errors passed to the the Next.js `onRequestError` instrumentation hook. - * - * @deprecated Use `captureRequestError` instead. - */ -// TODO(v9): Remove this export -export const experimental_captureRequestError = captureRequestError; diff --git a/packages/nextjs/src/common/index.ts b/packages/nextjs/src/common/index.ts index 7740c35c016c..b9a652522349 100644 --- a/packages/nextjs/src/common/index.ts +++ b/packages/nextjs/src/common/index.ts @@ -11,5 +11,4 @@ export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; export { wrapPageComponentWithSentry } from './pages-router-instrumentation/wrapPageComponentWithSentry'; export { wrapGenerationFunctionWithSentry } from './wrapGenerationFunctionWithSentry'; export { withServerActionInstrumentation } from './withServerActionInstrumentation'; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './captureRequestError'; +export { captureRequestError } from './captureRequestError'; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 1b6a0e09ed85..1b965828116f 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -142,5 +142,4 @@ export declare function wrapApiHandlerWithSentryVercelCrons(WrappingTarget: C): C; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './common/captureRequestError'; +export { captureRequestError } from './common/captureRequestError'; From d7e36265dc76391d9306abcf6296365f4e442f7a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 11 Dec 2024 11:22:52 +0000 Subject: [PATCH 04/38] feat(node)!: Collect request sessions via HTTP instrumentation --- packages/core/src/baseclient.ts | 32 ++++-- packages/core/src/server-runtime-client.ts | 97 +++---------------- packages/core/src/types-hoist/integration.ts | 10 ++ .../http/SentryHttpInstrumentation.ts | 89 ++++++++++++++--- packages/node/src/integrations/http/index.ts | 47 +++------ .../node/src/integrations/tracing/express.ts | 24 ----- 6 files changed, 139 insertions(+), 160 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index c394a0d77a95..910219a0b172 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -54,6 +54,7 @@ import { prepareEvent } from './utils/prepareEvent'; import { showSpanDropWarning } from './utils/spanUtils'; const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured."; +const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of miss ing or non-string release'; /** * Base implementation for all JavaScript SDK clients. @@ -235,13 +236,9 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public captureSession(session: Session): void { - if (!(typeof session.release === 'string')) { - DEBUG_BUILD && logger.warn('Discarded session because of missing or non-string release'); - } else { - this.sendSession(session); - // After sending, we set init false to indicate it's not the first occurrence - updateSession(session, { init: false }); - } + this.sendSession(session); + // After sending, we set init false to indicate it's not the first occurrence + updateSession(session, { init: false }); } /** @@ -370,6 +367,27 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public sendSession(session: Session | SessionAggregates): void { + const clientReleaseOption = this._options.release; + const clientEnvironmentOption = this._options.environment; + if ('aggregates' in session) { + // TODO(v9): Remove eslint disable + // eslint-disable-next-line @sentry-internal/sdk/no-optional-chaining + if (!session.attrs?.release && !clientReleaseOption) { + DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); + return; + } + session.attrs = session.attrs || {}; + session.attrs.release = session.attrs.release || clientReleaseOption; + session.attrs.environment = session.attrs.environment || clientEnvironmentOption; + } else { + if (!session.release && !clientReleaseOption) { + DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); + return; + } + session.release = session.release || clientReleaseOption; + session.environment = session.environment || clientEnvironmentOption; + } + const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel); // sendEnvelope should not throw diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index bc9489406282..170aa0159923 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -17,7 +17,6 @@ import { createCheckInEnvelope } from './checkin'; import { getIsolationScope, getTraceContextFromScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import type { Scope } from './scope'; -import { SessionFlusher } from './sessionflusher'; import { getDynamicSamplingContextFromScope, getDynamicSamplingContextFromSpan, @@ -42,9 +41,6 @@ export interface ServerRuntimeClientOptions extends ClientOptions extends BaseClient { - // eslint-disable-next-line deprecation/deprecation - protected _sessionFlusher: SessionFlusher | undefined; - /** * Creates a new Edge SDK instance. * @param options Configuration options for this SDK. @@ -80,21 +76,6 @@ export class ServerRuntimeClient< * @inheritDoc */ public captureException(exception: unknown, hint?: EventHint, scope?: Scope): string { - // Check if `_sessionFlusher` exists because it is initialized (defined) only when the `autoSessionTracking` is enabled. - // The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled. - // TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions. - // TODO(v9): This should move into the httpIntegration. - if (this._options.autoSessionTracking && this._sessionFlusher) { - // eslint-disable-next-line deprecation/deprecation - const requestSession = getIsolationScope().getRequestSession(); - - // Necessary checks to ensure this is code block is executed only within a request - // Should override the status only if `requestSession.status` is `Ok`, which is its initial stage - if (requestSession && requestSession.status === 'ok') { - requestSession.status = 'errored'; - } - } - return super.captureException(exception, hint, scope); } @@ -102,62 +83,24 @@ export class ServerRuntimeClient< * @inheritDoc */ public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string { - // Check if `_sessionFlusher` exists because it is initialized only when the `autoSessionTracking` is enabled. - // The expectation is that session aggregates are only sent when `autoSessionTracking` is enabled. - // TODO(v9): Our goal in the future is to not have the `autoSessionTracking` option and instead rely on integrations doing the creation and sending of sessions. We will not have a central kill-switch for sessions. - // TODO(v9): This should move into the httpIntegration. - if (this._options.autoSessionTracking && this._sessionFlusher) { - const eventType = event.type || 'exception'; - const isException = - eventType === 'exception' && event.exception && event.exception.values && event.exception.values.length > 0; - - // If the event is of type Exception, then a request session should be captured - if (isException) { - // eslint-disable-next-line deprecation/deprecation - const requestSession = getIsolationScope().getRequestSession(); - - // Ensure that this is happening within the bounds of a request, and make sure not to override - // Session Status if Errored / Crashed - if (requestSession && requestSession.status === 'ok') { - requestSession.status = 'errored'; - } + // If the event is of type Exception, then a request session should be captured + const isException = !event.type && event.exception && event.exception.values && event.exception.values.length > 0; + if (isException) { + // TODO: Check if mechanism === unhandled + const isolationScope = getIsolationScope(); + const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; + if (requestSession) { + isolationScope.setSDKProcessingMetadata({ + requestSession: { + status: 'errored', + }, + }); } } return super.captureEvent(event, hint, scope); } - /** - * - * @inheritdoc - */ - public close(timeout?: number): PromiseLike { - if (this._sessionFlusher) { - this._sessionFlusher.close(); - } - return super.close(timeout); - } - - /** - * Initializes an instance of SessionFlusher on the client which will aggregate and periodically flush session data. - * - * NOTICE: This method will implicitly create an interval that is periodically called. - * To clean up this resources, call `.close()` when you no longer intend to use the client. - * Not doing so will result in a memory leak. - */ - public initSessionFlusher(): void { - const { release, environment } = this._options; - if (!release) { - DEBUG_BUILD && logger.warn('Cannot initialize an instance of SessionFlusher if no release is provided!'); - } else { - // eslint-disable-next-line deprecation/deprecation - this._sessionFlusher = new SessionFlusher(this, { - release, - environment, - }); - } - } - /** * Create a cron monitor check in and send it to Sentry. * @@ -168,7 +111,7 @@ export class ServerRuntimeClient< public captureCheckIn(checkIn: CheckIn, monitorConfig?: MonitorConfig, scope?: Scope): string { const id = 'checkInId' in checkIn && checkIn.checkInId ? checkIn.checkInId : uuid4(); if (!this._isEnabled()) { - DEBUG_BUILD && logger.warn('SDK not enabled, will not capture checkin.'); + DEBUG_BUILD && logger.warn('SDK not enabled, will not capture check-in.'); return id; } @@ -222,20 +165,6 @@ export class ServerRuntimeClient< return id; } - /** - * Method responsible for capturing/ending a request session by calling `incrementSessionStatusCount` to increment - * appropriate session aggregates bucket - * - * @deprecated This method should not be used or extended. It's functionality will move into the `httpIntegration` and not be part of any public API. - */ - protected _captureRequestSession(): void { - if (!this._sessionFlusher) { - DEBUG_BUILD && logger.warn('Discarded request mode session because autoSessionTracking option was disabled'); - } else { - this._sessionFlusher.incrementSessionStatusCount(); - } - } - /** * @inheritDoc */ diff --git a/packages/core/src/types-hoist/integration.ts b/packages/core/src/types-hoist/integration.ts index deb23baaca51..85d32b1c682a 100644 --- a/packages/core/src/types-hoist/integration.ts +++ b/packages/core/src/types-hoist/integration.ts @@ -51,6 +51,16 @@ export interface Integration { * This receives the client that the integration was installed for as third argument. */ processEvent?(event: Event, hint: EventHint, client: Client): Event | null | PromiseLike; + + /** + * TODO + */ + flush?(client: Client): Promise; + + /** + * TODO + */ + close?(client: Client): Promise; } /** diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 30421e66a345..e0a32607a52c 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -1,14 +1,14 @@ +/* eslint-disable max-lines */ import type * as http from 'node:http'; import type { IncomingMessage, RequestOptions } from 'node:http'; import type * as https from 'node:https'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; -import type { RequestEventData, SanitizedRequestData, Scope } from '@sentry/core'; +import type { AggregationCounts, Client, RequestEventData, SanitizedRequestData, Scope } from '@sentry/core'; import { addBreadcrumb, getBreadcrumbLogLevelFromHttpStatusCode, - getClient, getIsolationScope, getSanitizedUrlString, httpRequestToRequestData, @@ -18,9 +18,18 @@ import { withIsolationScope, } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; -import type { NodeClient } from '../../sdk/client'; import { getRequestUrl } from '../../utils/getRequestUrl'; import { getRequestInfo } from './vendor/getRequestInfo'; + +const clientToAggregatesMap = new Map< + Client, + { [timestampRoundedToSeconds: string]: { exited: number; errored: number } } +>(); + +interface RequestSession { + status: 'ok' | 'errored'; +} + type Http = typeof http; type Https = typeof https; @@ -42,6 +51,13 @@ type SentryHttpInstrumentationOptions = InstrumentationConfig & { * @param request Contains the {@type RequestOptions} object used to make the outgoing request. */ ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; + + /** + * TODO + * + * Defaults to `true`. + */ + trackIncomingRequestsAsSessions?: boolean; }; // We only want to capture request bodies up to 1mb. @@ -134,6 +150,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase(); - if (client && client.getOptions().autoSessionTracking) { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - } - // attempt to update the scope's `transactionName` based on the request URL // Ideally, framework instrumentations coming after the HttpInstrumentation // update the transactionName once we get a parameterized route. @@ -162,6 +173,62 @@ export class SentryHttpInstrumentation extends InstrumentationBase { + const client = isolationScope.getClient(); + const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession as + | RequestSession + | undefined; + + if (client && requestSession) { + const roundedDate = new Date(); + roundedDate.setSeconds(0, 0); + const dateBucketKey = roundedDate.toISOString(); + + const existingClientAggregate = clientToAggregatesMap.get(client); + if (existingClientAggregate) { + DEBUG_BUILD && logger.debug(`Recorded request session with status: ${requestSession.status}`); + const bucket = existingClientAggregate[dateBucketKey] || { errored: 0, exited: 0 }; + bucket[requestSession.status === 'ok' ? 'exited' : 'errored']++; + existingClientAggregate[dateBucketKey] = bucket; + } else { + DEBUG_BUILD && logger.debug('Opened new request session aggregate.'); + const bucket = { errored: 0, exited: 0 }; + bucket[requestSession.status === 'ok' ? 'exited' : 'errored']++; + const newClientAggregate = { [dateBucketKey]: bucket }; + clientToAggregatesMap.set(client, newClientAggregate); + + const flushPendingClientAggregates = (): void => { + clearTimeout(timeout); + unregisterClientFlushHook(); + clientToAggregatesMap.delete(client); + + const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( + ([timestamp, value]) => ({ + started: timestamp, + exited: value.exited, + errored: value.errored, + }), + ); + client.sendSession({ aggregates: aggregatePayload }); + }; + + const unregisterClientFlushHook = client.on('flush', () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); + flushPendingClientAggregates(); + }); + const timeout = setTimeout(() => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, 60_000).unref(); + } + } + }); + } + return withIsolationScope(isolationScope, () => { return original.apply(this, [event, ...args]); }); @@ -362,8 +429,8 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): if (event === 'data') { const callback = new Proxy(listener, { apply: (target, thisArg, args: Parameters) => { - // If we have already read more than the max body length, we stop addiing chunks - // To avoid growing the memory indefinitely if a respons is e.g. streamed + // If we have already read more than the max body length, we stop adding chunks + // To avoid growing the memory indefinitely if a response is e.g. streamed if (getChunksSize() < MAX_BODY_BYTE_LENGTH) { const chunk = args[0] as Buffer; chunks.push(chunk); diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 10f0583b5ac6..1f287bd0d1ff 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -4,9 +4,7 @@ import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-h import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import type { Span } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; -import { getClient } from '@sentry/opentelemetry'; import { generateInstrumentOnce } from '../../otel/instrument'; -import type { NodeClient } from '../../sdk/client'; import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { getRequestUrl } from '../../utils/getRequestUrl'; @@ -35,8 +33,6 @@ interface HttpOptions { * Read more about Release Health: https://docs.sentry.io/product/releases/health/ * * Defaults to `true`. - * - * Note: If `autoSessionTracking` is set to `false` in `Sentry.init()` or the Client owning this integration, this option will be ignored. */ trackIncomingRequestsAsSessions?: boolean; @@ -90,13 +86,15 @@ interface HttpOptions { }; } -export const instrumentSentryHttp = generateInstrumentOnce<{ +const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: HttpOptions['breadcrumbs']; ignoreOutgoingRequests?: HttpOptions['ignoreOutgoingRequests']; + trackIncomingRequestsAsSessions?: HttpOptions['trackIncomingRequestsAsSessions']; }>(`${INTEGRATION_NAME}.sentry`, options => { return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs, ignoreOutgoingRequests: options?.ignoreOutgoingRequests, + trackIncomingRequestsAsSessions: options?.trackIncomingRequestsAsSessions, }); }); @@ -117,22 +115,6 @@ export const instrumentOtelHttp = generateInstrumentOnce { - // This is the "regular" OTEL instrumentation that emits spans - if (options.spans !== false) { - const instrumentationConfig = getConfigWithDefaults(options); - instrumentOtelHttp(instrumentationConfig); - } - - // This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs - // Note that this _has_ to be wrapped after the OTEL instrumentation, - // otherwise the isolation will not work correctly - instrumentSentryHttp(options); -}; - /** * The http integration instruments Node's internal http and https modules. * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. @@ -141,7 +123,16 @@ export const httpIntegration = defineIntegration((options: HttpOptions = {}) => return { name: INTEGRATION_NAME, setupOnce() { - instrumentHttp(options); + // This is the "regular" OTEL instrumentation that emits spans + if (options.spans !== false) { + const instrumentationConfig = getConfigWithDefaults(options); + instrumentOtelHttp(instrumentationConfig); + } + + // This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs + // Note that this _has_ to be wrapped after the OTEL instrumentation, + // otherwise the isolation will not work correctly + instrumentSentryHttp(options); }, }; }); @@ -214,18 +205,6 @@ function getConfigWithDefaults(options: Partial = {}): HttpInstrume options.instrumentation?.requestHook?.(span, req); }, responseHook: (span, res) => { - const client = getClient(); - - if ( - client && - client.getOptions().autoSessionTracking !== false && - options.trackIncomingRequestsAsSessions !== false - ) { - setImmediate(() => { - client['_captureRequestSession'](); - }); - } - options.instrumentation?.responseHook?.(span, res); }, applyCustomAttributesOnSpan: ( diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index bcb668d52f80..de39093dda73 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -5,7 +5,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_OP, captureException, defineIntegration, - getClient, getDefaultIsolationScope, getIsolationScope, logger, @@ -13,7 +12,6 @@ import { } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; import { generateInstrumentOnce } from '../../otel/instrument'; -import type { NodeClient } from '../../sdk/client'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { ensureIsWrapped } from '../../utils/ensureIsWrapped'; @@ -121,30 +119,8 @@ export function expressErrorHandler(options?: ExpressHandlerOptions): ExpressMid const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; if (shouldHandleError(error)) { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - // Check if the `SessionFlusher` is instantiated on the client to go into this branch that marks the - // `requestSession.status` as `Crashed`, and this check is necessary because the `SessionFlusher` is only - // instantiated when the the`requestHandler` middleware is initialised, which indicates that we should be - // running in SessionAggregates mode - const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined; - if (isSessionAggregatesMode) { - // eslint-disable-next-line deprecation/deprecation - const requestSession = getIsolationScope().getRequestSession(); - // If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a - // Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within - // the bounds of a request, and if so the status is updated - if (requestSession && requestSession.status !== undefined) { - requestSession.status = 'crashed'; - } - } - } - const eventId = captureException(error, { mechanism: { type: 'middleware', handled: false } }); (res as { sentry?: string }).sentry = eventId; - next(error); - - return; } next(error); From 5430723faa947dee2b62e60dbc4bae58d1da7ddc Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 11 Dec 2024 11:51:22 +0000 Subject: [PATCH 05/38] depr notice --- docs/migration/draft-v9-migration-guide.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index 3faab01d450e..7b05cc7c6d3f 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -51,6 +51,7 @@ - Deprecated `RequestSessionStatus` type. No replacements. - Deprecated `SessionFlusherLike` type. No replacements. - Deprecated `SessionFlusher`. No replacements. +- Deprecated `initSessionFlusher` on `ServerRuntimeClient`. No replacements. The `httpIntegration` will flush sessions by itself. ## `@sentry/nestjs` From 6610144af0acbf98887f9666fcc2e5718601590e Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 6 Dec 2024 11:27:48 +0100 Subject: [PATCH 06/38] ci(v9): Ensure CI runs on v8 & v9 branches (#14604) In order for us to have size-limit comparison etc, we need to ensure CI runs on v8 & v9 branches too. --- .github/workflows/build.yml | 10 ++++++---- .github/workflows/enforce-license-compliance.yml | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf9ba21376bb..31f5cea28bda 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,6 +4,8 @@ on: branches: - develop - master + - v9 + - v8 - release/** pull_request: merge_group: @@ -105,7 +107,7 @@ jobs: outputs: commit_label: '${{ env.COMMIT_SHA }}: ${{ env.COMMIT_MESSAGE }}' # Note: These next three have to be checked as strings ('true'/'false')! - is_develop: ${{ github.ref == 'refs/heads/develop' }} + is_base_branch: ${{ github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/v9' || github.ref == 'refs/heads/v8'}} is_release: ${{ startsWith(github.ref, 'refs/heads/release/') }} changed_profiling_node: ${{ steps.changed.outputs.profiling_node == 'true' }} changed_ci: ${{ steps.changed.outputs.workflow == 'true' }} @@ -126,7 +128,7 @@ jobs: timeout-minutes: 15 if: | needs.job_get_metadata.outputs.changed_any_code == 'true' || - needs.job_get_metadata.outputs.is_develop == 'true' || + needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' || (needs.job_get_metadata.outputs.is_gitflow_sync == 'false' && needs.job_get_metadata.outputs.has_gitflow_label == 'false') steps: @@ -171,7 +173,7 @@ jobs: key: nx-Linux-${{ github.ref }}-${{ env.HEAD_COMMIT || github.sha }} # On develop branch, we want to _store_ the cache (so it can be used by other branches), but never _restore_ from it restore-keys: - ${{needs.job_get_metadata.outputs.is_develop == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} + ${{needs.job_get_metadata.outputs.is_base_branch == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} - name: Build packages # Set the CODECOV_TOKEN for Bundle Analysis @@ -219,7 +221,7 @@ jobs: timeout-minutes: 15 runs-on: ubuntu-20.04 if: - github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_develop == 'true' || + github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' steps: - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) diff --git a/.github/workflows/enforce-license-compliance.yml b/.github/workflows/enforce-license-compliance.yml index 8f63b6ca063b..776f8135178d 100644 --- a/.github/workflows/enforce-license-compliance.yml +++ b/.github/workflows/enforce-license-compliance.yml @@ -2,9 +2,18 @@ name: "CI: Enforce License Compliance" on: push: - branches: [master, develop, release/*] + branches: + - develop + - master + - v9 + - v8 + - release/** pull_request: - branches: [master, develop] + branches: + - develop + - master + - v9 + - v8 jobs: enforce-license-compliance: From 164869809e343aee924ab4fa9895e6a6c85777ea Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:06:10 +0100 Subject: [PATCH 07/38] feat(vue/nuxt)!: No longer create `"update"` spans for component tracking by default (#14602) Resolves https://github.com/getsentry/sentry-javascript/issues/12851 --- docs/migration/draft-v9-migration-guide.md | 22 ++++++++++++++++++++++ packages/vue/src/constants.ts | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index acf268c81ef4..461b740d06b7 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -107,6 +107,28 @@ }); ``` +## `@sentry/nuxt` and `@sentry/vue` + +- When component tracking is enabled, "update" spans are no longer created by default. + Add an `"update"` item to the `tracingOptions.hooks` option via the `vueIntegration()` to restore this behavior. + + ```ts + Sentry.init({ + integrations: [ + Sentry.vueIntegration({ + tracingOptions: { + trackComponents: true, + hooks: [ + 'mount', + 'update', // <-- + 'unmount', + ], + }, + }), + ], + }); + ``` + ## `@sentry/astro` - Deprecated passing `dsn`, `release`, `environment`, `sampleRate`, `tracesSampleRate`, `replaysSessionSampleRate` to the integration. Use the runtime-specific `Sentry.init()` calls for passing these options instead. diff --git a/packages/vue/src/constants.ts b/packages/vue/src/constants.ts index e254d988c40c..50aa82f77885 100644 --- a/packages/vue/src/constants.ts +++ b/packages/vue/src/constants.ts @@ -1,3 +1,3 @@ import type { Operation } from './types'; -export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount', 'update']; +export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount']; From d9ee102335b13173ee8d4d375aa7d71de6db9aed Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:43:05 +0100 Subject: [PATCH 08/38] feat(nextjs)!: Remove `experimental_captureRequestError` (#14607) Fixes https://github.com/getsentry/sentry-javascript/issues/14302 --- .../e2e-tests/test-applications/nextjs-t3/package.json | 6 +++--- .../test-applications/nextjs-t3/src/trpc/server.ts | 7 +++---- packages/nextjs/src/common/captureRequestError.ts | 8 -------- packages/nextjs/src/common/index.ts | 3 +-- packages/nextjs/src/index.types.ts | 3 +-- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json index 2fd54b440e2e..4cdd6509ddbd 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json @@ -21,9 +21,9 @@ "@trpc/react-query": "^11.0.0-rc.446", "@trpc/server": "^11.0.0-rc.446", "geist": "^1.3.0", - "next": "^14.2.4", - "react": "^18.3.1", - "react-dom": "^18.3.1", + "next": "14.2.4", + "react": "18.3.1", + "react-dom": "18.3.1", "server-only": "^0.0.1", "superjson": "^2.2.1", "zod": "^3.23.3" diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts index b6cb13a70781..7760873eb51d 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts @@ -2,7 +2,6 @@ import 'server-only'; import { createHydrationHelpers } from '@trpc/react-query/rsc'; import { headers } from 'next/headers'; -import { cache } from 'react'; import { type AppRouter, createCaller } from '~/server/api/root'; import { createTRPCContext } from '~/server/api/trpc'; @@ -12,16 +11,16 @@ import { createQueryClient } from './query-client'; * This wraps the `createTRPCContext` helper and provides the required context for the tRPC API when * handling a tRPC call from a React Server Component. */ -const createContext = cache(() => { +const createContext = () => { const heads = new Headers(headers()); heads.set('x-trpc-source', 'rsc'); return createTRPCContext({ headers: heads, }); -}); +}; -const getQueryClient = cache(createQueryClient); +const getQueryClient = createQueryClient; const caller = createCaller(createContext); export const { trpc: api, HydrateClient } = createHydrationHelpers(caller, getQueryClient); diff --git a/packages/nextjs/src/common/captureRequestError.ts b/packages/nextjs/src/common/captureRequestError.ts index 6de33ad11a8e..26fdaab4953b 100644 --- a/packages/nextjs/src/common/captureRequestError.ts +++ b/packages/nextjs/src/common/captureRequestError.ts @@ -41,11 +41,3 @@ export function captureRequestError(error: unknown, request: RequestInfo, errorC }); }); } - -/** - * Reports errors passed to the the Next.js `onRequestError` instrumentation hook. - * - * @deprecated Use `captureRequestError` instead. - */ -// TODO(v9): Remove this export -export const experimental_captureRequestError = captureRequestError; diff --git a/packages/nextjs/src/common/index.ts b/packages/nextjs/src/common/index.ts index 7740c35c016c..b9a652522349 100644 --- a/packages/nextjs/src/common/index.ts +++ b/packages/nextjs/src/common/index.ts @@ -11,5 +11,4 @@ export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; export { wrapPageComponentWithSentry } from './pages-router-instrumentation/wrapPageComponentWithSentry'; export { wrapGenerationFunctionWithSentry } from './wrapGenerationFunctionWithSentry'; export { withServerActionInstrumentation } from './withServerActionInstrumentation'; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './captureRequestError'; +export { captureRequestError } from './captureRequestError'; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 1b6a0e09ed85..1b965828116f 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -142,5 +142,4 @@ export declare function wrapApiHandlerWithSentryVercelCrons(WrappingTarget: C): C; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './common/captureRequestError'; +export { captureRequestError } from './common/captureRequestError'; From 32a1cffdcf2984c3d4a84ed5605ed4661c93f11d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 11 Dec 2024 17:48:02 +0100 Subject: [PATCH 09/38] meta(v9): Add v9 migration guide (#14296) --- MIGRATION.md | 1 + docs/migration/v8-to-v9.md | 159 +++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 docs/migration/v8-to-v9.md diff --git a/MIGRATION.md b/MIGRATION.md index 78657e360d2b..b142902cd6d1 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -5,6 +5,7 @@ These docs walk through how to migrate our JavaScript SDKs through different maj - Upgrading from [SDK 4.x to 5.x/6.x](./docs/migration/v4-to-v5_v6.md) - Upgrading from [SDK 6.x to 7.x](./docs/migration/v6-to-v7.md) - Upgrading from [SDK 7.x to 8.x](./MIGRATION.md#upgrading-from-7x-to-8x) +- Upgrading from [SDK 8.x to 9.x](./docs/migration/v8-to-v9.md) (Work in Progress - v9 is not released yet) # Upgrading from 7.x to 8.x diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md new file mode 100644 index 000000000000..baa14f19f8a4 --- /dev/null +++ b/docs/migration/v8-to-v9.md @@ -0,0 +1,159 @@ +# Upgrading from 8.x to 9.x + +**DISCLAIMER: THIS MIGRATION GUIDE IS WORK IN PROGRESS** + +Version 9 of the Sentry SDK concerns itself with API cleanup and compatibility updates. +This update contains behavioral changes that will not be caught by TypeScript or linters, so we recommend carefully reading the section on [Behavioral Changes](#2-behavior-changes). + +Before updating to version `9.x` of the SDK, we recommend upgrading to the latest version of `8.x`. +You can then go through the [Deprecations in 8.x](#deprecations-in-8x) and remove and migrate usages of deprecated APIs in your code before upgrading to `9.x`. + +Version 9 of the JavaScript SDK is compatible with Sentry self-hosted versions 24.4.2 or higher (unchanged from last major). +Lower versions may continue to work, but may not support all features. + +## 1. Version Support Changes: + +Version 9 of the Sentry SDK has new compatibility ranges for runtimes and frameworks. +We periodically update the compatibility ranges in major versions to increase reliability and quality of APIs and instrumentation data. + +### General Runtime Support Changes + +**ECMAScript Version:** All of the JavaScript code in the Sentry SDK packages may now contain ECMAScript 2020 features. +This includes features like Nullish Coalescing (`??`), Optional Chaining (`?.`), `String.matchAll()`, Logical Assignment Operators (`&&=`, `||=`, `??=`), and `Promise.allSettled()`. + +If you observe failures due to syntax or features listed above, it may be an indicator that your current runtime does not support ES2020. +If your runtime does not support ES2020, we recommend transpiling the SDK using Babel or similar tooling. + +**Node.js:** The minimum supported Node.js versions are TBD, TBD, and TBD. +We no longer test against Node TBD, TBD, or TBD and cannot guarantee that the SDK will work as expected on these versions. + +**Browsers:** Due to SDK code now including ES2020 features, the minimum supported browser list now looks as follows: + +- Chrome 80 +- Edge 80 +- Safari 14, iOS Safari 14.4 +- Firefox 74 +- Opera 67 +- Samsung Internet 13.0 + +If you need to support older browsers, we recommend transpiling your code using Babel or similar tooling. + +### Framework Support Changes + +**Angular:** TBD + +**Ember:** TBD + +**Next.js:** TBD + +**Nuxt:** TBD + +**React:** TBD + +**Vue:** TBD + +**Astro:** TBD + +**Gatsby:** TBD + +**NestJS:** TBD + +**Svelte:** TBD + +**SvelteKit:** TBD + +**Bun:** TBD + +**Cloudflare Workers:** TBD + +**Deno:** TBD + +**Solid:** TBD + +**SolidStart:** TBD + +**GCP Functions:** TBD + +**AWS Lambda:** TBD + +## 2. Behavior Changes + +- Next.js withSentryConfig returning Promise +- `request` on sdk processing metadata will be ignored going forward +- respect sourcemap generation settings +- SDK init options undefined +- no more polyfills +- no more update spans in vue component tracking by default +- new propagation context +- Client & Scope renaming + +## 3. Package Removals + +As part of an architectural cleanup we deprecated the following packages: + +- `@sentry/utils` +- `@sentry/types` + +All of these packages exports and APIs have been moved into the `@sentry/core` package. + +The `@sentry/utils` package will no longer be published. + +The `@sentry/types` package will continue to be published but it is deprecated and we don't plan on extending its APi. +You may experience slight compatibility issues in the future by using it. +We decided to keep this package around to temporarily lessen the upgrade burden. +It will be removed in a future major version. + +## 4. Removal of Deprecated APIs + +- [General](#general) +- [Server-side SDKs (Node, Deno, Bun, ...)](#server-side-sdks-node-deno-bun-) +- [Next.js SDK](#nextjs-sdk) +- [Vue/Nuxt SDK](#vuenuxt-sdk) + +### General + +- sessionTimingIntegration +- debugIntegration +- `Request` type +- spanid on propagation context +- makeFifoCache in utils + +### Server-side SDKs (Node, Deno, Bun, ...) + +- processThreadBreadcrumbIntegration +- NestJS stuff in Node sdk +- various NestJS APIs +- NestJS `@WithSentry` +- `AddRequestDataToEventOptions.transaction` + +### Next.js SDK + +- `experimental_captureRequestError` + +### Vue/Nuxt SDK + +- vueComponent tracking options + +## 5. Build Changes + +Previously the CJS versions of the SDK code (wrongfully) contained compatibility statements for default exports in ESM: + +```js +Object.defineProperty(exports, '__esModule', { value: true }); +``` + +The SDK no longer contains these statements. +Let us know if this is causing issues in your setup by opening an issue on GitHub. + +# Deprecations in 8.x + +TBD (Copy over from migrations list we collected) + +# No Version Support Timeline + +Version support timelines are stressful for anybody using the SDK, so we won't be defining one. +Instead, we will be applying bug fixes and features to older versions as long as there is demand for them. +We also hold ourselves to high standards security-wise, meaning that if any vulnerabilities are found, we will in almost all cases backport them. + +Note, that we will decide on a case-per-case basis, what gets backported or not. +If you need a fix or feature in a previous version of the SDK, feel free to reach out via a GitHub issue. From 3add6cdce8e7b13128f5615d59c6ca30e2c43030 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 12 Dec 2024 14:56:05 +0100 Subject: [PATCH 10/38] feat(node)!: Remove `processThreadBreadcrumbIntegration` (#14666) Resolves https://github.com/getsentry/sentry-javascript/issues/14277 --- .../node-exports-test-app/scripts/consistentExports.ts | 2 -- packages/astro/src/index.server.ts | 2 -- packages/aws-serverless/src/index.ts | 2 -- packages/google-cloud-serverless/src/index.ts | 2 -- packages/node/src/index.ts | 3 +-- packages/node/src/integrations/childProcess.ts | 7 ------- 6 files changed, 1 insertion(+), 17 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts index 83f9c1639cdc..8c3e51b14024 100644 --- a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts +++ b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts @@ -50,8 +50,6 @@ const DEPENDENTS: Dependent[] = [ ignoreExports: [ // not supported in bun: 'NodeClient', - // Bun doesn't emit the required diagnostics_channel events - 'processThreadBreadcrumbIntegration', 'childProcessIntegration', ], }, diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 7eca9de9a41a..c628bb7605ff 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -97,8 +97,6 @@ export { parameterize, postgresIntegration, prismaIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, redisIntegration, requestDataIntegration, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 3f167b62a7e3..a4f7f378ed8b 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -110,8 +110,6 @@ export { setupNestErrorHandler, postgresIntegration, prismaIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, hapiIntegration, setupHapiErrorHandler, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 6f89769c2a37..d23c78f412d9 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -123,8 +123,6 @@ export { zodErrorsIntegration, profiler, amqplibIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, } from '@sentry/node'; diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index fa16ac4e6b3d..71b1b80ffe82 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -34,8 +34,7 @@ export { tediousIntegration } from './integrations/tracing/tedious'; export { genericPoolIntegration } from './integrations/tracing/genericPool'; export { dataloaderIntegration } from './integrations/tracing/dataloader'; export { amqplibIntegration } from './integrations/tracing/amqplib'; -// eslint-disable-next-line deprecation/deprecation -export { processThreadBreadcrumbIntegration, childProcessIntegration } from './integrations/childProcess'; +export { childProcessIntegration } from './integrations/childProcess'; export { SentryContextManager } from './otel/contextManager'; export { generateInstrumentOnce } from './otel/instrument'; diff --git a/packages/node/src/integrations/childProcess.ts b/packages/node/src/integrations/childProcess.ts index 99525b4092b4..e78cc843f279 100644 --- a/packages/node/src/integrations/childProcess.ts +++ b/packages/node/src/integrations/childProcess.ts @@ -39,13 +39,6 @@ export const childProcessIntegration = defineIntegration((options: Options = {}) }; }); -/** - * Capture breadcrumbs for child processes and worker threads. - * - * @deprecated Use `childProcessIntegration` integration instead. Functionally they are the same. `processThreadBreadcrumbIntegration` will be removed in the next major version. - */ -export const processThreadBreadcrumbIntegration = childProcessIntegration; - function captureChildProcessEvents(child: ChildProcess, options: Options): void { let hasExited = false; let data: Record | undefined; From 8b81476c765ec29e79f431515e3e8a3b439fad32 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 13 Dec 2024 09:19:11 +0100 Subject: [PATCH 11/38] feat(node)!: Avoid http spans by default for custom OTEL setups (#14678) With this PR, the default value for the `spans` option in the `httpIntegration` is changed to `false`, if `skipOpenTelemetrySetup: true` is configured. This is what you'd expect as a user, you do not want Sentry to register any OTEL instrumentation and emit any spans in this scenario. Closes https://github.com/getsentry/sentry-javascript/issues/14675 --- .../node-otel-custom-sampler/src/instrument.ts | 1 + .../node-otel-sdk-node/src/instrument.ts | 1 + .../src/instrument.ts | 3 --- docs/migration/draft-v9-migration-guide.md | 1 + docs/migration/v8-to-v9.md | 6 ++++++ packages/node/src/integrations/http/index.ts | 14 +++++++++++++- packages/node/test/integrations/http.test.ts | 18 ++++++++++++++++++ 7 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 packages/node/test/integrations/http.test.ts diff --git a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts index b7279c9942a7..d0aed916864b 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts @@ -14,6 +14,7 @@ Sentry.init({ skipOpenTelemetrySetup: true, // By defining _any_ sample rate, tracing integrations will be added by default tracesSampleRate: 0, + integrations: [Sentry.httpIntegration({ spans: true })], }); const provider = new NodeTracerProvider({ diff --git a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts index fb270e1252d3..5cb2e5570db9 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts @@ -14,6 +14,7 @@ const sentryClient = Sentry.init({ tracesSampleRate: 1, skipOpenTelemetrySetup: true, + integrations: [Sentry.httpIntegration({ spans: true })], }); const sdk = new opentelemetry.NodeSDK({ diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index 47078a504e18..ebabd499fee5 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts @@ -15,9 +15,6 @@ Sentry.init({ debug: !!process.env.DEBUG, tunnel: `http://localhost:3031/`, // proxy server // Tracing is completely disabled - - integrations: [Sentry.httpIntegration({ spans: false })], - // Custom OTEL setup skipOpenTelemetrySetup: true, }); diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index 461b740d06b7..f731770fe142 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -158,3 +158,4 @@ - Deprecated `addOpenTelemetryInstrumentation`. Use the `openTelemetryInstrumentations` option in `Sentry.init()` or your custom Sentry Client instead. - Deprecated `registerEsmLoaderHooks.include` and `registerEsmLoaderHooks.exclude`. Set `onlyIncludeInstrumentedModules: true` instead. - `registerEsmLoaderHooks` will only accept `true | false | undefined` in the future. The SDK will default to wrapping modules that are used as part of OpenTelemetry Instrumentation. +- `httpIntegration({ spans: false })` is configured by default if `skipOpenTelemetrySetup: true` is set. You can still overwrite this if desired. diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index baa14f19f8a4..bb0cfe487da0 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -78,6 +78,12 @@ If you need to support older browsers, we recommend transpiling your code using ## 2. Behavior Changes +### `@sentry/node` + +- When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed. + +### Uncategorized (TODO) + - Next.js withSentryConfig returning Promise - `request` on sdk processing metadata will be ignored going forward - respect sourcemap generation settings diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index c976bb4da2a1..f06e12979074 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -8,6 +8,7 @@ import { getClient } from '@sentry/opentelemetry'; import { generateInstrumentOnce } from '../../otel/instrument'; import type { NodeClient } from '../../sdk/client'; import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; +import type { NodeClientOptions } from '../../types'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { getRequestUrl } from '../../utils/getRequestUrl'; import { SentryHttpInstrumentation } from './SentryHttpInstrumentation'; @@ -27,6 +28,8 @@ interface HttpOptions { * If set to false, do not emit any spans. * This will ensure that the default HttpInstrumentation from OpenTelemetry is not setup, * only the Sentry-specific instrumentation for request isolation is applied. + * + * If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`. */ spans?: boolean; @@ -118,12 +121,21 @@ export const instrumentOtelHttp = generateInstrumentOnce = {}): boolean { + // If `spans` is passed in, it takes precedence + // Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` + return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup; +} + /** * Instrument the HTTP and HTTPS modules. */ const instrumentHttp = (options: HttpOptions = {}): void => { + const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); + // This is the "regular" OTEL instrumentation that emits spans - if (options.spans !== false) { + if (instrumentSpans) { const instrumentationConfig = getConfigWithDefaults(options); instrumentOtelHttp(instrumentationConfig); } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts new file mode 100644 index 000000000000..a6a8e612c019 --- /dev/null +++ b/packages/node/test/integrations/http.test.ts @@ -0,0 +1,18 @@ +import { _shouldInstrumentSpans } from '../../src/integrations/http'; + +describe('httpIntegration', () => { + describe('_shouldInstrumentSpans', () => { + it.each([ + [{}, {}, true], + [{ spans: true }, {}, true], + [{ spans: false }, {}, false], + [{ spans: true }, { skipOpenTelemetrySetup: true }, true], + [{ spans: false }, { skipOpenTelemetrySetup: true }, false], + [{}, { skipOpenTelemetrySetup: true }, false], + [{}, { skipOpenTelemetrySetup: false }, true], + ])('returns the correct value for options=%p and clientOptions=%p', (options, clientOptions, expected) => { + const actual = _shouldInstrumentSpans(options, clientOptions); + expect(actual).toBe(expected); + }); + }); +}); From 1c9544bb25fe7a7ee0731739fa634172949ca853 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 11:01:57 +0100 Subject: [PATCH 12/38] ref!: Don't polyfill optional chaining and nullish coalescing (#14603) --- dev-packages/rollup-utils/npmHelpers.mjs | 6 - .../plugins/extractPolyfillsPlugin.mjs | 214 ------------------ .../rollup-utils/plugins/npmPlugins.mjs | 2 - dev-packages/test-utils/.eslintrc.js | 8 - packages/astro/.eslintrc.cjs | 7 - packages/aws-serverless/.eslintrc.js | 3 - packages/bun/.eslintrc.js | 2 - packages/cloudflare/.eslintrc.js | 2 - .../src/utils-hoist/buildPolyfills/README.md | 30 --- .../buildPolyfills/_asyncNullishCoalesce.ts | 51 ----- .../buildPolyfills/_asyncOptionalChain.ts | 82 ------- .../_asyncOptionalChainDelete.ts | 51 ----- .../buildPolyfills/_nullishCoalesce.ts | 49 ---- .../buildPolyfills/_optionalChain.ts | 82 ------- .../buildPolyfills/_optionalChainDelete.ts | 52 ----- .../src/utils-hoist/buildPolyfills/index.ts | 6 - .../src/utils-hoist/buildPolyfills/types.ts | 7 - packages/core/src/utils-hoist/index.ts | 7 - .../buildPolyfills/nullishCoalesce.test.ts | 31 --- .../buildPolyfills/optionalChain.test.ts | 92 -------- .../utils-hoist/buildPolyfills/originals.ts | 85 ------- packages/deno/.eslintrc.js | 2 - packages/eslint-config-sdk/src/base.js | 7 - packages/eslint-plugin-sdk/src/index.js | 2 - .../src/rules/no-nullish-coalescing.js | 48 ---- .../src/rules/no-optional-chaining.js | 61 ----- packages/google-cloud-serverless/.eslintrc.js | 3 - packages/nextjs/.eslintrc.js | 4 - packages/nitro-utils/.eslintrc.js | 15 -- packages/node/.eslintrc.js | 2 - packages/nuxt/.eslintrc.js | 7 - packages/opentelemetry/.eslintrc.js | 3 - packages/profiling-node/.eslintrc.js | 2 - packages/remix/.eslintrc.js | 3 - packages/solidstart/.eslintrc.js | 7 - packages/sveltekit/.eslintrc.js | 6 - packages/sveltekit/src/vite/autoInstrument.ts | 1 - packages/sveltekit/src/vite/svelteConfig.ts | 2 - packages/utils/src/index.ts | 24 -- packages/vercel-edge/.eslintrc.js | 2 - packages/wasm/.eslintrc.js | 3 - 41 files changed, 1073 deletions(-) delete mode 100644 dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/README.md delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/index.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/types.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/originals.ts delete mode 100644 packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js delete mode 100644 packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js diff --git a/dev-packages/rollup-utils/npmHelpers.mjs b/dev-packages/rollup-utils/npmHelpers.mjs index 4e6483364ee4..4cb8deaa61e0 100644 --- a/dev-packages/rollup-utils/npmHelpers.mjs +++ b/dev-packages/rollup-utils/npmHelpers.mjs @@ -15,7 +15,6 @@ import { defineConfig } from 'rollup'; import { makeCleanupPlugin, makeDebugBuildStatementReplacePlugin, - makeExtractPolyfillsPlugin, makeImportMetaUrlReplacePlugin, makeNodeResolvePlugin, makeRrwebBuildPlugin, @@ -44,7 +43,6 @@ export function makeBaseNPMConfig(options = {}) { const debugBuildStatementReplacePlugin = makeDebugBuildStatementReplacePlugin(); const importMetaUrlReplacePlugin = makeImportMetaUrlReplacePlugin(); const cleanupPlugin = makeCleanupPlugin(); - const extractPolyfillsPlugin = makeExtractPolyfillsPlugin(); const rrwebBuildPlugin = makeRrwebBuildPlugin({ excludeShadowDom: undefined, excludeIframe: undefined, @@ -121,10 +119,6 @@ export function makeBaseNPMConfig(options = {}) { ], }; - if (addPolyfills) { - defaultBaseConfig.plugins.push(extractPolyfillsPlugin); - } - return deepMerge(defaultBaseConfig, packageSpecificConfig, { // Plugins have to be in the correct order or everything breaks, so when merging we have to manually re-order them customMerge: key => (key === 'plugins' ? mergePlugins : undefined), diff --git a/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs b/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs deleted file mode 100644 index e0a21b400f35..000000000000 --- a/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs +++ /dev/null @@ -1,214 +0,0 @@ -import * as path from 'path'; - -import * as acorn from 'acorn'; -import * as recast from 'recast'; - -const POLYFILL_NAMES = new Set([ - '_asyncNullishCoalesce', - '_asyncOptionalChain', - '_asyncOptionalChainDelete', - '_nullishCoalesce', - '_optionalChain', - '_optionalChainDelete', -]); - -/** - * Create a plugin which will replace function definitions of any of the above functions with an `import` or `require` - * statement pulling them in from a central source. Mimics tsc's `importHelpers` option. - */ -export function makeExtractPolyfillsPlugin() { - let moduleFormat; - - // For more on the hooks used in this plugin, see https://rollupjs.org/guide/en/#output-generation-hooks - return { - name: 'extractPolyfills', - - // Figure out which build we're currently in (esm or cjs) - outputOptions(options) { - moduleFormat = options.format; - }, - - // This runs after both the sucrase transpilation (which happens in the `transform` hook) and rollup's own - // esm-i-fying or cjs-i-fying work (which happens right before `renderChunk`), in other words, after all polyfills - // will have been injected - renderChunk(code, chunk) { - const sourceFile = chunk.fileName; - - // We don't want to pull the function definitions out of their actual sourcefiles, just the places where they've - // been injected - if (sourceFile.includes('buildPolyfills')) { - return null; - } - - // The index.js file of the core package will include identifiers named after polyfills so we would inject the - // polyfills, however that would override the exports so we should just skip that file. - const isCorePackage = process.cwd().endsWith(`packages${path.sep}core`); - if (isCorePackage && sourceFile === 'index.js') { - return null; - } - - const parserOptions = { - sourceFileName: sourceFile, - // We supply a custom parser which wraps the provided `acorn` parser in order to override the `ecmaVersion` value. - // See https://github.com/benjamn/recast/issues/578. - parser: { - parse(source, options) { - return acorn.parse(source, { - ...options, - // By this point in the build, everything should already have been down-compiled to whatever JS version - // we're targeting. Setting this parser to `latest` just means that whatever that version is (or changes - // to in the future), this parser will be able to handle the generated code. - ecmaVersion: 'latest', - }); - }, - }, - }; - - const ast = recast.parse(code, parserOptions); - - // Find function definitions and function expressions whose identifiers match a known polyfill name - const polyfillNodes = findPolyfillNodes(ast); - - if (polyfillNodes.length === 0) { - return null; - } - - console.log(`${sourceFile} - polyfills: ${polyfillNodes.map(node => node.name)}`); - - // Depending on the output format, generate `import { x, y, z } from '...'` or `var { x, y, z } = require('...')` - const importOrRequireNode = createImportOrRequireNode(polyfillNodes, sourceFile, moduleFormat); - - // Insert our new `import` or `require` node at the top of the file, and then delete the function definitions it's - // meant to replace (polyfill nodes get marked for deletion in `findPolyfillNodes`) - ast.program.body = [importOrRequireNode, ...ast.program.body.filter(node => !node.shouldDelete)]; - - // In spite of the name, this doesn't actually print anything - it just stringifies the code, and keeps track of - // where original nodes end up in order to generate a sourcemap. - const result = recast.print(ast, { - sourceMapName: `${sourceFile}.map`, - quote: 'single', - }); - - return { code: result.code, map: result.map }; - }, - }; -} - -/** - * Extract the function name, regardless of the format in which the function is declared - */ -function getNodeName(node) { - // Function expressions and functions pulled from objects - if (node.type === 'VariableDeclaration') { - // In practice sucrase and rollup only ever declare one polyfill at a time, so it's safe to just grab the first - // entry here - const declarationId = node.declarations[0].id; - - // Note: Sucrase and rollup seem to only use the first type of variable declaration for their polyfills, but good to - // cover our bases - - // Declarations of the form - // `const dogs = function() { return "are great"; };` - // or - // `const dogs = () => "are great"; - if (declarationId.type === 'Identifier') { - return declarationId.name; - } - // Declarations of the form - // `const { dogs } = { dogs: function() { return "are great"; } }` - // or - // `const { dogs } = { dogs: () => "are great" }` - else if (declarationId.type === 'ObjectPattern') { - return declarationId.properties[0].key.name; - } - // Any other format - else { - return 'unknown variable'; - } - } - - // Regular old functions, of the form - // `function dogs() { return "are great"; }` - else if (node.type === 'FunctionDeclaration') { - return node.id.name; - } - - // If we get here, this isn't a node we're interested in, so just return a string we know will never match any of the - // polyfill names - else { - return 'nope'; - } -} - -/** - * Find all nodes whose identifiers match a known polyfill name. - * - * Note: In theory, this could yield false positives, if any of the magic names were assigned to something other than a - * polyfill function, but the chances of that are slim. Also, it only searches the module global scope, but that's - * always where the polyfills appear, so no reason to traverse the whole tree. - */ -function findPolyfillNodes(ast) { - const isPolyfillNode = node => { - const nodeName = getNodeName(node); - if (POLYFILL_NAMES.has(nodeName)) { - // Mark this node for later deletion, since we're going to replace it with an import statement - node.shouldDelete = true; - // Store the name in a consistent spot, regardless of node type - node.name = nodeName; - - return true; - } - - return false; - }; - - return ast.program.body.filter(isPolyfillNode); -} - -/** - * Create a node representing an `import` or `require` statement of the form - * - * import { < polyfills > } from '...' - * or - * var { < polyfills > } = require('...') - * - * @param polyfillNodes The nodes from the current version of the code, defining the polyfill functions - * @param currentSourceFile The path, relative to `src/`, of the file currently being transpiled - * @param moduleFormat Either 'cjs' or 'esm' - * @returns A single node which can be subbed in for the polyfill definition nodes - */ -function createImportOrRequireNode(polyfillNodes, currentSourceFile, moduleFormat) { - const { - callExpression, - identifier, - importDeclaration, - importSpecifier, - literal, - objectPattern, - property, - variableDeclaration, - variableDeclarator, - } = recast.types.builders; - - // Since our polyfills live in `@sentry/core`, if we're importing or requiring them there the path will have to be - // relative - const isCorePackage = process.cwd().endsWith(path.join('packages', 'core')); - const importSource = literal( - isCorePackage ? `.${path.sep}${path.relative(path.dirname(currentSourceFile), 'buildPolyfills')}` : '@sentry/core', - ); - - // This is the `x, y, z` of inside of `import { x, y, z }` or `var { x, y, z }` - const importees = polyfillNodes.map(({ name: fnName }) => - moduleFormat === 'esm' - ? importSpecifier(identifier(fnName)) - : property.from({ kind: 'init', key: identifier(fnName), value: identifier(fnName), shorthand: true }), - ); - - const requireFn = identifier('require'); - - return moduleFormat === 'esm' - ? importDeclaration(importees, importSource) - : variableDeclaration('var', [ - variableDeclarator(objectPattern(importees), callExpression(requireFn, [importSource])), - ]); -} diff --git a/dev-packages/rollup-utils/plugins/npmPlugins.mjs b/dev-packages/rollup-utils/plugins/npmPlugins.mjs index 6597e2244ab8..5f577507b102 100644 --- a/dev-packages/rollup-utils/plugins/npmPlugins.mjs +++ b/dev-packages/rollup-utils/plugins/npmPlugins.mjs @@ -173,5 +173,3 @@ export function makeCodeCovPlugin() { uploadToken: process.env.CODECOV_TOKEN, }); } - -export { makeExtractPolyfillsPlugin } from './extractPolyfillsPlugin.mjs'; diff --git a/dev-packages/test-utils/.eslintrc.js b/dev-packages/test-utils/.eslintrc.js index 98318aea5c41..fdb9952bae52 100644 --- a/dev-packages/test-utils/.eslintrc.js +++ b/dev-packages/test-utils/.eslintrc.js @@ -3,12 +3,4 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - overrides: [ - { - files: ['**/*.ts'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, - ], }; diff --git a/packages/astro/.eslintrc.cjs b/packages/astro/.eslintrc.cjs index c706032aaf35..29b78099e7c6 100644 --- a/packages/astro/.eslintrc.cjs +++ b/packages/astro/.eslintrc.cjs @@ -11,12 +11,5 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/integration/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], }; diff --git a/packages/aws-serverless/.eslintrc.js b/packages/aws-serverless/.eslintrc.js index 99fcba0976da..d1d4c4e12aa0 100644 --- a/packages/aws-serverless/.eslintrc.js +++ b/packages/aws-serverless/.eslintrc.js @@ -3,9 +3,6 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/bun/.eslintrc.js b/packages/bun/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/bun/.eslintrc.js +++ b/packages/bun/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/cloudflare/.eslintrc.js b/packages/cloudflare/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/cloudflare/.eslintrc.js +++ b/packages/cloudflare/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/core/src/utils-hoist/buildPolyfills/README.md b/packages/core/src/utils-hoist/buildPolyfills/README.md deleted file mode 100644 index 3b18ad989133..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/README.md +++ /dev/null @@ -1,30 +0,0 @@ -## Build Polyfills - -This is a collection of syntax and import/export polyfills either copied directly from or heavily inspired by those used -by [Rollup](https://github.com/rollup/rollup) and [Sucrase](https://github.com/alangpierce/sucrase). When either tool -uses one of these polyfills during a build, it injects the function source code into each file needing the function, -which can lead to a great deal of duplication. For our builds, we have therefore implemented something similar to -[`tsc`'s `importHelpers` behavior](https://www.typescriptlang.org/tsconfig#importHelpers): Instead of leaving the -polyfills injected in multiple places, we instead replace each injected function with an `import` or `require` -statement. - -Note that not all polyfills are currently used by the SDK, but all are included here for future compatibility, should -they ever be needed. Also, since we're never going to be calling these directly from within another TS file, their types -are fairly generic. In some cases testing required more specific types, which can be found in the test files. - ---- - -_Code from both Rollup and Sucrase is used under the MIT license, copyright 2017 and 2012-2018, respectively._ - -_Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated -documentation files (the "Software"), to deal in the Software without restriction, including without limitation the -rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit -persons to whom the Software is furnished to do so, subject to the following conditions:_ - -_The above copyright notice and this permission notice shall be included in all copies or substantial portions of the -Software._ - -_THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE -WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR -COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE._ diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts deleted file mode 100644 index 032b31011f96..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts +++ /dev/null @@ -1,51 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _nullishCoalesce } from './_nullishCoalesce'; - -/** - * Polyfill for the nullish coalescing operator (`??`), when used in situations where at least one of the values is the - * result of an async operation. - * - * Note that the RHS is wrapped in a function so that if it's a computed value, that evaluation won't happen unless the - * LHS evaluates to a nullish value, to mimic the operator's short-circuiting behavior. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * - * @param lhs The value of the expression to the left of the `??` - * @param rhsFn A function returning the value of the expression to the right of the `??` - * @returns The LHS value, unless it's `null` or `undefined`, in which case, the RHS value - */ -export async function _asyncNullishCoalesce(lhs: unknown, rhsFn: () => unknown): Promise { - return _nullishCoalesce(lhs, rhsFn); -} - -// Sucrase version: -// async function _asyncNullishCoalesce(lhs, rhsFn) { -// if (lhs != null) { -// return lhs; -// } else { -// return await rhsFn(); -// } -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts deleted file mode 100644 index 37489b5c9232..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts +++ /dev/null @@ -1,82 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import type { GenericFunction } from './types'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, for situations in which at least one part of the expression is async. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The value of the expression - */ -export async function _asyncOptionalChain(ops: unknown[]): Promise { - let lastAccessLHS: unknown = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i] as string; - const fn = ops[i + 1] as (intermediateValue: unknown) => Promise; - i += 2; - // by checking for loose equality to `null`, we catch both `null` and `undefined` - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - // really we're meaning to return `undefined` as an actual value here, but it saves bytes not to write it - return; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = await fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = await fn((...args: unknown[]) => (value as GenericFunction).call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// Sucrase version: -// async function _asyncOptionalChain(ops) { -// let lastAccessLHS = undefined; -// let value = ops[0]; -// let i = 1; -// while (i < ops.length) { -// const op = ops[i]; -// const fn = ops[i + 1]; -// i += 2; -// if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { -// return undefined; -// } -// if (op === 'access' || op === 'optionalAccess') { -// lastAccessLHS = value; -// value = await fn(value); -// } else if (op === 'call' || op === 'optionalCall') { -// value = await fn((...args) => value.call(lastAccessLHS, ...args)); -// lastAccessLHS = undefined; -// } -// } -// return value; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts deleted file mode 100644 index 9cef4fd791f0..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts +++ /dev/null @@ -1,51 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _asyncOptionalChain } from './_asyncOptionalChain'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, in cases where the value of the expression is to be deleted. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The return value of the `delete` operator: `true`, unless the deletion target is an own, non-configurable - * property (one which can't be deleted or turned into an accessor, and whose enumerability can't be changed), in which - * case `false`. - */ -export async function _asyncOptionalChainDelete(ops: unknown[]): Promise { - const result = (await _asyncOptionalChain(ops)) as Promise; - // If `result` is `null`, it means we didn't get to the end of the chain and so nothing was deleted (in which case, - // return `true` since that's what `delete` does when it no-ops). If it's non-null, we know the delete happened, in - // which case we return whatever the `delete` returned, which will be a boolean. - return result == null ? true : (result as Promise); -} - -// Sucrase version: -// async function asyncOptionalChainDelete(ops) { -// const result = await ASYNC_OPTIONAL_CHAIN_NAME(ops); -// return result == null ? true : result; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts b/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts deleted file mode 100644 index a11cd469bf11..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts +++ /dev/null @@ -1,49 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -/** - * Polyfill for the nullish coalescing operator (`??`). - * - * Note that the RHS is wrapped in a function so that if it's a computed value, that evaluation won't happen unless the - * LHS evaluates to a nullish value, to mimic the operator's short-circuiting behavior. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * - * @param lhs The value of the expression to the left of the `??` - * @param rhsFn A function returning the value of the expression to the right of the `??` - * @returns The LHS value, unless it's `null` or `undefined`, in which case, the RHS value - */ -export function _nullishCoalesce(lhs: unknown, rhsFn: () => unknown): unknown { - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return lhs != null ? lhs : rhsFn(); -} - -// Sucrase version: -// function _nullishCoalesce(lhs, rhsFn) { -// if (lhs != null) { -// return lhs; -// } else { -// return rhsFn(); -// } -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts b/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts deleted file mode 100644 index a7ea8338d744..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts +++ /dev/null @@ -1,82 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import type { GenericFunction } from './types'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * See https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The value of the expression - */ -export function _optionalChain(ops: unknown[]): unknown { - let lastAccessLHS: unknown = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i] as string; - const fn = ops[i + 1] as (intermediateValue: unknown) => unknown; - i += 2; - // by checking for loose equality to `null`, we catch both `null` and `undefined` - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - // really we're meaning to return `undefined` as an actual value here, but it saves bytes not to write it - return; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = fn((...args: unknown[]) => (value as GenericFunction).call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// Sucrase version -// function _optionalChain(ops) { -// let lastAccessLHS = undefined; -// let value = ops[0]; -// let i = 1; -// while (i < ops.length) { -// const op = ops[i]; -// const fn = ops[i + 1]; -// i += 2; -// if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { -// return undefined; -// } -// if (op === 'access' || op === 'optionalAccess') { -// lastAccessLHS = value; -// value = fn(value); -// } else if (op === 'call' || op === 'optionalCall') { -// value = fn((...args) => value.call(lastAccessLHS, ...args)); -// lastAccessLHS = undefined; -// } -// } -// return value; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts b/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts deleted file mode 100644 index 04bffc8ab385..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts +++ /dev/null @@ -1,52 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _optionalChain } from './_optionalChain'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, in cases where the value of the expression is to be deleted. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The return value of the `delete` operator: `true`, unless the deletion target is an own, non-configurable - * property (one which can't be deleted or turned into an accessor, and whose enumerability can't be changed), in which - * case `false`. - */ -export function _optionalChainDelete(ops: unknown[]): boolean { - const result = _optionalChain(ops) as boolean | null; - // If `result` is `null`, it means we didn't get to the end of the chain and so nothing was deleted (in which case, - // return `true` since that's what `delete` does when it no-ops). If it's non-null, we know the delete happened, in - // which case we return whatever the `delete` returned, which will be a boolean. - return result == null ? true : result; -} - -// Sucrase version: -// function _optionalChainDelete(ops) { -// const result = _optionalChain(ops); -// // by checking for loose equality to `null`, we catch both `null` and `undefined` -// return result == null ? true : result; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/index.ts b/packages/core/src/utils-hoist/buildPolyfills/index.ts deleted file mode 100644 index 2017dcbd9592..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/index.ts +++ /dev/null @@ -1,6 +0,0 @@ -export { _asyncNullishCoalesce } from './_asyncNullishCoalesce'; -export { _asyncOptionalChain } from './_asyncOptionalChain'; -export { _asyncOptionalChainDelete } from './_asyncOptionalChainDelete'; -export { _nullishCoalesce } from './_nullishCoalesce'; -export { _optionalChain } from './_optionalChain'; -export { _optionalChainDelete } from './_optionalChainDelete'; diff --git a/packages/core/src/utils-hoist/buildPolyfills/types.ts b/packages/core/src/utils-hoist/buildPolyfills/types.ts deleted file mode 100644 index 10e2f4a944a4..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/types.ts +++ /dev/null @@ -1,7 +0,0 @@ -import type { Primitive } from '../../types-hoist'; - -export type GenericObject = { [key: string]: Value }; -export type GenericFunction = (...args: unknown[]) => Value; -export type Value = Primitive | GenericFunction | GenericObject; - -export type RequireResult = GenericObject | (GenericFunction & GenericObject); diff --git a/packages/core/src/utils-hoist/index.ts b/packages/core/src/utils-hoist/index.ts index e53cd0edb59b..befb934905a2 100644 --- a/packages/core/src/utils-hoist/index.ts +++ b/packages/core/src/utils-hoist/index.ts @@ -179,10 +179,3 @@ export { SDK_VERSION } from './version'; export { getDebugImagesForResources, getFilenameToDebugIdMap } from './debug-ids'; export { escapeStringForRegex } from './vendor/escapeStringForRegex'; export { supportsHistory } from './vendor/supportsHistory'; - -export { _asyncNullishCoalesce } from './buildPolyfills/_asyncNullishCoalesce'; -export { _asyncOptionalChain } from './buildPolyfills/_asyncOptionalChain'; -export { _asyncOptionalChainDelete } from './buildPolyfills/_asyncOptionalChainDelete'; -export { _nullishCoalesce } from './buildPolyfills/_nullishCoalesce'; -export { _optionalChain } from './buildPolyfills/_optionalChain'; -export { _optionalChainDelete } from './buildPolyfills/_optionalChainDelete'; diff --git a/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts b/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts deleted file mode 100644 index 11c83b0711d9..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -// TODO(v9): Remove this test - -import { _nullishCoalesce } from '../../../src/utils-hoist/buildPolyfills'; -import type { Value } from '../../../src/utils-hoist/buildPolyfills/types'; -import { _nullishCoalesce as _nullishCoalesceOrig } from './originals'; - -const dogStr = 'dogs are great!'; -const dogFunc = () => dogStr; -const dogAdjectives = { maisey: 'silly', charlie: 'goofy' }; -const dogAdjectiveFunc = () => dogAdjectives; - -describe('_nullishCoalesce', () => { - describe('returns the same result as the original', () => { - const testCases: Array<[string, Value, () => Value, Value]> = [ - ['null LHS', null, dogFunc, dogStr], - ['undefined LHS', undefined, dogFunc, dogStr], - ['false LHS', false, dogFunc, false], - ['zero LHS', 0, dogFunc, 0], - ['empty string LHS', '', dogFunc, ''], - ['true LHS', true, dogFunc, true], - ['truthy primitive LHS', 12312012, dogFunc, 12312012], - ['truthy object LHS', dogAdjectives, dogFunc, dogAdjectives], - ['truthy function LHS', dogAdjectiveFunc, dogFunc, dogAdjectiveFunc], - ]; - - it.each(testCases)('%s', (_, lhs, rhs, expectedValue) => { - expect(_nullishCoalesce(lhs, rhs)).toEqual(_nullishCoalesceOrig(lhs, rhs)); - expect(_nullishCoalesce(lhs, rhs)).toEqual(expectedValue); - }); - }); -}); diff --git a/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts b/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts deleted file mode 100644 index bd7a7bb052fb..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -// TODO(v9): Remove this test - -import { shim as arrayFlatShim } from 'array.prototype.flat'; - -import { _optionalChain } from '../../../src/utils-hoist/buildPolyfills'; -import type { GenericFunction, GenericObject, Value } from '../../../src/utils-hoist/buildPolyfills/types'; -import { _optionalChain as _optionalChainOrig } from './originals'; - -// Older versions of Node don't have `Array.prototype.flat`, which crashes these tests. On newer versions that do have -// it, this is a no-op. -arrayFlatShim(); - -type OperationType = 'access' | 'call' | 'optionalAccess' | 'optionalCall'; -type OperationExecutor = - | ((intermediateValue: GenericObject) => Value) - | ((intermediateValue: GenericFunction) => Value); -type Operation = [OperationType, OperationExecutor]; - -const truthyObject = { maisey: 'silly', charlie: 'goofy' }; -const nullishObject = null; -const truthyFunc = (): GenericObject => truthyObject; -const nullishFunc = undefined; -const truthyReturn = (): GenericObject => truthyObject; -const nullishReturn = (): null => nullishObject; - -// The polyfill being tested here works under the assumption that the original code containing the optional chain has -// been transformed into an array of values, labels, and functions. For example, `truthyObject?.charlie` will have been -// transformed into `_optionalChain([truthyObject, 'optionalAccess', _ => _.charlie])`. We are not testing the -// transformation here, only what the polyfill does with the already-transformed inputs. - -describe('_optionalChain', () => { - describe('returns the same result as the original', () => { - // In these test cases, the array passed to `_optionalChain` has been broken up into the first entry followed by an - // array of pairs of subsequent elements, because this seemed the easiest way to express the type, which is really - // - // [Value, OperationType, Value => Value, OperationType, Value => Value, OperationType, Value => Value, ...]. - // - // (In other words, `[A, B, C, D, E]` has become `A, [[B, C], [D, E]]`, and these are then the second and third - // entries in each test case.) We then undo this wrapping before passing the data to our functions. - const testCases: Array<[string, Value, Operation[], Value]> = [ - ['truthyObject?.charlie', truthyObject, [['optionalAccess', (_: GenericObject) => _.charlie]], 'goofy'], - ['nullishObject?.maisey', nullishObject, [['optionalAccess', (_: GenericObject) => _.maisey]], undefined], - [ - 'truthyFunc?.().maisey', - truthyFunc, - [ - ['optionalCall', (_: GenericFunction) => _()], - ['access', (_: GenericObject) => _.maisey], - ], - 'silly', - ], - [ - 'nullishFunc?.().charlie', - nullishFunc, - [ - ['optionalCall', (_: GenericFunction) => _()], - ['access', (_: GenericObject) => _.charlie], - ], - undefined, - ], - [ - 'truthyReturn()?.maisey', - truthyReturn, - [ - ['call', (_: GenericFunction) => _()], - ['optionalAccess', (_: GenericObject) => _.maisey], - ], - 'silly', - ], - [ - 'nullishReturn()?.charlie', - nullishReturn, - [ - ['call', (_: GenericFunction) => _()], - ['optionalAccess', (_: GenericObject) => _.charlie], - ], - undefined, - ], - ]; - - it.each(testCases)('%s', (_, initialChainComponent, operations, expectedValue) => { - // `operations` is flattened and spread in order to undo the wrapping done in the test cases for TS purposes. - // @ts-expect-error this is what we're testing - expect(_optionalChain([initialChainComponent, ...operations.flat()])).toEqual( - // @ts-expect-error this is what we're testing - _optionalChainOrig([initialChainComponent, ...operations.flat()]), - ); - // @ts-expect-error this is what we're testing - expect(_optionalChain([initialChainComponent, ...operations.flat()])).toEqual(expectedValue); - }); - }); -}); diff --git a/packages/core/test/utils-hoist/buildPolyfills/originals.ts b/packages/core/test/utils-hoist/buildPolyfills/originals.ts deleted file mode 100644 index a504d5f0d871..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/originals.ts +++ /dev/null @@ -1,85 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck this is just used for tests - -// TODO(v9): Remove this file - -// Originals of the buildPolyfills from Sucrase and Rollup we use (which we have adapted in various ways), preserved here for testing, to prove that -// the modified versions do the same thing the originals do. - -// From Sucrase -export function _asyncNullishCoalesce(lhs, rhsFn) { - if (lhs != null) { - return lhs; - } else { - return rhsFn(); - } -} - -// From Sucrase -export async function _asyncOptionalChain(ops) { - let lastAccessLHS = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i]; - const fn = ops[i + 1]; - i += 2; - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - return undefined; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = await fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = await fn((...args) => value.call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// From Sucrase -export async function _asyncOptionalChainDelete(ops) { - const result = await _asyncOptionalChain(ops); - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return result == null ? true : result; -} - -// From Sucrase -export function _nullishCoalesce(lhs, rhsFn) { - if (lhs != null) { - return lhs; - } else { - return rhsFn(); - } -} - -// From Sucrase -export function _optionalChain(ops) { - let lastAccessLHS = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i]; - const fn = ops[i + 1]; - i += 2; - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - return undefined; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = fn((...args) => value.call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// From Sucrase -export function _optionalChainDelete(ops) { - const result = _optionalChain(ops); - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return result == null ? true : result; -} diff --git a/packages/deno/.eslintrc.js b/packages/deno/.eslintrc.js index 0c539a61b1b2..5a8ccd2be035 100644 --- a/packages/deno/.eslintrc.js +++ b/packages/deno/.eslintrc.js @@ -2,8 +2,6 @@ module.exports = { extends: ['../../.eslintrc.js'], ignorePatterns: ['lib.deno.d.ts', 'scripts/*.mjs', 'build-types/**', 'build-test/**', 'build/**'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/eslint-config-sdk/src/base.js b/packages/eslint-config-sdk/src/base.js index 525b24b4a334..c137729161c5 100644 --- a/packages/eslint-config-sdk/src/base.js +++ b/packages/eslint-config-sdk/src/base.js @@ -130,11 +130,6 @@ module.exports = { }, ], - // We want to prevent optional chaining & nullish coalescing usage in our files - // to prevent unnecessary bundle size. Turned off in tests. - '@sentry-internal/sdk/no-optional-chaining': 'error', - '@sentry-internal/sdk/no-nullish-coalescing': 'error', - // We want to avoid using the RegExp constructor as it can lead to invalid or dangerous regular expressions // if end user input is used in the constructor. It's fine to ignore this rule if it is safe to use the RegExp. // However, we want to flag each use case so that we're aware of the potential danger. @@ -181,8 +176,6 @@ module.exports = { '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/no-non-null-assertion': 'off', '@typescript-eslint/no-empty-function': 'off', - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@typescript-eslint/no-floating-promises': 'off', '@sentry-internal/sdk/no-focused-tests': 'error', '@sentry-internal/sdk/no-skipped-tests': 'error', diff --git a/packages/eslint-plugin-sdk/src/index.js b/packages/eslint-plugin-sdk/src/index.js index d7516c343d60..24cc9c4cc00c 100644 --- a/packages/eslint-plugin-sdk/src/index.js +++ b/packages/eslint-plugin-sdk/src/index.js @@ -10,8 +10,6 @@ module.exports = { rules: { - 'no-optional-chaining': require('./rules/no-optional-chaining'), - 'no-nullish-coalescing': require('./rules/no-nullish-coalescing'), 'no-eq-empty': require('./rules/no-eq-empty'), 'no-class-field-initializers': require('./rules/no-class-field-initializers'), 'no-regexp-constructor': require('./rules/no-regexp-constructor'), diff --git a/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js b/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js deleted file mode 100644 index 8944f2755632..000000000000 --- a/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @fileoverview disallow nullish coalescing operators as they were introduced only in ES2020 and hence require - * us to add a polyfill. This increases bundle size more than avoiding nullish coalescing operators all together. - * - * @author Lukas Stracke - * - * Based on: https://github.com/mysticatea/eslint-plugin-es/blob/v4.1.0/lib/rules/no-nullish-coalescing-operators.js - */ -'use strict'; - -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ - -module.exports = { - meta: { - type: 'problem', - docs: { - description: 'disallow nullish coalescing operators.', - category: 'Best Practices', - recommended: true, - }, - messages: { - forbidden: 'Avoid using nullish coalescing operators.', - }, - fixable: null, - schema: [], - }, - create(context) { - return { - "LogicalExpression[operator='??']"(node) { - context.report({ - node: context.getSourceCode().getTokenAfter(node.left, isNullishCoalescingOperator), - messageId: 'forbidden', - }); - }, - }; - }, -}; - -/** - * Checks if the given token is a nullish coalescing operator or not. - * @param {Token} token - The token to check. - * @returns {boolean} `true` if the token is a nullish coalescing operator. - */ -function isNullishCoalescingOperator(token) { - return token.value === '??' && token.type === 'Punctuator'; -} diff --git a/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js b/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js deleted file mode 100644 index 67da656bc782..000000000000 --- a/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * @fileoverview Rule to disallow using optional chaining - because this is transpiled into verbose code. - * @author Francesco Novy - * - * Based on https://github.com/facebook/lexical/pull/3233 - */ -'use strict'; - -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ - -/** @type {import('eslint').Rule.RuleModule} */ -module.exports = { - meta: { - type: 'problem', - docs: { - description: 'disallow usage of optional chaining', - category: 'Best Practices', - recommended: true, - }, - messages: { - forbidden: 'Avoid using optional chaining', - }, - fixable: null, - schema: [], - }, - - create(context) { - const sourceCode = context.getSourceCode(); - - /** - * Checks if the given token is a `?.` token or not. - * @param {Token} token The token to check. - * @returns {boolean} `true` if the token is a `?.` token. - */ - function isQuestionDotToken(token) { - return ( - token.value === '?.' && - (token.type === 'Punctuator' || // espree has been parsed well. - // espree@7.1.0 doesn't parse "?." tokens well. Therefore, get the string from the source code and check it. - sourceCode.getText(token) === '?.') - ); - } - - return { - 'CallExpression[optional=true]'(node) { - context.report({ - messageId: 'forbidden', - node: sourceCode.getTokenAfter(node.callee, isQuestionDotToken), - }); - }, - 'MemberExpression[optional=true]'(node) { - context.report({ - messageId: 'forbidden', - node: sourceCode.getTokenAfter(node.object, isQuestionDotToken), - }); - }, - }; - }, -}; diff --git a/packages/google-cloud-serverless/.eslintrc.js b/packages/google-cloud-serverless/.eslintrc.js index 99fcba0976da..d1d4c4e12aa0 100644 --- a/packages/google-cloud-serverless/.eslintrc.js +++ b/packages/google-cloud-serverless/.eslintrc.js @@ -3,9 +3,6 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/nextjs/.eslintrc.js b/packages/nextjs/.eslintrc.js index 95ce15bc668f..1525e502018f 100644 --- a/packages/nextjs/.eslintrc.js +++ b/packages/nextjs/.eslintrc.js @@ -7,10 +7,6 @@ module.exports = { jsx: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/nitro-utils/.eslintrc.js b/packages/nitro-utils/.eslintrc.js index 3849c1ee28a6..7ac3732750a5 100644 --- a/packages/nitro-utils/.eslintrc.js +++ b/packages/nitro-utils/.eslintrc.js @@ -3,19 +3,4 @@ module.exports = { env: { node: true, }, - overrides: [ - { - files: ['src/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, - { - files: ['src/metrics/**'], - rules: { - '@typescript-eslint/explicit-function-return-type': 'off', - '@typescript-eslint/no-non-null-assertion': 'off', - }, - }, - ], }; diff --git a/packages/node/.eslintrc.js b/packages/node/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/node/.eslintrc.js +++ b/packages/node/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/nuxt/.eslintrc.js b/packages/nuxt/.eslintrc.js index d567b12530d0..a22f9710cf6b 100644 --- a/packages/nuxt/.eslintrc.js +++ b/packages/nuxt/.eslintrc.js @@ -10,13 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/opentelemetry/.eslintrc.js b/packages/opentelemetry/.eslintrc.js index 9899ea1b73d8..fdb9952bae52 100644 --- a/packages/opentelemetry/.eslintrc.js +++ b/packages/opentelemetry/.eslintrc.js @@ -3,7 +3,4 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, }; diff --git a/packages/profiling-node/.eslintrc.js b/packages/profiling-node/.eslintrc.js index 02a39c9cf562..e4cb54c2f08c 100644 --- a/packages/profiling-node/.eslintrc.js +++ b/packages/profiling-node/.eslintrc.js @@ -6,8 +6,6 @@ module.exports = { ignorePatterns: ['lib/**/*', 'examples/**/*', 'jest.co'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/remix/.eslintrc.js b/packages/remix/.eslintrc.js index 992bcd3e9d2b..f1046a6136d6 100644 --- a/packages/remix/.eslintrc.js +++ b/packages/remix/.eslintrc.js @@ -8,9 +8,6 @@ module.exports = { }, ignorePatterns: ['playwright.config.ts', 'vitest.config.ts', 'test/integration/**'], extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/solidstart/.eslintrc.js b/packages/solidstart/.eslintrc.js index d567b12530d0..a22f9710cf6b 100644 --- a/packages/solidstart/.eslintrc.js +++ b/packages/solidstart/.eslintrc.js @@ -10,13 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/.eslintrc.js b/packages/sveltekit/.eslintrc.js index c1f55c94aadf..a22f9710cf6b 100644 --- a/packages/sveltekit/.eslintrc.js +++ b/packages/sveltekit/.eslintrc.js @@ -10,12 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index 28e903d89db7..7194a3ae3ac8 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -1,6 +1,5 @@ import * as fs from 'fs'; import * as path from 'path'; -/* eslint-disable @sentry-internal/sdk/no-optional-chaining */ import type { ExportNamedDeclaration } from '@babel/types'; import { parseModule } from 'magicast'; import type { Plugin } from 'vite'; diff --git a/packages/sveltekit/src/vite/svelteConfig.ts b/packages/sveltekit/src/vite/svelteConfig.ts index 9186e46caba6..02edad15382e 100644 --- a/packages/sveltekit/src/vite/svelteConfig.ts +++ b/packages/sveltekit/src/vite/svelteConfig.ts @@ -1,5 +1,3 @@ -/* eslint-disable @sentry-internal/sdk/no-optional-chaining */ - import * as fs from 'fs'; import * as path from 'path'; import * as url from 'url'; diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 1d8d712e5b0f..842f8475905d 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -14,13 +14,7 @@ import { SyncPromise as SyncPromise_imported, TRACEPARENT_REGEXP as TRACEPARENT_REGEXP_imported, UNKNOWN_FUNCTION as UNKNOWN_FUNCTION_imported, - _asyncNullishCoalesce as _asyncNullishCoalesce_imported, - _asyncOptionalChain as _asyncOptionalChain_imported, - _asyncOptionalChainDelete as _asyncOptionalChainDelete_imported, _browserPerformanceTimeOriginMode as _browserPerformanceTimeOriginMode_imported, - _nullishCoalesce as _nullishCoalesce_imported, - _optionalChain as _optionalChain_imported, - _optionalChainDelete as _optionalChainDelete_imported, addConsoleInstrumentationHandler as addConsoleInstrumentationHandler_imported, addContextToFrame as addContextToFrame_imported, addExceptionMechanism as addExceptionMechanism_imported, @@ -646,24 +640,6 @@ export const extractRequestData = extractRequestData_imported; // eslint-disable-next-line deprecation/deprecation export const addRequestDataToEvent = addRequestDataToEvent_imported; -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncNullishCoalesce = _asyncNullishCoalesce_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncOptionalChain = _asyncOptionalChain_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncOptionalChainDelete = _asyncOptionalChainDelete_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _nullishCoalesce = _nullishCoalesce_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _optionalChain = _optionalChain_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _optionalChainDelete = _optionalChainDelete_imported; - /** @deprecated Import from `@sentry/core` instead. */ // eslint-disable-next-line deprecation/deprecation export const BAGGAGE_HEADER_NAME = BAGGAGE_HEADER_NAME_imported; diff --git a/packages/vercel-edge/.eslintrc.js b/packages/vercel-edge/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/vercel-edge/.eslintrc.js +++ b/packages/vercel-edge/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/wasm/.eslintrc.js b/packages/wasm/.eslintrc.js index fefcfd5cb729..5a2cc7f1ec08 100644 --- a/packages/wasm/.eslintrc.js +++ b/packages/wasm/.eslintrc.js @@ -1,6 +1,3 @@ module.exports = { extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, }; From ac0d55a3532a416239d187d7c4cdafa2462df84b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 10:07:38 +0000 Subject: [PATCH 13/38] revert --- packages/core/src/baseclient.ts | 2 -- packages/core/src/types-hoist/integration.ts | 10 ---------- 2 files changed, 12 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 910219a0b172..f9505206c4d8 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -370,8 +370,6 @@ export abstract class BaseClient implements Client { const clientReleaseOption = this._options.release; const clientEnvironmentOption = this._options.environment; if ('aggregates' in session) { - // TODO(v9): Remove eslint disable - // eslint-disable-next-line @sentry-internal/sdk/no-optional-chaining if (!session.attrs?.release && !clientReleaseOption) { DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); return; diff --git a/packages/core/src/types-hoist/integration.ts b/packages/core/src/types-hoist/integration.ts index 85d32b1c682a..deb23baaca51 100644 --- a/packages/core/src/types-hoist/integration.ts +++ b/packages/core/src/types-hoist/integration.ts @@ -51,16 +51,6 @@ export interface Integration { * This receives the client that the integration was installed for as third argument. */ processEvent?(event: Event, hint: EventHint, client: Client): Event | null | PromiseLike; - - /** - * TODO - */ - flush?(client: Client): Promise; - - /** - * TODO - */ - close?(client: Client): Promise; } /** From 74ac9c33aa9af42564004132463de9dccae11ba2 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 11:16:42 +0000 Subject: [PATCH 14/38] Hmm --- packages/core/src/server-runtime-client.ts | 27 ++++++++----- .../http/SentryHttpInstrumentation.ts | 21 +++++----- packages/node/src/sdk/index.ts | 40 ------------------- 3 files changed, 30 insertions(+), 58 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 170aa0159923..563e4ef4854e 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -76,6 +76,8 @@ export class ServerRuntimeClient< * @inheritDoc */ public captureException(exception: unknown, hint?: EventHint, scope?: Scope): string { + // TODO: Check if mechanism === unhandled + setCurrentRequestSessionCrashed(); return super.captureException(exception, hint, scope); } @@ -87,15 +89,7 @@ export class ServerRuntimeClient< const isException = !event.type && event.exception && event.exception.values && event.exception.values.length > 0; if (isException) { // TODO: Check if mechanism === unhandled - const isolationScope = getIsolationScope(); - const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; - if (requestSession) { - isolationScope.setSDKProcessingMetadata({ - requestSession: { - status: 'errored', - }, - }); - } + setCurrentRequestSessionCrashed(); } return super.captureEvent(event, hint, scope); @@ -209,3 +203,18 @@ export class ServerRuntimeClient< return [dynamicSamplingContext, traceContext]; } } + +function setCurrentRequestSessionCrashed(): void { + const requestSession = getIsolationScope().getScopeData().sdkProcessingMetadata.requestSession as + | { status: 'ok' | 'crashed' } + | undefined; + + if (requestSession) { + // We mutate instead of doing `setSdkProcessingMetadata` because the http integration stores away a particular + // isolationScope. If that isolation scope is forked, setting the processing metadata here will not mutate the + // original isolation scope that the http integration stored away. + requestSession.status = 'crashed'; + + // TODO maybe send 'errored' when handled exception? + } +} diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index e0a32607a52c..4c8925ac1119 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -9,6 +9,7 @@ import type { AggregationCounts, Client, RequestEventData, SanitizedRequestData, import { addBreadcrumb, getBreadcrumbLogLevelFromHttpStatusCode, + getClient, getIsolationScope, getSanitizedUrlString, httpRequestToRequestData, @@ -23,11 +24,11 @@ import { getRequestInfo } from './vendor/getRequestInfo'; const clientToAggregatesMap = new Map< Client, - { [timestampRoundedToSeconds: string]: { exited: number; errored: number } } + { [timestampRoundedToSeconds: string]: { exited: number; crashed: number } } >(); interface RequestSession { - status: 'ok' | 'errored'; + status: 'ok' | 'crashed'; } type Http = typeof http; @@ -178,7 +179,8 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - const client = isolationScope.getClient(); + // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. + const client = getClient(); const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession as | RequestSession | undefined; @@ -191,13 +193,13 @@ export class SentryHttpInstrumentation extends InstrumentationBase ({ started: timestamp, exited: value.exited, - errored: value.errored, + crashed: value.crashed, }), ); client.sendSession({ aggregates: aggregatePayload }); @@ -223,7 +225,8 @@ export class SentryHttpInstrumentation extends InstrumentationBase { DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); flushPendingClientAggregates(); - }, 60_000).unref(); + // TODO: Increase to 60s + }, 5_000).unref(); } } }); diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 2ce75908168d..b326301cbaa4 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -2,12 +2,9 @@ import type { Integration, Options } from '@sentry/core'; import { consoleSandbox, dropUndefinedKeys, - endSession, functionToStringIntegration, - getClient, getCurrentScope, getIntegrationsToSetup, - getIsolationScope, hasTracingEnabled, inboundFiltersIntegration, linkedErrorsIntegration, @@ -15,7 +12,6 @@ import { propagationContextFromHeaders, requestDataIntegration, stackParserFromStackParserOptions, - startSession, } from '@sentry/core'; import { enhanceDscWithOpenTelemetryRootSpanName, @@ -155,13 +151,6 @@ function _init( client.init(); logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`); - - // TODO(V9): Remove this code since all of the logic should be in an integration - // eslint-disable-next-line deprecation/deprecation - if (options.autoSessionTracking) { - startSessionTracking(); - } - client.startClientReportTracking(); updateScopeFromEnvVariables(); @@ -312,32 +301,3 @@ function updateScopeFromEnvVariables(): void { getCurrentScope().setPropagationContext(propagationContext); } } - -/** - * Enable automatic Session Tracking for the node process. - */ -function startSessionTracking(): void { - const client = getClient(); - // eslint-disable-next-line deprecation/deprecation - if (client && client.getOptions().autoSessionTracking) { - client.initSessionFlusher(); - } - - startSession(); - - // Emitted in the case of healthy sessions, error of `mechanism.handled: true` and unhandledrejections because - // The 'beforeExit' event is not emitted for conditions causing explicit termination, - // such as calling process.exit() or uncaught exceptions. - // Ref: https://nodejs.org/api/process.html#process_event_beforeexit - process.on('beforeExit', () => { - const session = getIsolationScope().getSession(); - - // Only call endSession, if the Session exists on Scope and SessionStatus is not a - // Terminal Status i.e. Exited or Crashed because - // "When a session is moved away from ok it must not be updated anymore." - // Ref: https://develop.sentry.dev/sdk/sessions/ - if (session && session.status !== 'ok') { - endSession(); - } - }); -} From 5e65c875c049da547bce388693edecb74999ae94 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 6 Dec 2024 11:27:48 +0100 Subject: [PATCH 15/38] ci(v9): Ensure CI runs on v8 & v9 branches (#14604) In order for us to have size-limit comparison etc, we need to ensure CI runs on v8 & v9 branches too. --- .github/workflows/build.yml | 10 ++++++---- .github/workflows/enforce-license-compliance.yml | 13 +++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bf9ba21376bb..31f5cea28bda 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,6 +4,8 @@ on: branches: - develop - master + - v9 + - v8 - release/** pull_request: merge_group: @@ -105,7 +107,7 @@ jobs: outputs: commit_label: '${{ env.COMMIT_SHA }}: ${{ env.COMMIT_MESSAGE }}' # Note: These next three have to be checked as strings ('true'/'false')! - is_develop: ${{ github.ref == 'refs/heads/develop' }} + is_base_branch: ${{ github.ref == 'refs/heads/develop' || github.ref == 'refs/heads/v9' || github.ref == 'refs/heads/v8'}} is_release: ${{ startsWith(github.ref, 'refs/heads/release/') }} changed_profiling_node: ${{ steps.changed.outputs.profiling_node == 'true' }} changed_ci: ${{ steps.changed.outputs.workflow == 'true' }} @@ -126,7 +128,7 @@ jobs: timeout-minutes: 15 if: | needs.job_get_metadata.outputs.changed_any_code == 'true' || - needs.job_get_metadata.outputs.is_develop == 'true' || + needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' || (needs.job_get_metadata.outputs.is_gitflow_sync == 'false' && needs.job_get_metadata.outputs.has_gitflow_label == 'false') steps: @@ -171,7 +173,7 @@ jobs: key: nx-Linux-${{ github.ref }}-${{ env.HEAD_COMMIT || github.sha }} # On develop branch, we want to _store_ the cache (so it can be used by other branches), but never _restore_ from it restore-keys: - ${{needs.job_get_metadata.outputs.is_develop == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} + ${{needs.job_get_metadata.outputs.is_base_branch == 'false' && env.NX_CACHE_RESTORE_KEYS || 'nx-never-restore'}} - name: Build packages # Set the CODECOV_TOKEN for Bundle Analysis @@ -219,7 +221,7 @@ jobs: timeout-minutes: 15 runs-on: ubuntu-20.04 if: - github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_develop == 'true' || + github.event_name == 'pull_request' || needs.job_get_metadata.outputs.is_base_branch == 'true' || needs.job_get_metadata.outputs.is_release == 'true' steps: - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) diff --git a/.github/workflows/enforce-license-compliance.yml b/.github/workflows/enforce-license-compliance.yml index 8f63b6ca063b..776f8135178d 100644 --- a/.github/workflows/enforce-license-compliance.yml +++ b/.github/workflows/enforce-license-compliance.yml @@ -2,9 +2,18 @@ name: "CI: Enforce License Compliance" on: push: - branches: [master, develop, release/*] + branches: + - develop + - master + - v9 + - v8 + - release/** pull_request: - branches: [master, develop] + branches: + - develop + - master + - v9 + - v8 jobs: enforce-license-compliance: From fac8d973ba39981fa6935bdcd038abd41e6134d9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:06:10 +0100 Subject: [PATCH 16/38] feat(vue/nuxt)!: No longer create `"update"` spans for component tracking by default (#14602) Resolves https://github.com/getsentry/sentry-javascript/issues/12851 --- docs/migration/draft-v9-migration-guide.md | 22 ++++++++++++++++++++++ packages/vue/src/constants.ts | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index acf268c81ef4..461b740d06b7 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -107,6 +107,28 @@ }); ``` +## `@sentry/nuxt` and `@sentry/vue` + +- When component tracking is enabled, "update" spans are no longer created by default. + Add an `"update"` item to the `tracingOptions.hooks` option via the `vueIntegration()` to restore this behavior. + + ```ts + Sentry.init({ + integrations: [ + Sentry.vueIntegration({ + tracingOptions: { + trackComponents: true, + hooks: [ + 'mount', + 'update', // <-- + 'unmount', + ], + }, + }), + ], + }); + ``` + ## `@sentry/astro` - Deprecated passing `dsn`, `release`, `environment`, `sampleRate`, `tracesSampleRate`, `replaysSessionSampleRate` to the integration. Use the runtime-specific `Sentry.init()` calls for passing these options instead. diff --git a/packages/vue/src/constants.ts b/packages/vue/src/constants.ts index e254d988c40c..50aa82f77885 100644 --- a/packages/vue/src/constants.ts +++ b/packages/vue/src/constants.ts @@ -1,3 +1,3 @@ import type { Operation } from './types'; -export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount', 'update']; +export const DEFAULT_HOOKS: Operation[] = ['activate', 'mount']; From 677261517e58e0bd4a398bf7514bdb57763378da Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Tue, 10 Dec 2024 11:43:05 +0100 Subject: [PATCH 17/38] feat(nextjs)!: Remove `experimental_captureRequestError` (#14607) Fixes https://github.com/getsentry/sentry-javascript/issues/14302 --- .../e2e-tests/test-applications/nextjs-t3/package.json | 6 +++--- .../test-applications/nextjs-t3/src/trpc/server.ts | 7 +++---- packages/nextjs/src/common/captureRequestError.ts | 8 -------- packages/nextjs/src/common/index.ts | 3 +-- packages/nextjs/src/index.types.ts | 3 +-- 5 files changed, 8 insertions(+), 19 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json index 2fd54b440e2e..4cdd6509ddbd 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/package.json @@ -21,9 +21,9 @@ "@trpc/react-query": "^11.0.0-rc.446", "@trpc/server": "^11.0.0-rc.446", "geist": "^1.3.0", - "next": "^14.2.4", - "react": "^18.3.1", - "react-dom": "^18.3.1", + "next": "14.2.4", + "react": "18.3.1", + "react-dom": "18.3.1", "server-only": "^0.0.1", "superjson": "^2.2.1", "zod": "^3.23.3" diff --git a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts index b6cb13a70781..7760873eb51d 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-t3/src/trpc/server.ts @@ -2,7 +2,6 @@ import 'server-only'; import { createHydrationHelpers } from '@trpc/react-query/rsc'; import { headers } from 'next/headers'; -import { cache } from 'react'; import { type AppRouter, createCaller } from '~/server/api/root'; import { createTRPCContext } from '~/server/api/trpc'; @@ -12,16 +11,16 @@ import { createQueryClient } from './query-client'; * This wraps the `createTRPCContext` helper and provides the required context for the tRPC API when * handling a tRPC call from a React Server Component. */ -const createContext = cache(() => { +const createContext = () => { const heads = new Headers(headers()); heads.set('x-trpc-source', 'rsc'); return createTRPCContext({ headers: heads, }); -}); +}; -const getQueryClient = cache(createQueryClient); +const getQueryClient = createQueryClient; const caller = createCaller(createContext); export const { trpc: api, HydrateClient } = createHydrationHelpers(caller, getQueryClient); diff --git a/packages/nextjs/src/common/captureRequestError.ts b/packages/nextjs/src/common/captureRequestError.ts index 6de33ad11a8e..26fdaab4953b 100644 --- a/packages/nextjs/src/common/captureRequestError.ts +++ b/packages/nextjs/src/common/captureRequestError.ts @@ -41,11 +41,3 @@ export function captureRequestError(error: unknown, request: RequestInfo, errorC }); }); } - -/** - * Reports errors passed to the the Next.js `onRequestError` instrumentation hook. - * - * @deprecated Use `captureRequestError` instead. - */ -// TODO(v9): Remove this export -export const experimental_captureRequestError = captureRequestError; diff --git a/packages/nextjs/src/common/index.ts b/packages/nextjs/src/common/index.ts index 7740c35c016c..b9a652522349 100644 --- a/packages/nextjs/src/common/index.ts +++ b/packages/nextjs/src/common/index.ts @@ -11,5 +11,4 @@ export { wrapMiddlewareWithSentry } from './wrapMiddlewareWithSentry'; export { wrapPageComponentWithSentry } from './pages-router-instrumentation/wrapPageComponentWithSentry'; export { wrapGenerationFunctionWithSentry } from './wrapGenerationFunctionWithSentry'; export { withServerActionInstrumentation } from './withServerActionInstrumentation'; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './captureRequestError'; +export { captureRequestError } from './captureRequestError'; diff --git a/packages/nextjs/src/index.types.ts b/packages/nextjs/src/index.types.ts index 1b6a0e09ed85..1b965828116f 100644 --- a/packages/nextjs/src/index.types.ts +++ b/packages/nextjs/src/index.types.ts @@ -142,5 +142,4 @@ export declare function wrapApiHandlerWithSentryVercelCrons(WrappingTarget: C): C; -// eslint-disable-next-line deprecation/deprecation -export { experimental_captureRequestError, captureRequestError } from './common/captureRequestError'; +export { captureRequestError } from './common/captureRequestError'; From 6e88c55109ef1479a9bae2d340ca2a8656d13deb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 11 Dec 2024 17:48:02 +0100 Subject: [PATCH 18/38] meta(v9): Add v9 migration guide (#14296) --- MIGRATION.md | 1 + docs/migration/v8-to-v9.md | 159 +++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 docs/migration/v8-to-v9.md diff --git a/MIGRATION.md b/MIGRATION.md index 78657e360d2b..b142902cd6d1 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -5,6 +5,7 @@ These docs walk through how to migrate our JavaScript SDKs through different maj - Upgrading from [SDK 4.x to 5.x/6.x](./docs/migration/v4-to-v5_v6.md) - Upgrading from [SDK 6.x to 7.x](./docs/migration/v6-to-v7.md) - Upgrading from [SDK 7.x to 8.x](./MIGRATION.md#upgrading-from-7x-to-8x) +- Upgrading from [SDK 8.x to 9.x](./docs/migration/v8-to-v9.md) (Work in Progress - v9 is not released yet) # Upgrading from 7.x to 8.x diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md new file mode 100644 index 000000000000..baa14f19f8a4 --- /dev/null +++ b/docs/migration/v8-to-v9.md @@ -0,0 +1,159 @@ +# Upgrading from 8.x to 9.x + +**DISCLAIMER: THIS MIGRATION GUIDE IS WORK IN PROGRESS** + +Version 9 of the Sentry SDK concerns itself with API cleanup and compatibility updates. +This update contains behavioral changes that will not be caught by TypeScript or linters, so we recommend carefully reading the section on [Behavioral Changes](#2-behavior-changes). + +Before updating to version `9.x` of the SDK, we recommend upgrading to the latest version of `8.x`. +You can then go through the [Deprecations in 8.x](#deprecations-in-8x) and remove and migrate usages of deprecated APIs in your code before upgrading to `9.x`. + +Version 9 of the JavaScript SDK is compatible with Sentry self-hosted versions 24.4.2 or higher (unchanged from last major). +Lower versions may continue to work, but may not support all features. + +## 1. Version Support Changes: + +Version 9 of the Sentry SDK has new compatibility ranges for runtimes and frameworks. +We periodically update the compatibility ranges in major versions to increase reliability and quality of APIs and instrumentation data. + +### General Runtime Support Changes + +**ECMAScript Version:** All of the JavaScript code in the Sentry SDK packages may now contain ECMAScript 2020 features. +This includes features like Nullish Coalescing (`??`), Optional Chaining (`?.`), `String.matchAll()`, Logical Assignment Operators (`&&=`, `||=`, `??=`), and `Promise.allSettled()`. + +If you observe failures due to syntax or features listed above, it may be an indicator that your current runtime does not support ES2020. +If your runtime does not support ES2020, we recommend transpiling the SDK using Babel or similar tooling. + +**Node.js:** The minimum supported Node.js versions are TBD, TBD, and TBD. +We no longer test against Node TBD, TBD, or TBD and cannot guarantee that the SDK will work as expected on these versions. + +**Browsers:** Due to SDK code now including ES2020 features, the minimum supported browser list now looks as follows: + +- Chrome 80 +- Edge 80 +- Safari 14, iOS Safari 14.4 +- Firefox 74 +- Opera 67 +- Samsung Internet 13.0 + +If you need to support older browsers, we recommend transpiling your code using Babel or similar tooling. + +### Framework Support Changes + +**Angular:** TBD + +**Ember:** TBD + +**Next.js:** TBD + +**Nuxt:** TBD + +**React:** TBD + +**Vue:** TBD + +**Astro:** TBD + +**Gatsby:** TBD + +**NestJS:** TBD + +**Svelte:** TBD + +**SvelteKit:** TBD + +**Bun:** TBD + +**Cloudflare Workers:** TBD + +**Deno:** TBD + +**Solid:** TBD + +**SolidStart:** TBD + +**GCP Functions:** TBD + +**AWS Lambda:** TBD + +## 2. Behavior Changes + +- Next.js withSentryConfig returning Promise +- `request` on sdk processing metadata will be ignored going forward +- respect sourcemap generation settings +- SDK init options undefined +- no more polyfills +- no more update spans in vue component tracking by default +- new propagation context +- Client & Scope renaming + +## 3. Package Removals + +As part of an architectural cleanup we deprecated the following packages: + +- `@sentry/utils` +- `@sentry/types` + +All of these packages exports and APIs have been moved into the `@sentry/core` package. + +The `@sentry/utils` package will no longer be published. + +The `@sentry/types` package will continue to be published but it is deprecated and we don't plan on extending its APi. +You may experience slight compatibility issues in the future by using it. +We decided to keep this package around to temporarily lessen the upgrade burden. +It will be removed in a future major version. + +## 4. Removal of Deprecated APIs + +- [General](#general) +- [Server-side SDKs (Node, Deno, Bun, ...)](#server-side-sdks-node-deno-bun-) +- [Next.js SDK](#nextjs-sdk) +- [Vue/Nuxt SDK](#vuenuxt-sdk) + +### General + +- sessionTimingIntegration +- debugIntegration +- `Request` type +- spanid on propagation context +- makeFifoCache in utils + +### Server-side SDKs (Node, Deno, Bun, ...) + +- processThreadBreadcrumbIntegration +- NestJS stuff in Node sdk +- various NestJS APIs +- NestJS `@WithSentry` +- `AddRequestDataToEventOptions.transaction` + +### Next.js SDK + +- `experimental_captureRequestError` + +### Vue/Nuxt SDK + +- vueComponent tracking options + +## 5. Build Changes + +Previously the CJS versions of the SDK code (wrongfully) contained compatibility statements for default exports in ESM: + +```js +Object.defineProperty(exports, '__esModule', { value: true }); +``` + +The SDK no longer contains these statements. +Let us know if this is causing issues in your setup by opening an issue on GitHub. + +# Deprecations in 8.x + +TBD (Copy over from migrations list we collected) + +# No Version Support Timeline + +Version support timelines are stressful for anybody using the SDK, so we won't be defining one. +Instead, we will be applying bug fixes and features to older versions as long as there is demand for them. +We also hold ourselves to high standards security-wise, meaning that if any vulnerabilities are found, we will in almost all cases backport them. + +Note, that we will decide on a case-per-case basis, what gets backported or not. +If you need a fix or feature in a previous version of the SDK, feel free to reach out via a GitHub issue. From bfd865c0a9fa2d8204654a424ddfbe1cf2632b5f Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 12 Dec 2024 14:56:05 +0100 Subject: [PATCH 19/38] feat(node)!: Remove `processThreadBreadcrumbIntegration` (#14666) Resolves https://github.com/getsentry/sentry-javascript/issues/14277 --- .../node-exports-test-app/scripts/consistentExports.ts | 2 -- packages/astro/src/index.server.ts | 2 -- packages/aws-serverless/src/index.ts | 2 -- packages/google-cloud-serverless/src/index.ts | 2 -- packages/node/src/index.ts | 3 +-- packages/node/src/integrations/childProcess.ts | 7 ------- 6 files changed, 1 insertion(+), 17 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts index 83f9c1639cdc..8c3e51b14024 100644 --- a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts +++ b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts @@ -50,8 +50,6 @@ const DEPENDENTS: Dependent[] = [ ignoreExports: [ // not supported in bun: 'NodeClient', - // Bun doesn't emit the required diagnostics_channel events - 'processThreadBreadcrumbIntegration', 'childProcessIntegration', ], }, diff --git a/packages/astro/src/index.server.ts b/packages/astro/src/index.server.ts index 7eca9de9a41a..c628bb7605ff 100644 --- a/packages/astro/src/index.server.ts +++ b/packages/astro/src/index.server.ts @@ -97,8 +97,6 @@ export { parameterize, postgresIntegration, prismaIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, redisIntegration, requestDataIntegration, diff --git a/packages/aws-serverless/src/index.ts b/packages/aws-serverless/src/index.ts index 3f167b62a7e3..a4f7f378ed8b 100644 --- a/packages/aws-serverless/src/index.ts +++ b/packages/aws-serverless/src/index.ts @@ -110,8 +110,6 @@ export { setupNestErrorHandler, postgresIntegration, prismaIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, hapiIntegration, setupHapiErrorHandler, diff --git a/packages/google-cloud-serverless/src/index.ts b/packages/google-cloud-serverless/src/index.ts index 6f89769c2a37..d23c78f412d9 100644 --- a/packages/google-cloud-serverless/src/index.ts +++ b/packages/google-cloud-serverless/src/index.ts @@ -123,8 +123,6 @@ export { zodErrorsIntegration, profiler, amqplibIntegration, - // eslint-disable-next-line deprecation/deprecation - processThreadBreadcrumbIntegration, childProcessIntegration, } from '@sentry/node'; diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index fa16ac4e6b3d..71b1b80ffe82 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -34,8 +34,7 @@ export { tediousIntegration } from './integrations/tracing/tedious'; export { genericPoolIntegration } from './integrations/tracing/genericPool'; export { dataloaderIntegration } from './integrations/tracing/dataloader'; export { amqplibIntegration } from './integrations/tracing/amqplib'; -// eslint-disable-next-line deprecation/deprecation -export { processThreadBreadcrumbIntegration, childProcessIntegration } from './integrations/childProcess'; +export { childProcessIntegration } from './integrations/childProcess'; export { SentryContextManager } from './otel/contextManager'; export { generateInstrumentOnce } from './otel/instrument'; diff --git a/packages/node/src/integrations/childProcess.ts b/packages/node/src/integrations/childProcess.ts index 99525b4092b4..e78cc843f279 100644 --- a/packages/node/src/integrations/childProcess.ts +++ b/packages/node/src/integrations/childProcess.ts @@ -39,13 +39,6 @@ export const childProcessIntegration = defineIntegration((options: Options = {}) }; }); -/** - * Capture breadcrumbs for child processes and worker threads. - * - * @deprecated Use `childProcessIntegration` integration instead. Functionally they are the same. `processThreadBreadcrumbIntegration` will be removed in the next major version. - */ -export const processThreadBreadcrumbIntegration = childProcessIntegration; - function captureChildProcessEvents(child: ChildProcess, options: Options): void { let hasExited = false; let data: Record | undefined; From 550ceef8e88e34508881a729348eea2510672f09 Mon Sep 17 00:00:00 2001 From: Francesco Gringl-Novy Date: Fri, 13 Dec 2024 09:19:11 +0100 Subject: [PATCH 20/38] feat(node)!: Avoid http spans by default for custom OTEL setups (#14678) With this PR, the default value for the `spans` option in the `httpIntegration` is changed to `false`, if `skipOpenTelemetrySetup: true` is configured. This is what you'd expect as a user, you do not want Sentry to register any OTEL instrumentation and emit any spans in this scenario. Closes https://github.com/getsentry/sentry-javascript/issues/14675 --- .../node-otel-custom-sampler/src/instrument.ts | 1 + .../node-otel-sdk-node/src/instrument.ts | 1 + .../src/instrument.ts | 3 --- docs/migration/draft-v9-migration-guide.md | 1 + docs/migration/v8-to-v9.md | 6 ++++++ packages/node/src/integrations/http/index.ts | 14 +++++++++++++- packages/node/test/integrations/http.test.ts | 18 ++++++++++++++++++ 7 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 packages/node/test/integrations/http.test.ts diff --git a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts index b7279c9942a7..d0aed916864b 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/src/instrument.ts @@ -14,6 +14,7 @@ Sentry.init({ skipOpenTelemetrySetup: true, // By defining _any_ sample rate, tracing integrations will be added by default tracesSampleRate: 0, + integrations: [Sentry.httpIntegration({ spans: true })], }); const provider = new NodeTracerProvider({ diff --git a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts index fb270e1252d3..5cb2e5570db9 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/src/instrument.ts @@ -14,6 +14,7 @@ const sentryClient = Sentry.init({ tracesSampleRate: 1, skipOpenTelemetrySetup: true, + integrations: [Sentry.httpIntegration({ spans: true })], }); const sdk = new opentelemetry.NodeSDK({ diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index 47078a504e18..ebabd499fee5 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts @@ -15,9 +15,6 @@ Sentry.init({ debug: !!process.env.DEBUG, tunnel: `http://localhost:3031/`, // proxy server // Tracing is completely disabled - - integrations: [Sentry.httpIntegration({ spans: false })], - // Custom OTEL setup skipOpenTelemetrySetup: true, }); diff --git a/docs/migration/draft-v9-migration-guide.md b/docs/migration/draft-v9-migration-guide.md index 461b740d06b7..f731770fe142 100644 --- a/docs/migration/draft-v9-migration-guide.md +++ b/docs/migration/draft-v9-migration-guide.md @@ -158,3 +158,4 @@ - Deprecated `addOpenTelemetryInstrumentation`. Use the `openTelemetryInstrumentations` option in `Sentry.init()` or your custom Sentry Client instead. - Deprecated `registerEsmLoaderHooks.include` and `registerEsmLoaderHooks.exclude`. Set `onlyIncludeInstrumentedModules: true` instead. - `registerEsmLoaderHooks` will only accept `true | false | undefined` in the future. The SDK will default to wrapping modules that are used as part of OpenTelemetry Instrumentation. +- `httpIntegration({ spans: false })` is configured by default if `skipOpenTelemetrySetup: true` is set. You can still overwrite this if desired. diff --git a/docs/migration/v8-to-v9.md b/docs/migration/v8-to-v9.md index baa14f19f8a4..bb0cfe487da0 100644 --- a/docs/migration/v8-to-v9.md +++ b/docs/migration/v8-to-v9.md @@ -78,6 +78,12 @@ If you need to support older browsers, we recommend transpiling your code using ## 2. Behavior Changes +### `@sentry/node` + +- When `skipOpenTelemetrySetup: true` is configured, `httpIntegration({ spans: false })` will be configured by default. This means that you no longer have to specify this yourself in this scenario. With this change, no spans are emitted once `skipOpenTelemetrySetup: true` is configured, without any further configuration being needed. + +### Uncategorized (TODO) + - Next.js withSentryConfig returning Promise - `request` on sdk processing metadata will be ignored going forward - respect sourcemap generation settings diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index c976bb4da2a1..f06e12979074 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -8,6 +8,7 @@ import { getClient } from '@sentry/opentelemetry'; import { generateInstrumentOnce } from '../../otel/instrument'; import type { NodeClient } from '../../sdk/client'; import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; +import type { NodeClientOptions } from '../../types'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; import { getRequestUrl } from '../../utils/getRequestUrl'; import { SentryHttpInstrumentation } from './SentryHttpInstrumentation'; @@ -27,6 +28,8 @@ interface HttpOptions { * If set to false, do not emit any spans. * This will ensure that the default HttpInstrumentation from OpenTelemetry is not setup, * only the Sentry-specific instrumentation for request isolation is applied. + * + * If `skipOpenTelemetrySetup: true` is configured, this defaults to `false`, otherwise it defaults to `true`. */ spans?: boolean; @@ -118,12 +121,21 @@ export const instrumentOtelHttp = generateInstrumentOnce = {}): boolean { + // If `spans` is passed in, it takes precedence + // Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` + return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup; +} + /** * Instrument the HTTP and HTTPS modules. */ const instrumentHttp = (options: HttpOptions = {}): void => { + const instrumentSpans = _shouldInstrumentSpans(options, getClient()?.getOptions()); + // This is the "regular" OTEL instrumentation that emits spans - if (options.spans !== false) { + if (instrumentSpans) { const instrumentationConfig = getConfigWithDefaults(options); instrumentOtelHttp(instrumentationConfig); } diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts new file mode 100644 index 000000000000..a6a8e612c019 --- /dev/null +++ b/packages/node/test/integrations/http.test.ts @@ -0,0 +1,18 @@ +import { _shouldInstrumentSpans } from '../../src/integrations/http'; + +describe('httpIntegration', () => { + describe('_shouldInstrumentSpans', () => { + it.each([ + [{}, {}, true], + [{ spans: true }, {}, true], + [{ spans: false }, {}, false], + [{ spans: true }, { skipOpenTelemetrySetup: true }, true], + [{ spans: false }, { skipOpenTelemetrySetup: true }, false], + [{}, { skipOpenTelemetrySetup: true }, false], + [{}, { skipOpenTelemetrySetup: false }, true], + ])('returns the correct value for options=%p and clientOptions=%p', (options, clientOptions, expected) => { + const actual = _shouldInstrumentSpans(options, clientOptions); + expect(actual).toBe(expected); + }); + }); +}); From f792e647cab5aac66bc5f2dce1d6ae031366a536 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 11:01:57 +0100 Subject: [PATCH 21/38] ref!: Don't polyfill optional chaining and nullish coalescing (#14603) --- dev-packages/rollup-utils/npmHelpers.mjs | 6 - .../plugins/extractPolyfillsPlugin.mjs | 214 ------------------ .../rollup-utils/plugins/npmPlugins.mjs | 2 - dev-packages/test-utils/.eslintrc.js | 8 - packages/astro/.eslintrc.cjs | 7 - packages/aws-serverless/.eslintrc.js | 3 - packages/bun/.eslintrc.js | 2 - packages/cloudflare/.eslintrc.js | 2 - .../src/utils-hoist/buildPolyfills/README.md | 30 --- .../buildPolyfills/_asyncNullishCoalesce.ts | 51 ----- .../buildPolyfills/_asyncOptionalChain.ts | 82 ------- .../_asyncOptionalChainDelete.ts | 51 ----- .../buildPolyfills/_nullishCoalesce.ts | 49 ---- .../buildPolyfills/_optionalChain.ts | 82 ------- .../buildPolyfills/_optionalChainDelete.ts | 52 ----- .../src/utils-hoist/buildPolyfills/index.ts | 6 - .../src/utils-hoist/buildPolyfills/types.ts | 7 - packages/core/src/utils-hoist/index.ts | 7 - .../buildPolyfills/nullishCoalesce.test.ts | 31 --- .../buildPolyfills/optionalChain.test.ts | 92 -------- .../utils-hoist/buildPolyfills/originals.ts | 85 ------- packages/deno/.eslintrc.js | 2 - packages/eslint-config-sdk/src/base.js | 7 - packages/eslint-plugin-sdk/src/index.js | 2 - .../src/rules/no-nullish-coalescing.js | 48 ---- .../src/rules/no-optional-chaining.js | 61 ----- packages/google-cloud-serverless/.eslintrc.js | 3 - packages/nextjs/.eslintrc.js | 4 - packages/nitro-utils/.eslintrc.js | 15 -- packages/node/.eslintrc.js | 2 - packages/nuxt/.eslintrc.js | 7 - packages/opentelemetry/.eslintrc.js | 3 - packages/profiling-node/.eslintrc.js | 2 - packages/remix/.eslintrc.js | 3 - packages/solidstart/.eslintrc.js | 7 - packages/sveltekit/.eslintrc.js | 6 - packages/sveltekit/src/vite/autoInstrument.ts | 1 - packages/sveltekit/src/vite/svelteConfig.ts | 2 - packages/utils/src/index.ts | 24 -- packages/vercel-edge/.eslintrc.js | 2 - packages/wasm/.eslintrc.js | 3 - 41 files changed, 1073 deletions(-) delete mode 100644 dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/README.md delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/index.ts delete mode 100644 packages/core/src/utils-hoist/buildPolyfills/types.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts delete mode 100644 packages/core/test/utils-hoist/buildPolyfills/originals.ts delete mode 100644 packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js delete mode 100644 packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js diff --git a/dev-packages/rollup-utils/npmHelpers.mjs b/dev-packages/rollup-utils/npmHelpers.mjs index 4e6483364ee4..4cb8deaa61e0 100644 --- a/dev-packages/rollup-utils/npmHelpers.mjs +++ b/dev-packages/rollup-utils/npmHelpers.mjs @@ -15,7 +15,6 @@ import { defineConfig } from 'rollup'; import { makeCleanupPlugin, makeDebugBuildStatementReplacePlugin, - makeExtractPolyfillsPlugin, makeImportMetaUrlReplacePlugin, makeNodeResolvePlugin, makeRrwebBuildPlugin, @@ -44,7 +43,6 @@ export function makeBaseNPMConfig(options = {}) { const debugBuildStatementReplacePlugin = makeDebugBuildStatementReplacePlugin(); const importMetaUrlReplacePlugin = makeImportMetaUrlReplacePlugin(); const cleanupPlugin = makeCleanupPlugin(); - const extractPolyfillsPlugin = makeExtractPolyfillsPlugin(); const rrwebBuildPlugin = makeRrwebBuildPlugin({ excludeShadowDom: undefined, excludeIframe: undefined, @@ -121,10 +119,6 @@ export function makeBaseNPMConfig(options = {}) { ], }; - if (addPolyfills) { - defaultBaseConfig.plugins.push(extractPolyfillsPlugin); - } - return deepMerge(defaultBaseConfig, packageSpecificConfig, { // Plugins have to be in the correct order or everything breaks, so when merging we have to manually re-order them customMerge: key => (key === 'plugins' ? mergePlugins : undefined), diff --git a/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs b/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs deleted file mode 100644 index e0a21b400f35..000000000000 --- a/dev-packages/rollup-utils/plugins/extractPolyfillsPlugin.mjs +++ /dev/null @@ -1,214 +0,0 @@ -import * as path from 'path'; - -import * as acorn from 'acorn'; -import * as recast from 'recast'; - -const POLYFILL_NAMES = new Set([ - '_asyncNullishCoalesce', - '_asyncOptionalChain', - '_asyncOptionalChainDelete', - '_nullishCoalesce', - '_optionalChain', - '_optionalChainDelete', -]); - -/** - * Create a plugin which will replace function definitions of any of the above functions with an `import` or `require` - * statement pulling them in from a central source. Mimics tsc's `importHelpers` option. - */ -export function makeExtractPolyfillsPlugin() { - let moduleFormat; - - // For more on the hooks used in this plugin, see https://rollupjs.org/guide/en/#output-generation-hooks - return { - name: 'extractPolyfills', - - // Figure out which build we're currently in (esm or cjs) - outputOptions(options) { - moduleFormat = options.format; - }, - - // This runs after both the sucrase transpilation (which happens in the `transform` hook) and rollup's own - // esm-i-fying or cjs-i-fying work (which happens right before `renderChunk`), in other words, after all polyfills - // will have been injected - renderChunk(code, chunk) { - const sourceFile = chunk.fileName; - - // We don't want to pull the function definitions out of their actual sourcefiles, just the places where they've - // been injected - if (sourceFile.includes('buildPolyfills')) { - return null; - } - - // The index.js file of the core package will include identifiers named after polyfills so we would inject the - // polyfills, however that would override the exports so we should just skip that file. - const isCorePackage = process.cwd().endsWith(`packages${path.sep}core`); - if (isCorePackage && sourceFile === 'index.js') { - return null; - } - - const parserOptions = { - sourceFileName: sourceFile, - // We supply a custom parser which wraps the provided `acorn` parser in order to override the `ecmaVersion` value. - // See https://github.com/benjamn/recast/issues/578. - parser: { - parse(source, options) { - return acorn.parse(source, { - ...options, - // By this point in the build, everything should already have been down-compiled to whatever JS version - // we're targeting. Setting this parser to `latest` just means that whatever that version is (or changes - // to in the future), this parser will be able to handle the generated code. - ecmaVersion: 'latest', - }); - }, - }, - }; - - const ast = recast.parse(code, parserOptions); - - // Find function definitions and function expressions whose identifiers match a known polyfill name - const polyfillNodes = findPolyfillNodes(ast); - - if (polyfillNodes.length === 0) { - return null; - } - - console.log(`${sourceFile} - polyfills: ${polyfillNodes.map(node => node.name)}`); - - // Depending on the output format, generate `import { x, y, z } from '...'` or `var { x, y, z } = require('...')` - const importOrRequireNode = createImportOrRequireNode(polyfillNodes, sourceFile, moduleFormat); - - // Insert our new `import` or `require` node at the top of the file, and then delete the function definitions it's - // meant to replace (polyfill nodes get marked for deletion in `findPolyfillNodes`) - ast.program.body = [importOrRequireNode, ...ast.program.body.filter(node => !node.shouldDelete)]; - - // In spite of the name, this doesn't actually print anything - it just stringifies the code, and keeps track of - // where original nodes end up in order to generate a sourcemap. - const result = recast.print(ast, { - sourceMapName: `${sourceFile}.map`, - quote: 'single', - }); - - return { code: result.code, map: result.map }; - }, - }; -} - -/** - * Extract the function name, regardless of the format in which the function is declared - */ -function getNodeName(node) { - // Function expressions and functions pulled from objects - if (node.type === 'VariableDeclaration') { - // In practice sucrase and rollup only ever declare one polyfill at a time, so it's safe to just grab the first - // entry here - const declarationId = node.declarations[0].id; - - // Note: Sucrase and rollup seem to only use the first type of variable declaration for their polyfills, but good to - // cover our bases - - // Declarations of the form - // `const dogs = function() { return "are great"; };` - // or - // `const dogs = () => "are great"; - if (declarationId.type === 'Identifier') { - return declarationId.name; - } - // Declarations of the form - // `const { dogs } = { dogs: function() { return "are great"; } }` - // or - // `const { dogs } = { dogs: () => "are great" }` - else if (declarationId.type === 'ObjectPattern') { - return declarationId.properties[0].key.name; - } - // Any other format - else { - return 'unknown variable'; - } - } - - // Regular old functions, of the form - // `function dogs() { return "are great"; }` - else if (node.type === 'FunctionDeclaration') { - return node.id.name; - } - - // If we get here, this isn't a node we're interested in, so just return a string we know will never match any of the - // polyfill names - else { - return 'nope'; - } -} - -/** - * Find all nodes whose identifiers match a known polyfill name. - * - * Note: In theory, this could yield false positives, if any of the magic names were assigned to something other than a - * polyfill function, but the chances of that are slim. Also, it only searches the module global scope, but that's - * always where the polyfills appear, so no reason to traverse the whole tree. - */ -function findPolyfillNodes(ast) { - const isPolyfillNode = node => { - const nodeName = getNodeName(node); - if (POLYFILL_NAMES.has(nodeName)) { - // Mark this node for later deletion, since we're going to replace it with an import statement - node.shouldDelete = true; - // Store the name in a consistent spot, regardless of node type - node.name = nodeName; - - return true; - } - - return false; - }; - - return ast.program.body.filter(isPolyfillNode); -} - -/** - * Create a node representing an `import` or `require` statement of the form - * - * import { < polyfills > } from '...' - * or - * var { < polyfills > } = require('...') - * - * @param polyfillNodes The nodes from the current version of the code, defining the polyfill functions - * @param currentSourceFile The path, relative to `src/`, of the file currently being transpiled - * @param moduleFormat Either 'cjs' or 'esm' - * @returns A single node which can be subbed in for the polyfill definition nodes - */ -function createImportOrRequireNode(polyfillNodes, currentSourceFile, moduleFormat) { - const { - callExpression, - identifier, - importDeclaration, - importSpecifier, - literal, - objectPattern, - property, - variableDeclaration, - variableDeclarator, - } = recast.types.builders; - - // Since our polyfills live in `@sentry/core`, if we're importing or requiring them there the path will have to be - // relative - const isCorePackage = process.cwd().endsWith(path.join('packages', 'core')); - const importSource = literal( - isCorePackage ? `.${path.sep}${path.relative(path.dirname(currentSourceFile), 'buildPolyfills')}` : '@sentry/core', - ); - - // This is the `x, y, z` of inside of `import { x, y, z }` or `var { x, y, z }` - const importees = polyfillNodes.map(({ name: fnName }) => - moduleFormat === 'esm' - ? importSpecifier(identifier(fnName)) - : property.from({ kind: 'init', key: identifier(fnName), value: identifier(fnName), shorthand: true }), - ); - - const requireFn = identifier('require'); - - return moduleFormat === 'esm' - ? importDeclaration(importees, importSource) - : variableDeclaration('var', [ - variableDeclarator(objectPattern(importees), callExpression(requireFn, [importSource])), - ]); -} diff --git a/dev-packages/rollup-utils/plugins/npmPlugins.mjs b/dev-packages/rollup-utils/plugins/npmPlugins.mjs index 6597e2244ab8..5f577507b102 100644 --- a/dev-packages/rollup-utils/plugins/npmPlugins.mjs +++ b/dev-packages/rollup-utils/plugins/npmPlugins.mjs @@ -173,5 +173,3 @@ export function makeCodeCovPlugin() { uploadToken: process.env.CODECOV_TOKEN, }); } - -export { makeExtractPolyfillsPlugin } from './extractPolyfillsPlugin.mjs'; diff --git a/dev-packages/test-utils/.eslintrc.js b/dev-packages/test-utils/.eslintrc.js index 98318aea5c41..fdb9952bae52 100644 --- a/dev-packages/test-utils/.eslintrc.js +++ b/dev-packages/test-utils/.eslintrc.js @@ -3,12 +3,4 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - overrides: [ - { - files: ['**/*.ts'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, - ], }; diff --git a/packages/astro/.eslintrc.cjs b/packages/astro/.eslintrc.cjs index c706032aaf35..29b78099e7c6 100644 --- a/packages/astro/.eslintrc.cjs +++ b/packages/astro/.eslintrc.cjs @@ -11,12 +11,5 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/integration/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], }; diff --git a/packages/aws-serverless/.eslintrc.js b/packages/aws-serverless/.eslintrc.js index 99fcba0976da..d1d4c4e12aa0 100644 --- a/packages/aws-serverless/.eslintrc.js +++ b/packages/aws-serverless/.eslintrc.js @@ -3,9 +3,6 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/bun/.eslintrc.js b/packages/bun/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/bun/.eslintrc.js +++ b/packages/bun/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/cloudflare/.eslintrc.js b/packages/cloudflare/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/cloudflare/.eslintrc.js +++ b/packages/cloudflare/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/core/src/utils-hoist/buildPolyfills/README.md b/packages/core/src/utils-hoist/buildPolyfills/README.md deleted file mode 100644 index 3b18ad989133..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/README.md +++ /dev/null @@ -1,30 +0,0 @@ -## Build Polyfills - -This is a collection of syntax and import/export polyfills either copied directly from or heavily inspired by those used -by [Rollup](https://github.com/rollup/rollup) and [Sucrase](https://github.com/alangpierce/sucrase). When either tool -uses one of these polyfills during a build, it injects the function source code into each file needing the function, -which can lead to a great deal of duplication. For our builds, we have therefore implemented something similar to -[`tsc`'s `importHelpers` behavior](https://www.typescriptlang.org/tsconfig#importHelpers): Instead of leaving the -polyfills injected in multiple places, we instead replace each injected function with an `import` or `require` -statement. - -Note that not all polyfills are currently used by the SDK, but all are included here for future compatibility, should -they ever be needed. Also, since we're never going to be calling these directly from within another TS file, their types -are fairly generic. In some cases testing required more specific types, which can be found in the test files. - ---- - -_Code from both Rollup and Sucrase is used under the MIT license, copyright 2017 and 2012-2018, respectively._ - -_Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated -documentation files (the "Software"), to deal in the Software without restriction, including without limitation the -rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit -persons to whom the Software is furnished to do so, subject to the following conditions:_ - -_The above copyright notice and this permission notice shall be included in all copies or substantial portions of the -Software._ - -_THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE -WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR -COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE._ diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts deleted file mode 100644 index 032b31011f96..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncNullishCoalesce.ts +++ /dev/null @@ -1,51 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _nullishCoalesce } from './_nullishCoalesce'; - -/** - * Polyfill for the nullish coalescing operator (`??`), when used in situations where at least one of the values is the - * result of an async operation. - * - * Note that the RHS is wrapped in a function so that if it's a computed value, that evaluation won't happen unless the - * LHS evaluates to a nullish value, to mimic the operator's short-circuiting behavior. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * - * @param lhs The value of the expression to the left of the `??` - * @param rhsFn A function returning the value of the expression to the right of the `??` - * @returns The LHS value, unless it's `null` or `undefined`, in which case, the RHS value - */ -export async function _asyncNullishCoalesce(lhs: unknown, rhsFn: () => unknown): Promise { - return _nullishCoalesce(lhs, rhsFn); -} - -// Sucrase version: -// async function _asyncNullishCoalesce(lhs, rhsFn) { -// if (lhs != null) { -// return lhs; -// } else { -// return await rhsFn(); -// } -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts deleted file mode 100644 index 37489b5c9232..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChain.ts +++ /dev/null @@ -1,82 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import type { GenericFunction } from './types'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, for situations in which at least one part of the expression is async. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The value of the expression - */ -export async function _asyncOptionalChain(ops: unknown[]): Promise { - let lastAccessLHS: unknown = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i] as string; - const fn = ops[i + 1] as (intermediateValue: unknown) => Promise; - i += 2; - // by checking for loose equality to `null`, we catch both `null` and `undefined` - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - // really we're meaning to return `undefined` as an actual value here, but it saves bytes not to write it - return; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = await fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = await fn((...args: unknown[]) => (value as GenericFunction).call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// Sucrase version: -// async function _asyncOptionalChain(ops) { -// let lastAccessLHS = undefined; -// let value = ops[0]; -// let i = 1; -// while (i < ops.length) { -// const op = ops[i]; -// const fn = ops[i + 1]; -// i += 2; -// if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { -// return undefined; -// } -// if (op === 'access' || op === 'optionalAccess') { -// lastAccessLHS = value; -// value = await fn(value); -// } else if (op === 'call' || op === 'optionalCall') { -// value = await fn((...args) => value.call(lastAccessLHS, ...args)); -// lastAccessLHS = undefined; -// } -// } -// return value; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts b/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts deleted file mode 100644 index 9cef4fd791f0..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_asyncOptionalChainDelete.ts +++ /dev/null @@ -1,51 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _asyncOptionalChain } from './_asyncOptionalChain'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, in cases where the value of the expression is to be deleted. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The return value of the `delete` operator: `true`, unless the deletion target is an own, non-configurable - * property (one which can't be deleted or turned into an accessor, and whose enumerability can't be changed), in which - * case `false`. - */ -export async function _asyncOptionalChainDelete(ops: unknown[]): Promise { - const result = (await _asyncOptionalChain(ops)) as Promise; - // If `result` is `null`, it means we didn't get to the end of the chain and so nothing was deleted (in which case, - // return `true` since that's what `delete` does when it no-ops). If it's non-null, we know the delete happened, in - // which case we return whatever the `delete` returned, which will be a boolean. - return result == null ? true : (result as Promise); -} - -// Sucrase version: -// async function asyncOptionalChainDelete(ops) { -// const result = await ASYNC_OPTIONAL_CHAIN_NAME(ops); -// return result == null ? true : result; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts b/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts deleted file mode 100644 index a11cd469bf11..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_nullishCoalesce.ts +++ /dev/null @@ -1,49 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -/** - * Polyfill for the nullish coalescing operator (`??`). - * - * Note that the RHS is wrapped in a function so that if it's a computed value, that evaluation won't happen unless the - * LHS evaluates to a nullish value, to mimic the operator's short-circuiting behavior. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * - * @param lhs The value of the expression to the left of the `??` - * @param rhsFn A function returning the value of the expression to the right of the `??` - * @returns The LHS value, unless it's `null` or `undefined`, in which case, the RHS value - */ -export function _nullishCoalesce(lhs: unknown, rhsFn: () => unknown): unknown { - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return lhs != null ? lhs : rhsFn(); -} - -// Sucrase version: -// function _nullishCoalesce(lhs, rhsFn) { -// if (lhs != null) { -// return lhs; -// } else { -// return rhsFn(); -// } -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts b/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts deleted file mode 100644 index a7ea8338d744..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_optionalChain.ts +++ /dev/null @@ -1,82 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import type { GenericFunction } from './types'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) - * See https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The value of the expression - */ -export function _optionalChain(ops: unknown[]): unknown { - let lastAccessLHS: unknown = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i] as string; - const fn = ops[i + 1] as (intermediateValue: unknown) => unknown; - i += 2; - // by checking for loose equality to `null`, we catch both `null` and `undefined` - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - // really we're meaning to return `undefined` as an actual value here, but it saves bytes not to write it - return; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = fn((...args: unknown[]) => (value as GenericFunction).call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// Sucrase version -// function _optionalChain(ops) { -// let lastAccessLHS = undefined; -// let value = ops[0]; -// let i = 1; -// while (i < ops.length) { -// const op = ops[i]; -// const fn = ops[i + 1]; -// i += 2; -// if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { -// return undefined; -// } -// if (op === 'access' || op === 'optionalAccess') { -// lastAccessLHS = value; -// value = fn(value); -// } else if (op === 'call' || op === 'optionalCall') { -// value = fn((...args) => value.call(lastAccessLHS, ...args)); -// lastAccessLHS = undefined; -// } -// } -// return value; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts b/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts deleted file mode 100644 index 04bffc8ab385..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/_optionalChainDelete.ts +++ /dev/null @@ -1,52 +0,0 @@ -// https://github.com/alangpierce/sucrase/tree/265887868966917f3b924ce38dfad01fbab1329f -// -// The MIT License (MIT) -// -// Copyright (c) 2012-2018 various contributors (see AUTHORS) -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -import { _optionalChain } from './_optionalChain'; - -/** - * Polyfill for the optional chain operator, `?.`, given previous conversion of the expression into an array of values, - * descriptors, and functions, in cases where the value of the expression is to be deleted. - * - * Adapted from Sucrase (https://github.com/alangpierce/sucrase) See - * https://github.com/alangpierce/sucrase/blob/265887868966917f3b924ce38dfad01fbab1329f/src/transformers/OptionalChainingNullishTransformer.ts#L15 - * - * @param ops Array result of expression conversion - * @returns The return value of the `delete` operator: `true`, unless the deletion target is an own, non-configurable - * property (one which can't be deleted or turned into an accessor, and whose enumerability can't be changed), in which - * case `false`. - */ -export function _optionalChainDelete(ops: unknown[]): boolean { - const result = _optionalChain(ops) as boolean | null; - // If `result` is `null`, it means we didn't get to the end of the chain and so nothing was deleted (in which case, - // return `true` since that's what `delete` does when it no-ops). If it's non-null, we know the delete happened, in - // which case we return whatever the `delete` returned, which will be a boolean. - return result == null ? true : result; -} - -// Sucrase version: -// function _optionalChainDelete(ops) { -// const result = _optionalChain(ops); -// // by checking for loose equality to `null`, we catch both `null` and `undefined` -// return result == null ? true : result; -// } diff --git a/packages/core/src/utils-hoist/buildPolyfills/index.ts b/packages/core/src/utils-hoist/buildPolyfills/index.ts deleted file mode 100644 index 2017dcbd9592..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/index.ts +++ /dev/null @@ -1,6 +0,0 @@ -export { _asyncNullishCoalesce } from './_asyncNullishCoalesce'; -export { _asyncOptionalChain } from './_asyncOptionalChain'; -export { _asyncOptionalChainDelete } from './_asyncOptionalChainDelete'; -export { _nullishCoalesce } from './_nullishCoalesce'; -export { _optionalChain } from './_optionalChain'; -export { _optionalChainDelete } from './_optionalChainDelete'; diff --git a/packages/core/src/utils-hoist/buildPolyfills/types.ts b/packages/core/src/utils-hoist/buildPolyfills/types.ts deleted file mode 100644 index 10e2f4a944a4..000000000000 --- a/packages/core/src/utils-hoist/buildPolyfills/types.ts +++ /dev/null @@ -1,7 +0,0 @@ -import type { Primitive } from '../../types-hoist'; - -export type GenericObject = { [key: string]: Value }; -export type GenericFunction = (...args: unknown[]) => Value; -export type Value = Primitive | GenericFunction | GenericObject; - -export type RequireResult = GenericObject | (GenericFunction & GenericObject); diff --git a/packages/core/src/utils-hoist/index.ts b/packages/core/src/utils-hoist/index.ts index e53cd0edb59b..befb934905a2 100644 --- a/packages/core/src/utils-hoist/index.ts +++ b/packages/core/src/utils-hoist/index.ts @@ -179,10 +179,3 @@ export { SDK_VERSION } from './version'; export { getDebugImagesForResources, getFilenameToDebugIdMap } from './debug-ids'; export { escapeStringForRegex } from './vendor/escapeStringForRegex'; export { supportsHistory } from './vendor/supportsHistory'; - -export { _asyncNullishCoalesce } from './buildPolyfills/_asyncNullishCoalesce'; -export { _asyncOptionalChain } from './buildPolyfills/_asyncOptionalChain'; -export { _asyncOptionalChainDelete } from './buildPolyfills/_asyncOptionalChainDelete'; -export { _nullishCoalesce } from './buildPolyfills/_nullishCoalesce'; -export { _optionalChain } from './buildPolyfills/_optionalChain'; -export { _optionalChainDelete } from './buildPolyfills/_optionalChainDelete'; diff --git a/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts b/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts deleted file mode 100644 index 11c83b0711d9..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/nullishCoalesce.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -// TODO(v9): Remove this test - -import { _nullishCoalesce } from '../../../src/utils-hoist/buildPolyfills'; -import type { Value } from '../../../src/utils-hoist/buildPolyfills/types'; -import { _nullishCoalesce as _nullishCoalesceOrig } from './originals'; - -const dogStr = 'dogs are great!'; -const dogFunc = () => dogStr; -const dogAdjectives = { maisey: 'silly', charlie: 'goofy' }; -const dogAdjectiveFunc = () => dogAdjectives; - -describe('_nullishCoalesce', () => { - describe('returns the same result as the original', () => { - const testCases: Array<[string, Value, () => Value, Value]> = [ - ['null LHS', null, dogFunc, dogStr], - ['undefined LHS', undefined, dogFunc, dogStr], - ['false LHS', false, dogFunc, false], - ['zero LHS', 0, dogFunc, 0], - ['empty string LHS', '', dogFunc, ''], - ['true LHS', true, dogFunc, true], - ['truthy primitive LHS', 12312012, dogFunc, 12312012], - ['truthy object LHS', dogAdjectives, dogFunc, dogAdjectives], - ['truthy function LHS', dogAdjectiveFunc, dogFunc, dogAdjectiveFunc], - ]; - - it.each(testCases)('%s', (_, lhs, rhs, expectedValue) => { - expect(_nullishCoalesce(lhs, rhs)).toEqual(_nullishCoalesceOrig(lhs, rhs)); - expect(_nullishCoalesce(lhs, rhs)).toEqual(expectedValue); - }); - }); -}); diff --git a/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts b/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts deleted file mode 100644 index bd7a7bb052fb..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/optionalChain.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -// TODO(v9): Remove this test - -import { shim as arrayFlatShim } from 'array.prototype.flat'; - -import { _optionalChain } from '../../../src/utils-hoist/buildPolyfills'; -import type { GenericFunction, GenericObject, Value } from '../../../src/utils-hoist/buildPolyfills/types'; -import { _optionalChain as _optionalChainOrig } from './originals'; - -// Older versions of Node don't have `Array.prototype.flat`, which crashes these tests. On newer versions that do have -// it, this is a no-op. -arrayFlatShim(); - -type OperationType = 'access' | 'call' | 'optionalAccess' | 'optionalCall'; -type OperationExecutor = - | ((intermediateValue: GenericObject) => Value) - | ((intermediateValue: GenericFunction) => Value); -type Operation = [OperationType, OperationExecutor]; - -const truthyObject = { maisey: 'silly', charlie: 'goofy' }; -const nullishObject = null; -const truthyFunc = (): GenericObject => truthyObject; -const nullishFunc = undefined; -const truthyReturn = (): GenericObject => truthyObject; -const nullishReturn = (): null => nullishObject; - -// The polyfill being tested here works under the assumption that the original code containing the optional chain has -// been transformed into an array of values, labels, and functions. For example, `truthyObject?.charlie` will have been -// transformed into `_optionalChain([truthyObject, 'optionalAccess', _ => _.charlie])`. We are not testing the -// transformation here, only what the polyfill does with the already-transformed inputs. - -describe('_optionalChain', () => { - describe('returns the same result as the original', () => { - // In these test cases, the array passed to `_optionalChain` has been broken up into the first entry followed by an - // array of pairs of subsequent elements, because this seemed the easiest way to express the type, which is really - // - // [Value, OperationType, Value => Value, OperationType, Value => Value, OperationType, Value => Value, ...]. - // - // (In other words, `[A, B, C, D, E]` has become `A, [[B, C], [D, E]]`, and these are then the second and third - // entries in each test case.) We then undo this wrapping before passing the data to our functions. - const testCases: Array<[string, Value, Operation[], Value]> = [ - ['truthyObject?.charlie', truthyObject, [['optionalAccess', (_: GenericObject) => _.charlie]], 'goofy'], - ['nullishObject?.maisey', nullishObject, [['optionalAccess', (_: GenericObject) => _.maisey]], undefined], - [ - 'truthyFunc?.().maisey', - truthyFunc, - [ - ['optionalCall', (_: GenericFunction) => _()], - ['access', (_: GenericObject) => _.maisey], - ], - 'silly', - ], - [ - 'nullishFunc?.().charlie', - nullishFunc, - [ - ['optionalCall', (_: GenericFunction) => _()], - ['access', (_: GenericObject) => _.charlie], - ], - undefined, - ], - [ - 'truthyReturn()?.maisey', - truthyReturn, - [ - ['call', (_: GenericFunction) => _()], - ['optionalAccess', (_: GenericObject) => _.maisey], - ], - 'silly', - ], - [ - 'nullishReturn()?.charlie', - nullishReturn, - [ - ['call', (_: GenericFunction) => _()], - ['optionalAccess', (_: GenericObject) => _.charlie], - ], - undefined, - ], - ]; - - it.each(testCases)('%s', (_, initialChainComponent, operations, expectedValue) => { - // `operations` is flattened and spread in order to undo the wrapping done in the test cases for TS purposes. - // @ts-expect-error this is what we're testing - expect(_optionalChain([initialChainComponent, ...operations.flat()])).toEqual( - // @ts-expect-error this is what we're testing - _optionalChainOrig([initialChainComponent, ...operations.flat()]), - ); - // @ts-expect-error this is what we're testing - expect(_optionalChain([initialChainComponent, ...operations.flat()])).toEqual(expectedValue); - }); - }); -}); diff --git a/packages/core/test/utils-hoist/buildPolyfills/originals.ts b/packages/core/test/utils-hoist/buildPolyfills/originals.ts deleted file mode 100644 index a504d5f0d871..000000000000 --- a/packages/core/test/utils-hoist/buildPolyfills/originals.ts +++ /dev/null @@ -1,85 +0,0 @@ -// eslint-disable-next-line @typescript-eslint/ban-ts-comment -// @ts-nocheck this is just used for tests - -// TODO(v9): Remove this file - -// Originals of the buildPolyfills from Sucrase and Rollup we use (which we have adapted in various ways), preserved here for testing, to prove that -// the modified versions do the same thing the originals do. - -// From Sucrase -export function _asyncNullishCoalesce(lhs, rhsFn) { - if (lhs != null) { - return lhs; - } else { - return rhsFn(); - } -} - -// From Sucrase -export async function _asyncOptionalChain(ops) { - let lastAccessLHS = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i]; - const fn = ops[i + 1]; - i += 2; - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - return undefined; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = await fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = await fn((...args) => value.call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// From Sucrase -export async function _asyncOptionalChainDelete(ops) { - const result = await _asyncOptionalChain(ops); - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return result == null ? true : result; -} - -// From Sucrase -export function _nullishCoalesce(lhs, rhsFn) { - if (lhs != null) { - return lhs; - } else { - return rhsFn(); - } -} - -// From Sucrase -export function _optionalChain(ops) { - let lastAccessLHS = undefined; - let value = ops[0]; - let i = 1; - while (i < ops.length) { - const op = ops[i]; - const fn = ops[i + 1]; - i += 2; - if ((op === 'optionalAccess' || op === 'optionalCall') && value == null) { - return undefined; - } - if (op === 'access' || op === 'optionalAccess') { - lastAccessLHS = value; - value = fn(value); - } else if (op === 'call' || op === 'optionalCall') { - value = fn((...args) => value.call(lastAccessLHS, ...args)); - lastAccessLHS = undefined; - } - } - return value; -} - -// From Sucrase -export function _optionalChainDelete(ops) { - const result = _optionalChain(ops); - // by checking for loose equality to `null`, we catch both `null` and `undefined` - return result == null ? true : result; -} diff --git a/packages/deno/.eslintrc.js b/packages/deno/.eslintrc.js index 0c539a61b1b2..5a8ccd2be035 100644 --- a/packages/deno/.eslintrc.js +++ b/packages/deno/.eslintrc.js @@ -2,8 +2,6 @@ module.exports = { extends: ['../../.eslintrc.js'], ignorePatterns: ['lib.deno.d.ts', 'scripts/*.mjs', 'build-types/**', 'build-test/**', 'build/**'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/eslint-config-sdk/src/base.js b/packages/eslint-config-sdk/src/base.js index 525b24b4a334..c137729161c5 100644 --- a/packages/eslint-config-sdk/src/base.js +++ b/packages/eslint-config-sdk/src/base.js @@ -130,11 +130,6 @@ module.exports = { }, ], - // We want to prevent optional chaining & nullish coalescing usage in our files - // to prevent unnecessary bundle size. Turned off in tests. - '@sentry-internal/sdk/no-optional-chaining': 'error', - '@sentry-internal/sdk/no-nullish-coalescing': 'error', - // We want to avoid using the RegExp constructor as it can lead to invalid or dangerous regular expressions // if end user input is used in the constructor. It's fine to ignore this rule if it is safe to use the RegExp. // However, we want to flag each use case so that we're aware of the potential danger. @@ -181,8 +176,6 @@ module.exports = { '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/no-non-null-assertion': 'off', '@typescript-eslint/no-empty-function': 'off', - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@typescript-eslint/no-floating-promises': 'off', '@sentry-internal/sdk/no-focused-tests': 'error', '@sentry-internal/sdk/no-skipped-tests': 'error', diff --git a/packages/eslint-plugin-sdk/src/index.js b/packages/eslint-plugin-sdk/src/index.js index d7516c343d60..24cc9c4cc00c 100644 --- a/packages/eslint-plugin-sdk/src/index.js +++ b/packages/eslint-plugin-sdk/src/index.js @@ -10,8 +10,6 @@ module.exports = { rules: { - 'no-optional-chaining': require('./rules/no-optional-chaining'), - 'no-nullish-coalescing': require('./rules/no-nullish-coalescing'), 'no-eq-empty': require('./rules/no-eq-empty'), 'no-class-field-initializers': require('./rules/no-class-field-initializers'), 'no-regexp-constructor': require('./rules/no-regexp-constructor'), diff --git a/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js b/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js deleted file mode 100644 index 8944f2755632..000000000000 --- a/packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @fileoverview disallow nullish coalescing operators as they were introduced only in ES2020 and hence require - * us to add a polyfill. This increases bundle size more than avoiding nullish coalescing operators all together. - * - * @author Lukas Stracke - * - * Based on: https://github.com/mysticatea/eslint-plugin-es/blob/v4.1.0/lib/rules/no-nullish-coalescing-operators.js - */ -'use strict'; - -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ - -module.exports = { - meta: { - type: 'problem', - docs: { - description: 'disallow nullish coalescing operators.', - category: 'Best Practices', - recommended: true, - }, - messages: { - forbidden: 'Avoid using nullish coalescing operators.', - }, - fixable: null, - schema: [], - }, - create(context) { - return { - "LogicalExpression[operator='??']"(node) { - context.report({ - node: context.getSourceCode().getTokenAfter(node.left, isNullishCoalescingOperator), - messageId: 'forbidden', - }); - }, - }; - }, -}; - -/** - * Checks if the given token is a nullish coalescing operator or not. - * @param {Token} token - The token to check. - * @returns {boolean} `true` if the token is a nullish coalescing operator. - */ -function isNullishCoalescingOperator(token) { - return token.value === '??' && token.type === 'Punctuator'; -} diff --git a/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js b/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js deleted file mode 100644 index 67da656bc782..000000000000 --- a/packages/eslint-plugin-sdk/src/rules/no-optional-chaining.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * @fileoverview Rule to disallow using optional chaining - because this is transpiled into verbose code. - * @author Francesco Novy - * - * Based on https://github.com/facebook/lexical/pull/3233 - */ -'use strict'; - -// ------------------------------------------------------------------------------ -// Rule Definition -// ------------------------------------------------------------------------------ - -/** @type {import('eslint').Rule.RuleModule} */ -module.exports = { - meta: { - type: 'problem', - docs: { - description: 'disallow usage of optional chaining', - category: 'Best Practices', - recommended: true, - }, - messages: { - forbidden: 'Avoid using optional chaining', - }, - fixable: null, - schema: [], - }, - - create(context) { - const sourceCode = context.getSourceCode(); - - /** - * Checks if the given token is a `?.` token or not. - * @param {Token} token The token to check. - * @returns {boolean} `true` if the token is a `?.` token. - */ - function isQuestionDotToken(token) { - return ( - token.value === '?.' && - (token.type === 'Punctuator' || // espree has been parsed well. - // espree@7.1.0 doesn't parse "?." tokens well. Therefore, get the string from the source code and check it. - sourceCode.getText(token) === '?.') - ); - } - - return { - 'CallExpression[optional=true]'(node) { - context.report({ - messageId: 'forbidden', - node: sourceCode.getTokenAfter(node.callee, isQuestionDotToken), - }); - }, - 'MemberExpression[optional=true]'(node) { - context.report({ - messageId: 'forbidden', - node: sourceCode.getTokenAfter(node.object, isQuestionDotToken), - }); - }, - }; - }, -}; diff --git a/packages/google-cloud-serverless/.eslintrc.js b/packages/google-cloud-serverless/.eslintrc.js index 99fcba0976da..d1d4c4e12aa0 100644 --- a/packages/google-cloud-serverless/.eslintrc.js +++ b/packages/google-cloud-serverless/.eslintrc.js @@ -3,9 +3,6 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/nextjs/.eslintrc.js b/packages/nextjs/.eslintrc.js index 95ce15bc668f..1525e502018f 100644 --- a/packages/nextjs/.eslintrc.js +++ b/packages/nextjs/.eslintrc.js @@ -7,10 +7,6 @@ module.exports = { jsx: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/nitro-utils/.eslintrc.js b/packages/nitro-utils/.eslintrc.js index 3849c1ee28a6..7ac3732750a5 100644 --- a/packages/nitro-utils/.eslintrc.js +++ b/packages/nitro-utils/.eslintrc.js @@ -3,19 +3,4 @@ module.exports = { env: { node: true, }, - overrides: [ - { - files: ['src/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, - { - files: ['src/metrics/**'], - rules: { - '@typescript-eslint/explicit-function-return-type': 'off', - '@typescript-eslint/no-non-null-assertion': 'off', - }, - }, - ], }; diff --git a/packages/node/.eslintrc.js b/packages/node/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/node/.eslintrc.js +++ b/packages/node/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/nuxt/.eslintrc.js b/packages/nuxt/.eslintrc.js index d567b12530d0..a22f9710cf6b 100644 --- a/packages/nuxt/.eslintrc.js +++ b/packages/nuxt/.eslintrc.js @@ -10,13 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/opentelemetry/.eslintrc.js b/packages/opentelemetry/.eslintrc.js index 9899ea1b73d8..fdb9952bae52 100644 --- a/packages/opentelemetry/.eslintrc.js +++ b/packages/opentelemetry/.eslintrc.js @@ -3,7 +3,4 @@ module.exports = { node: true, }, extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, }; diff --git a/packages/profiling-node/.eslintrc.js b/packages/profiling-node/.eslintrc.js index 02a39c9cf562..e4cb54c2f08c 100644 --- a/packages/profiling-node/.eslintrc.js +++ b/packages/profiling-node/.eslintrc.js @@ -6,8 +6,6 @@ module.exports = { ignorePatterns: ['lib/**/*', 'examples/**/*', 'jest.co'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/remix/.eslintrc.js b/packages/remix/.eslintrc.js index 992bcd3e9d2b..f1046a6136d6 100644 --- a/packages/remix/.eslintrc.js +++ b/packages/remix/.eslintrc.js @@ -8,9 +8,6 @@ module.exports = { }, ignorePatterns: ['playwright.config.ts', 'vitest.config.ts', 'test/integration/**'], extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, overrides: [ { files: ['scripts/**/*.ts'], diff --git a/packages/solidstart/.eslintrc.js b/packages/solidstart/.eslintrc.js index d567b12530d0..a22f9710cf6b 100644 --- a/packages/solidstart/.eslintrc.js +++ b/packages/solidstart/.eslintrc.js @@ -10,13 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/.eslintrc.js b/packages/sveltekit/.eslintrc.js index c1f55c94aadf..a22f9710cf6b 100644 --- a/packages/sveltekit/.eslintrc.js +++ b/packages/sveltekit/.eslintrc.js @@ -10,12 +10,6 @@ module.exports = { project: ['tsconfig.test.json'], }, }, - { - files: ['src/vite/**', 'src/server/**'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, - }, ], extends: ['../../.eslintrc.js'], }; diff --git a/packages/sveltekit/src/vite/autoInstrument.ts b/packages/sveltekit/src/vite/autoInstrument.ts index 28e903d89db7..7194a3ae3ac8 100644 --- a/packages/sveltekit/src/vite/autoInstrument.ts +++ b/packages/sveltekit/src/vite/autoInstrument.ts @@ -1,6 +1,5 @@ import * as fs from 'fs'; import * as path from 'path'; -/* eslint-disable @sentry-internal/sdk/no-optional-chaining */ import type { ExportNamedDeclaration } from '@babel/types'; import { parseModule } from 'magicast'; import type { Plugin } from 'vite'; diff --git a/packages/sveltekit/src/vite/svelteConfig.ts b/packages/sveltekit/src/vite/svelteConfig.ts index 9186e46caba6..02edad15382e 100644 --- a/packages/sveltekit/src/vite/svelteConfig.ts +++ b/packages/sveltekit/src/vite/svelteConfig.ts @@ -1,5 +1,3 @@ -/* eslint-disable @sentry-internal/sdk/no-optional-chaining */ - import * as fs from 'fs'; import * as path from 'path'; import * as url from 'url'; diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 1d8d712e5b0f..842f8475905d 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -14,13 +14,7 @@ import { SyncPromise as SyncPromise_imported, TRACEPARENT_REGEXP as TRACEPARENT_REGEXP_imported, UNKNOWN_FUNCTION as UNKNOWN_FUNCTION_imported, - _asyncNullishCoalesce as _asyncNullishCoalesce_imported, - _asyncOptionalChain as _asyncOptionalChain_imported, - _asyncOptionalChainDelete as _asyncOptionalChainDelete_imported, _browserPerformanceTimeOriginMode as _browserPerformanceTimeOriginMode_imported, - _nullishCoalesce as _nullishCoalesce_imported, - _optionalChain as _optionalChain_imported, - _optionalChainDelete as _optionalChainDelete_imported, addConsoleInstrumentationHandler as addConsoleInstrumentationHandler_imported, addContextToFrame as addContextToFrame_imported, addExceptionMechanism as addExceptionMechanism_imported, @@ -646,24 +640,6 @@ export const extractRequestData = extractRequestData_imported; // eslint-disable-next-line deprecation/deprecation export const addRequestDataToEvent = addRequestDataToEvent_imported; -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncNullishCoalesce = _asyncNullishCoalesce_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncOptionalChain = _asyncOptionalChain_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _asyncOptionalChainDelete = _asyncOptionalChainDelete_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _nullishCoalesce = _nullishCoalesce_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _optionalChain = _optionalChain_imported; - -/** @deprecated Import from `@sentry/core` instead. */ -export const _optionalChainDelete = _optionalChainDelete_imported; - /** @deprecated Import from `@sentry/core` instead. */ // eslint-disable-next-line deprecation/deprecation export const BAGGAGE_HEADER_NAME = BAGGAGE_HEADER_NAME_imported; diff --git a/packages/vercel-edge/.eslintrc.js b/packages/vercel-edge/.eslintrc.js index 9d915d4f4c3b..6da218bd8641 100644 --- a/packages/vercel-edge/.eslintrc.js +++ b/packages/vercel-edge/.eslintrc.js @@ -4,8 +4,6 @@ module.exports = { }, extends: ['../../.eslintrc.js'], rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - '@sentry-internal/sdk/no-nullish-coalescing': 'off', '@sentry-internal/sdk/no-class-field-initializers': 'off', }, }; diff --git a/packages/wasm/.eslintrc.js b/packages/wasm/.eslintrc.js index fefcfd5cb729..5a2cc7f1ec08 100644 --- a/packages/wasm/.eslintrc.js +++ b/packages/wasm/.eslintrc.js @@ -1,6 +1,3 @@ module.exports = { extends: ['../../.eslintrc.js'], - rules: { - '@sentry-internal/sdk/no-optional-chaining': 'off', - }, }; From a2ca9b06eaf2f9d3669a0b8a244bd10df098a74c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 13:20:04 +0000 Subject: [PATCH 22/38] crashed and errored --- packages/core/src/server-runtime-client.ts | 24 ++++++++++++------- .../http/SentryHttpInstrumentation.ts | 17 +++++++------ 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 6763f66adb4f..1037b88f7db9 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -79,8 +79,7 @@ export class ServerRuntimeClient< * @inheritDoc */ public captureException(exception: unknown, hint?: EventHint, scope?: Scope): string { - // TODO: Check if mechanism === unhandled - setCurrentRequestSessionCrashed(); + setCurrentRequestSessionErroredOrCrashed(hint); return super.captureException(exception, hint, scope); } @@ -91,8 +90,7 @@ export class ServerRuntimeClient< // If the event is of type Exception, then a request session should be captured const isException = !event.type && event.exception && event.exception.values && event.exception.values.length > 0; if (isException) { - // TODO: Check if mechanism === unhandled - setCurrentRequestSessionCrashed(); + setCurrentRequestSessionErroredOrCrashed(hint); } return super.captureEvent(event, hint, scope); @@ -207,17 +205,25 @@ export class ServerRuntimeClient< } } -function setCurrentRequestSessionCrashed(): void { +function setCurrentRequestSessionErroredOrCrashed(eventHint?: EventHint): void { + const isHandledException = eventHint?.mechanism?.handled ?? true; + const requestSession = getIsolationScope().getScopeData().sdkProcessingMetadata.requestSession as - | { status: 'ok' | 'crashed' } + | { status: 'ok' | 'errored' | 'crashed' } | undefined; if (requestSession) { // We mutate instead of doing `setSdkProcessingMetadata` because the http integration stores away a particular // isolationScope. If that isolation scope is forked, setting the processing metadata here will not mutate the // original isolation scope that the http integration stored away. - requestSession.status = 'crashed'; - - // TODO maybe send 'errored' when handled exception? + if (isHandledException) { + // A request session can go from "errored" -> "crashed" but not "crashed" -> "errored". + // Crashed (unhandled exception) is worse than errored (handled exception). + if (requestSession.status !== 'crashed') { + requestSession.status = 'errored'; + } + } else { + requestSession.status = 'crashed'; + } } } diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 4c8925ac1119..506909e31575 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -24,11 +24,11 @@ import { getRequestInfo } from './vendor/getRequestInfo'; const clientToAggregatesMap = new Map< Client, - { [timestampRoundedToSeconds: string]: { exited: number; crashed: number } } + { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } >(); interface RequestSession { - status: 'ok' | 'crashed'; + status: 'ok' | 'errored' | 'crashed'; } type Http = typeof http; @@ -186,20 +186,20 @@ export class SentryHttpInstrumentation extends InstrumentationBase { DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); flushPendingClientAggregates(); - // TODO: Increase to 60s - }, 5_000).unref(); + }, 60_000).unref(); } } }); From 64ab7f64851f3201b2e8bfad6a46d18fe0fd79bb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 14:02:01 +0000 Subject: [PATCH 23/38] rm unneeded tests --- packages/core/src/index.ts | 2 - packages/core/src/sessionflusher.ts | 124 -------- packages/core/src/types-hoist/index.ts | 6 - packages/core/src/types-hoist/session.ts | 23 +- packages/core/test/lib/sessionflusher.test.ts | 129 -------- .../node/test/integrations/express.test.ts | 144 --------- packages/node/test/sdk/client.test.ts | 283 +----------------- packages/types/src/index.ts | 4 - 8 files changed, 7 insertions(+), 708 deletions(-) delete mode 100644 packages/core/src/sessionflusher.ts delete mode 100644 packages/core/test/lib/sessionflusher.test.ts delete mode 100644 packages/node/test/integrations/express.test.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 77259d2434d4..1a083ec86274 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -47,8 +47,6 @@ export { export { setAsyncContextStrategy } from './asyncContext'; export { getMainCarrier } from './carrier'; export { makeSession, closeSession, updateSession } from './session'; -// eslint-disable-next-line deprecation/deprecation -export { SessionFlusher } from './sessionflusher'; export { Scope } from './scope'; export { notifyEventProcessors } from './eventProcessors'; export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from './api'; diff --git a/packages/core/src/sessionflusher.ts b/packages/core/src/sessionflusher.ts deleted file mode 100644 index 2434023bf797..000000000000 --- a/packages/core/src/sessionflusher.ts +++ /dev/null @@ -1,124 +0,0 @@ -import { getIsolationScope } from './currentScopes'; -import type { - AggregationCounts, - Client, - RequestSessionStatus, - SessionAggregates, - SessionFlusherLike, -} from './types-hoist'; -import { dropUndefinedKeys } from './utils-hoist/object'; - -type ReleaseHealthAttributes = { - environment?: string; - release: string; -}; - -/** - * @deprecated `SessionFlusher` is deprecated and will be removed in the next major version of the SDK. - */ -// TODO(v9): The goal for the SessionFlusher is to become a stupidly simple mechanism to aggregate "Sessions" (actually "RequestSessions"). It should probably live directly inside the Http integration/instrumentation. -// eslint-disable-next-line deprecation/deprecation -export class SessionFlusher implements SessionFlusherLike { - public readonly flushTimeout: number; - private _pendingAggregates: Map; - private _sessionAttrs: ReleaseHealthAttributes; - // We adjust the type here to add the `unref()` part, as setInterval can technically return a number or a NodeJS.Timer - private readonly _intervalId: ReturnType & { unref?: () => void }; - private _isEnabled: boolean; - private _client: Client; - - public constructor(client: Client, attrs: ReleaseHealthAttributes) { - this._client = client; - this.flushTimeout = 60; - this._pendingAggregates = new Map(); - this._isEnabled = true; - - // Call to setInterval, so that flush is called every 60 seconds. - this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000); - if (this._intervalId.unref) { - this._intervalId.unref(); - } - this._sessionAttrs = attrs; - } - - /** Checks if `pendingAggregates` has entries, and if it does flushes them by calling `sendSession` */ - public flush(): void { - const sessionAggregates = this.getSessionAggregates(); - if (sessionAggregates.aggregates.length === 0) { - return; - } - this._pendingAggregates = new Map(); - this._client.sendSession(sessionAggregates); - } - - /** Massages the entries in `pendingAggregates` and returns aggregated sessions */ - public getSessionAggregates(): SessionAggregates { - const aggregates: AggregationCounts[] = Array.from(this._pendingAggregates.values()); - - const sessionAggregates: SessionAggregates = { - attrs: this._sessionAttrs, - aggregates, - }; - return dropUndefinedKeys(sessionAggregates); - } - - /** JSDoc */ - public close(): void { - clearInterval(this._intervalId); - this._isEnabled = false; - this.flush(); - } - - /** - * Wrapper function for _incrementSessionStatusCount that checks if the instance of SessionFlusher is enabled then - * fetches the session status of the request from `Scope.getRequestSession().status` on the scope and passes them to - * `_incrementSessionStatusCount` along with the start date - */ - public incrementSessionStatusCount(): void { - if (!this._isEnabled) { - return; - } - const isolationScope = getIsolationScope(); - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - - if (requestSession && requestSession.status) { - this._incrementSessionStatusCount(requestSession.status, new Date()); - // This is not entirely necessarily but is added as a safe guard to indicate the bounds of a request and so in - // case captureRequestSession is called more than once to prevent double count - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession(undefined); - /* eslint-enable @typescript-eslint/no-unsafe-member-access */ - } - } - - /** - * Increments status bucket in pendingAggregates buffer (internal state) corresponding to status of - * the session received - */ - // eslint-disable-next-line deprecation/deprecation - private _incrementSessionStatusCount(status: RequestSessionStatus, date: Date): number { - // Truncate minutes and seconds on Session Started attribute to have one minute bucket keys - const sessionStartedTrunc = new Date(date).setSeconds(0, 0); - - // corresponds to aggregated sessions in one specific minute bucket - // for example, {"started":"2021-03-16T08:00:00.000Z","exited":4, "errored": 1} - let aggregationCounts = this._pendingAggregates.get(sessionStartedTrunc); - if (!aggregationCounts) { - aggregationCounts = { started: new Date(sessionStartedTrunc).toISOString() }; - this._pendingAggregates.set(sessionStartedTrunc, aggregationCounts); - } - - switch (status) { - case 'errored': - aggregationCounts.errored = (aggregationCounts.errored || 0) + 1; - return aggregationCounts.errored; - case 'ok': - aggregationCounts.exited = (aggregationCounts.exited || 0) + 1; - return aggregationCounts.exited; - default: - aggregationCounts.crashed = (aggregationCounts.crashed || 0) + 1; - return aggregationCounts.crashed; - } - } -} diff --git a/packages/core/src/types-hoist/index.ts b/packages/core/src/types-hoist/index.ts index 3433c17092cf..baa94145385a 100644 --- a/packages/core/src/types-hoist/index.ts +++ b/packages/core/src/types-hoist/index.ts @@ -106,12 +106,6 @@ export type { Session, SessionContext, SessionStatus, - // eslint-disable-next-line deprecation/deprecation - RequestSession, - // eslint-disable-next-line deprecation/deprecation - RequestSessionStatus, - // eslint-disable-next-line deprecation/deprecation - SessionFlusherLike, SerializedSession, } from './session'; diff --git a/packages/core/src/types-hoist/session.ts b/packages/core/src/types-hoist/session.ts index 47cfa348acbb..4adc77240f4f 100644 --- a/packages/core/src/types-hoist/session.ts +++ b/packages/core/src/types-hoist/session.ts @@ -54,27 +54,14 @@ export interface SessionAggregates { aggregates: Array; } -/** - * @deprecated This type is deprecated and will be removed in the next major version of the SDK. - */ -export interface SessionFlusherLike { - /** - * Increments the Session Status bucket in SessionAggregates Object corresponding to the status of the session - * captured - */ - incrementSessionStatusCount(): void; - - /** Empties Aggregate Buckets and Sends them to Transport Buffer */ - flush(): void; - - /** Clears setInterval and calls flush */ - close(): void; -} - export interface AggregationCounts { + /** ISO Timestamp rounded to the second */ started: string; - errored?: number; + /** Number of sessions that did not have errors */ exited?: number; + /** Number of sessions that had handled errors */ + errored?: number; + /** Number of sessions that had unhandled errors */ crashed?: number; } diff --git a/packages/core/test/lib/sessionflusher.test.ts b/packages/core/test/lib/sessionflusher.test.ts deleted file mode 100644 index 53fe930b58c2..000000000000 --- a/packages/core/test/lib/sessionflusher.test.ts +++ /dev/null @@ -1,129 +0,0 @@ -import type { Client } from '../../src/types-hoist'; - -import { SessionFlusher } from '../../src/sessionflusher'; - -describe('Session Flusher', () => { - let sendSession: jest.Mock; - let mockClient: Client; - - beforeEach(() => { - jest.useFakeTimers(); - sendSession = jest.fn(() => Promise.resolve({ status: 'success' })); - mockClient = { - sendSession, - } as unknown as Client; - }); - - afterEach(() => { - jest.restoreAllMocks(); - }); - - test('test incrementSessionStatusCount updates the internal SessionFlusher state', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' }); - - const date = new Date('2021-04-08T12:18:23.043Z'); - let count = (flusher as any)._incrementSessionStatusCount('ok', date); - expect(count).toEqual(1); - count = (flusher as any)._incrementSessionStatusCount('ok', date); - expect(count).toEqual(2); - count = (flusher as any)._incrementSessionStatusCount('errored', date); - expect(count).toEqual(1); - date.setMinutes(date.getMinutes() + 1); - count = (flusher as any)._incrementSessionStatusCount('ok', date); - expect(count).toEqual(1); - count = (flusher as any)._incrementSessionStatusCount('errored', date); - expect(count).toEqual(1); - - expect(flusher.getSessionAggregates().aggregates).toEqual([ - { errored: 1, exited: 2, started: '2021-04-08T12:18:00.000Z' }, - { errored: 1, exited: 1, started: '2021-04-08T12:19:00.000Z' }, - ]); - expect(flusher.getSessionAggregates().attrs).toEqual({ release: '1.0.0', environment: 'dev' }); - }); - - test('test undefined attributes are excluded, on incrementSessionStatusCount call', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.0' }); - - const date = new Date('2021-04-08T12:18:23.043Z'); - (flusher as any)._incrementSessionStatusCount('ok', date); - (flusher as any)._incrementSessionStatusCount('errored', date); - - expect(flusher.getSessionAggregates()).toEqual({ - aggregates: [{ errored: 1, exited: 1, started: '2021-04-08T12:18:00.000Z' }], - attrs: { release: '1.0.0' }, - }); - }); - - test('flush is called every 60 seconds after initialisation of an instance of SessionFlusher', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' }); - const flusherFlushFunc = jest.spyOn(flusher, 'flush'); - jest.advanceTimersByTime(59000); - expect(flusherFlushFunc).toHaveBeenCalledTimes(0); - jest.advanceTimersByTime(2000); - expect(flusherFlushFunc).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(58000); - expect(flusherFlushFunc).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(2000); - expect(flusherFlushFunc).toHaveBeenCalledTimes(2); - }); - - test('sendSessions is called on flush if sessions were captured', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' }); - const flusherFlushFunc = jest.spyOn(flusher, 'flush'); - const date = new Date('2021-04-08T12:18:23.043Z'); - (flusher as any)._incrementSessionStatusCount('ok', date); - (flusher as any)._incrementSessionStatusCount('ok', date); - - expect(sendSession).toHaveBeenCalledTimes(0); - - jest.advanceTimersByTime(61000); - - expect(flusherFlushFunc).toHaveBeenCalledTimes(1); - expect(sendSession).toHaveBeenCalledWith( - expect.objectContaining({ - attrs: { release: '1.0.0', environment: 'dev' }, - aggregates: [{ started: '2021-04-08T12:18:00.000Z', exited: 2 }], - }), - ); - }); - - test('sendSessions is not called on flush if no sessions were captured', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.0', environment: 'dev' }); - const flusherFlushFunc = jest.spyOn(flusher, 'flush'); - - expect(sendSession).toHaveBeenCalledTimes(0); - jest.advanceTimersByTime(61000); - expect(flusherFlushFunc).toHaveBeenCalledTimes(1); - expect(sendSession).toHaveBeenCalledTimes(0); - }); - - test('calling close on SessionFlusher should disable SessionFlusher', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.x' }); - flusher.close(); - expect((flusher as any)._isEnabled).toEqual(false); - }); - - test('calling close on SessionFlusher will force call flush', () => { - // eslint-disable-next-line deprecation/deprecation - const flusher = new SessionFlusher(mockClient, { release: '1.0.x' }); - const flusherFlushFunc = jest.spyOn(flusher, 'flush'); - const date = new Date('2021-04-08T12:18:23.043Z'); - (flusher as any)._incrementSessionStatusCount('ok', date); - (flusher as any)._incrementSessionStatusCount('ok', date); - flusher.close(); - - expect(flusherFlushFunc).toHaveBeenCalledTimes(1); - expect(sendSession).toHaveBeenCalledWith( - expect.objectContaining({ - attrs: { release: '1.0.x' }, - aggregates: [{ started: '2021-04-08T12:18:00.000Z', exited: 2 }], - }), - ); - }); -}); diff --git a/packages/node/test/integrations/express.test.ts b/packages/node/test/integrations/express.test.ts deleted file mode 100644 index 1213e7bb38cf..000000000000 --- a/packages/node/test/integrations/express.test.ts +++ /dev/null @@ -1,144 +0,0 @@ -import * as http from 'http'; -import { getCurrentScope, getIsolationScope, setAsyncContextStrategy, setCurrentClient, withScope } from '@sentry/core'; -import type { Scope } from '@sentry/core'; -import { expressErrorHandler } from '../../src/integrations/tracing/express'; -import { NodeClient } from '../../src/sdk/client'; -import { getDefaultNodeClientOptions } from '../helpers/getDefaultNodeClientOptions'; - -describe('expressErrorHandler()', () => { - beforeEach(() => { - getCurrentScope().clear(); - getIsolationScope().clear(); - - setAsyncContextStrategy(undefined); - }); - - const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' }; - const method = 'wagging'; - const protocol = 'mutualsniffing'; - const hostname = 'the.dog.park'; - const path = '/by/the/trees/'; - const queryString = 'chase=me&please=thankyou'; - - const sentryErrorMiddleware = expressErrorHandler(); - - let req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined; - let client: NodeClient; - - function createNoOpSpy() { - const noop = { noop: () => undefined }; // this is wrapped in an object so jest can spy on it - return jest.spyOn(noop, 'noop') as any; - } - - beforeEach(() => { - req = { - headers, - method, - protocol, - hostname, - originalUrl: `${path}?${queryString}`, - } as unknown as http.IncomingMessage; - res = new http.ServerResponse(req); - next = createNoOpSpy(); - }); - - afterEach(() => { - if (client['_sessionFlusher']) { - clearInterval(client['_sessionFlusher']['_intervalId']); - } - jest.restoreAllMocks(); - }); - it('when autoSessionTracking is disabled, does not set requestSession status on Crash', done => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '3.3' }); - client = new NodeClient(options); - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - setCurrentClient(client); - - jest.spyOn(client, '_captureRequestSession'); - - // eslint-disable-next-line deprecation/deprecation - getIsolationScope().setRequestSession({ status: 'ok' }); - - let isolationScope: Scope; - sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => { - isolationScope = getIsolationScope(); - return next(); - }); - - setImmediate(() => { - // eslint-disable-next-line deprecation/deprecation - expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' }); - done(); - }); - }); - - it('autoSessionTracking is enabled + requestHandler is not used -> does not set requestSession status on Crash', done => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '3.3' }); - client = new NodeClient(options); - setCurrentClient(client); - - jest.spyOn(client, '_captureRequestSession'); - - // eslint-disable-next-line deprecation/deprecation - getIsolationScope().setRequestSession({ status: 'ok' }); - - let isolationScope: Scope; - sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => { - isolationScope = getIsolationScope(); - return next(); - }); - - setImmediate(() => { - // eslint-disable-next-line deprecation/deprecation - expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' }); - done(); - }); - }); - - it('when autoSessionTracking is enabled, should set requestSession status to Crashed when an unhandled error occurs within the bounds of a request', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.1' }); - client = new NodeClient(options); - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - setCurrentClient(client); - - jest.spyOn(client, '_captureRequestSession'); - - withScope(() => { - // eslint-disable-next-line deprecation/deprecation - getIsolationScope().setRequestSession({ status: 'ok' }); - sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => { - // eslint-disable-next-line deprecation/deprecation - expect(getIsolationScope().getRequestSession()).toEqual({ status: 'crashed' }); - }); - }); - }); - - it('when autoSessionTracking is enabled, should not set requestSession status on Crash when it occurs outside the bounds of a request', done => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' }); - client = new NodeClient(options); - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - setCurrentClient(client); - - jest.spyOn(client, '_captureRequestSession'); - - let isolationScope: Scope; - sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => { - isolationScope = getIsolationScope(); - return next(); - }); - - setImmediate(() => { - // eslint-disable-next-line deprecation/deprecation - expect(isolationScope.getRequestSession()).toEqual(undefined); - done(); - }); - }); -}); diff --git a/packages/node/test/sdk/client.test.ts b/packages/node/test/sdk/client.test.ts index 5add93731c5e..8b3842a95661 100644 --- a/packages/node/test/sdk/client.test.ts +++ b/packages/node/test/sdk/client.test.ts @@ -1,20 +1,11 @@ import * as os from 'os'; import { ProxyTracer } from '@opentelemetry/api'; import * as opentelemetryInstrumentationPackage from '@opentelemetry/instrumentation'; -import { - SDK_VERSION, - SessionFlusher, - getCurrentScope, - getGlobalScope, - getIsolationScope, - setCurrentClient, - withIsolationScope, -} from '@sentry/core'; import type { Event, EventHint } from '@sentry/core'; -import type { Scope } from '@sentry/core'; +import { SDK_VERSION, getCurrentScope, getGlobalScope, getIsolationScope } from '@sentry/core'; import { setOpenTelemetryContextAsyncContextStrategy } from '@sentry/opentelemetry'; -import { NodeClient, initOpenTelemetry } from '../../src'; +import { NodeClient } from '../../src'; import { getDefaultNodeClientOptions } from '../helpers/getDefaultNodeClientOptions'; import { cleanupOtel } from '../helpers/mockSdkInit'; @@ -73,251 +64,6 @@ describe('NodeClient', () => { expect(tracer2).toBe(tracer); }); - describe('captureException', () => { - test('when autoSessionTracking is enabled, and requestHandler is not used -> requestStatus should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureException(new Error('test exception')); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - - test('when autoSessionTracking is disabled -> requestStatus should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureException(new Error('test exception')); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - - test('when autoSessionTracking is enabled + requestSession status is Crashed -> requestStatus should not be overridden', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'crashed' }); - - client.captureException(new Error('test exception')); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('crashed'); - }); - }); - - test('when autoSessionTracking is enabled + error occurs within request bounds -> requestStatus should be set to Errored', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureException(new Error('test exception')); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('errored'); - }); - }); - - test('when autoSessionTracking is enabled + error occurs outside of request bounds -> requestStatus should not be set to Errored', done => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - let isolationScope: Scope; - withIsolationScope(_isolationScope => { - // eslint-disable-next-line deprecation/deprecation - _isolationScope.setRequestSession({ status: 'ok' }); - isolationScope = _isolationScope; - }); - - client.captureException(new Error('test exception')); - - setImmediate(() => { - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession).toEqual({ status: 'ok' }); - done(); - }); - }); - }); - - describe('captureEvent()', () => { - test('If autoSessionTracking is disabled, requestSession status should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '1.4' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }); - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - - test('When captureEvent is called with an exception, requestSession status should be set to Errored', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' }); - const client = new NodeClient(options); - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('errored'); - }); - }); - - test('When captureEvent is called without an exception, requestSession status should not be set to Errored', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureEvent({ message: 'message' }); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - - test('When captureEvent is called with an exception but outside of a request, then requestStatus should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - isolationScope.clear(); - client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }); - - // eslint-disable-next-line deprecation/deprecation - expect(isolationScope.getRequestSession()).toEqual(undefined); - }); - }); - - test('When captureEvent is called with a transaction, then requestSession status should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.3' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - // It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised - // by the`requestHandler`) - client.initSessionFlusher(); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureEvent({ message: 'message', type: 'transaction' }); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - - test('When captureEvent is called with an exception but requestHandler is not used, then requestSession status should not be set', () => { - const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.3' }); - const client = new NodeClient(options); - setCurrentClient(client); - client.init(); - initOpenTelemetry(client); - - withIsolationScope(isolationScope => { - // eslint-disable-next-line deprecation/deprecation - isolationScope.setRequestSession({ status: 'ok' }); - - client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }); - - // eslint-disable-next-line deprecation/deprecation - const requestSession = isolationScope.getRequestSession(); - expect(requestSession!.status).toEqual('ok'); - }); - }); - }); - describe('_prepareEvent', () => { test('adds platform to event', () => { const options = getDefaultNodeClientOptions({}); @@ -533,28 +279,3 @@ describe('NodeClient', () => { ); }); }); - -describe('flush/close', () => { - test('client close function disables _sessionFlusher', async () => { - jest.useRealTimers(); - - const options = getDefaultNodeClientOptions({ - autoSessionTracking: true, - release: '1.1', - }); - const client = new NodeClient(options); - client.initSessionFlusher(); - // Clearing interval is important here to ensure that the flush function later on is called by the `client.close()` - // not due to the interval running every 60s - clearInterval(client['_sessionFlusher']!['_intervalId']); - - // eslint-disable-next-line deprecation/deprecation - const sessionFlusherFlushFunc = jest.spyOn(SessionFlusher.prototype, 'flush'); - - const delay = 1; - await client.close(delay); - - expect(client['_sessionFlusher']!['_isEnabled']).toBeFalsy(); - expect(sessionFlusherFlushFunc).toHaveBeenCalledTimes(1); - }); -}); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 565589bc0fbb..461870828f15 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -137,7 +137,6 @@ import type { SessionAggregates as SessionAggregates_imported, SessionContext as SessionContext_imported, SessionEnvelope as SessionEnvelope_imported, - SessionFlusherLike as SessionFlusherLike_imported, SessionItem as SessionItem_imported, SessionStatus as SessionStatus_imported, SeverityLevel as SeverityLevel_imported, @@ -429,9 +428,6 @@ export type RequestSession = RequestSession_imported; // eslint-disable-next-line deprecation/deprecation export type RequestSessionStatus = RequestSessionStatus_imported; /** @deprecated This type has been moved to `@sentry/core`. */ -// eslint-disable-next-line deprecation/deprecation -export type SessionFlusherLike = SessionFlusherLike_imported; -/** @deprecated This type has been moved to `@sentry/core`. */ export type SerializedSession = SerializedSession_imported; /** @deprecated This type has been moved to `@sentry/core`. */ export type SeverityLevel = SeverityLevel_imported; From 0d38856bdad65e3fedbc6f658b9cef2e0a8f390c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 14:29:03 +0000 Subject: [PATCH 24/38] More cleanup --- .../suites/sessions/server.ts | 9 ---- packages/core/src/scope.ts | 37 ++-------------- packages/core/src/types-hoist/scope.ts | 20 +-------- packages/core/src/types-hoist/session.ts | 13 ------ packages/core/src/utils/prepareEvent.ts | 1 - packages/core/test/lib/prepareEvent.test.ts | 3 +- packages/core/test/lib/scope.test.ts | 42 +------------------ packages/node/src/utils/prepareEvent.ts | 1 - packages/types/src/index.ts | 8 ---- 9 files changed, 7 insertions(+), 127 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/sessions/server.ts b/dev-packages/node-integration-tests/suites/sessions/server.ts index 62b154accd45..b5696c5b1522 100644 --- a/dev-packages/node-integration-tests/suites/sessions/server.ts +++ b/dev-packages/node-integration-tests/suites/sessions/server.ts @@ -1,5 +1,4 @@ import { loggingTransport } from '@sentry-internal/node-integration-tests'; -import type { SessionFlusher } from '@sentry/core'; import * as Sentry from '@sentry/node'; Sentry.init({ @@ -13,14 +12,6 @@ import express from 'express'; const app = express(); -// eslint-disable-next-line deprecation/deprecation -const flusher = (Sentry.getClient() as Sentry.NodeClient)['_sessionFlusher'] as SessionFlusher; - -// Flush after 2 seconds (to avoid waiting for the default 60s) -setTimeout(() => { - flusher?.flush(); -}, 2000); - app.get('/test/success', (_req, res) => { res.send('Success!'); }); diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 5bba8615e876..a557fcb547b5 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -13,7 +13,6 @@ import type { Extras, Primitive, PropagationContext, - RequestSession, Scope as ScopeInterface, ScopeContext, ScopeData, @@ -93,10 +92,6 @@ class ScopeClass implements ScopeInterface { /** Session */ protected _session?: Session; - /** Request Mode Session Status */ - // eslint-disable-next-line deprecation/deprecation - protected _requestSession?: RequestSession; - /** The client on this scope */ protected _client?: Client; @@ -145,7 +140,6 @@ class ScopeClass implements ScopeInterface { newScope._transactionName = this._transactionName; newScope._fingerprint = this._fingerprint; newScope._eventProcessors = [...this._eventProcessors]; - newScope._requestSession = this._requestSession; newScope._attachments = [...this._attachments]; newScope._sdkProcessingMetadata = { ...this._sdkProcessingMetadata }; newScope._propagationContext = { ...this._propagationContext }; @@ -228,23 +222,6 @@ class ScopeClass implements ScopeInterface { return this._user; } - /** - * @inheritDoc - */ - // eslint-disable-next-line deprecation/deprecation - public getRequestSession(): RequestSession | undefined { - return this._requestSession; - } - - /** - * @inheritDoc - */ - // eslint-disable-next-line deprecation/deprecation - public setRequestSession(requestSession?: RequestSession): this { - this._requestSession = requestSession; - return this; - } - /** * @inheritDoc */ @@ -359,13 +336,12 @@ class ScopeClass implements ScopeInterface { const scopeToMerge = typeof captureContext === 'function' ? captureContext(this) : captureContext; - const [scopeInstance, requestSession] = + const scopeInstance = scopeToMerge instanceof Scope - ? // eslint-disable-next-line deprecation/deprecation - [scopeToMerge.getScopeData(), scopeToMerge.getRequestSession()] + ? scopeToMerge.getScopeData() : isPlainObject(scopeToMerge) - ? [captureContext as ScopeContext, (captureContext as ScopeContext).requestSession] - : []; + ? (captureContext as ScopeContext) + : undefined; const { tags, extra, user, contexts, level, fingerprint = [], propagationContext } = scopeInstance || {}; @@ -389,10 +365,6 @@ class ScopeClass implements ScopeInterface { this._propagationContext = propagationContext; } - if (requestSession) { - this._requestSession = requestSession; - } - return this; } @@ -409,7 +381,6 @@ class ScopeClass implements ScopeInterface { this._level = undefined; this._transactionName = undefined; this._fingerprint = undefined; - this._requestSession = undefined; this._session = undefined; _setSpanForScope(this, undefined); this._attachments = []; diff --git a/packages/core/src/types-hoist/scope.ts b/packages/core/src/types-hoist/scope.ts index 57990d310820..106ebe978fd9 100644 --- a/packages/core/src/types-hoist/scope.ts +++ b/packages/core/src/types-hoist/scope.ts @@ -6,7 +6,7 @@ import type { Event, EventHint } from './event'; import type { EventProcessor } from './eventprocessor'; import type { Extra, Extras } from './extra'; import type { Primitive } from './misc'; -import type { RequestSession, Session } from './session'; +import type { Session } from './session'; import type { SeverityLevel } from './severity'; import type { Span } from './span'; import type { PropagationContext } from './tracing'; @@ -23,8 +23,6 @@ export interface ScopeContext { contexts: Contexts; tags: { [key: string]: Primitive }; fingerprint: string[]; - // eslint-disable-next-line deprecation/deprecation - requestSession: RequestSession; propagationContext: PropagationContext; } @@ -167,22 +165,6 @@ export interface Scope { */ setSession(session?: Session): this; - /** - * Returns the `RequestSession` if there is one - * - * @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`; - */ - // eslint-disable-next-line deprecation/deprecation - getRequestSession(): RequestSession | undefined; - - /** - * Sets the `RequestSession` on the scope - * - * @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`; - */ - // eslint-disable-next-line deprecation/deprecation - setRequestSession(requestSession?: RequestSession): this; - /** * Updates the scope with provided data. Can work in three variations: * - plain object containing updatable attributes diff --git a/packages/core/src/types-hoist/session.ts b/packages/core/src/types-hoist/session.ts index 4adc77240f4f..589c71f9094c 100644 --- a/packages/core/src/types-hoist/session.ts +++ b/packages/core/src/types-hoist/session.ts @@ -1,13 +1,5 @@ import type { User } from './user'; -/** - * @deprecated This type is deprecated and will be removed in the next major version of the SDK. - */ -export interface RequestSession { - // eslint-disable-next-line deprecation/deprecation - status?: RequestSessionStatus; -} - export interface Session { sid: string; did?: string | number; @@ -40,11 +32,6 @@ export type SessionContext = Partial; export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal'; -/** - * @deprecated This type is deprecated and will be removed in the next major version of the SDK. - */ -export type RequestSessionStatus = 'ok' | 'errored' | 'crashed'; - /** JSDoc */ export interface SessionAggregates { attrs?: { diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index e896a401be20..a7691b40bcfe 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -369,7 +369,6 @@ const captureContextKeys: readonly ScopeContextProperty[] = [ 'contexts', 'tags', 'fingerprint', - 'requestSession', 'propagationContext', ] as const; diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index bd0798cc0898..655b51b5c3ab 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -162,7 +162,6 @@ describe('parseEventHintOrCaptureContext', () => { contexts: { os: { name: 'linux' } }, tags: { foo: 'bar' }, fingerprint: ['xx', 'yy'], - requestSession: { status: 'ok' }, propagationContext: { traceId: 'xxx', spanId: 'yyy', @@ -175,8 +174,8 @@ describe('parseEventHintOrCaptureContext', () => { it('triggers a TS error if trying to mix ScopeContext & EventHint', () => { const actual = parseEventHintOrCaptureContext({ - // @ts-expect-error We are specifically testing that this errors! user: { id: 'xxx' }, + // @ts-expect-error We are specifically testing that this errors! mechanism: { handled: false }, }); diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 76130e779fed..6a2bab364d4e 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -6,7 +6,7 @@ import { withIsolationScope, withScope, } from '../../src'; -import type { Breadcrumb, Client, Event, RequestSessionStatus } from '../../src/types-hoist'; +import type { Breadcrumb, Client, Event } from '../../src/types-hoist'; import { Scope } from '../../src/scope'; import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; @@ -259,15 +259,6 @@ describe('Scope', () => { expect(parentScope['_extra']).toEqual(scope['_extra']); }); - test('_requestSession clone', () => { - const parentScope = new Scope(); - // eslint-disable-next-line deprecation/deprecation - parentScope.setRequestSession({ status: 'errored' }); - const scope = parentScope.clone(); - // eslint-disable-next-line deprecation/deprecation - expect(parentScope.getRequestSession()).toEqual(scope.getRequestSession()); - }); - test('parent changed inheritance', () => { const parentScope = new Scope(); const scope = parentScope.clone(); @@ -286,26 +277,6 @@ describe('Scope', () => { expect(scope['_extra']).toEqual({ a: 2 }); }); - test('child override should set the value of parent _requestSession', () => { - // Test that ensures if the status value of `status` of `_requestSession` is changed in a child scope - // that it should also change in parent scope because we are copying the reference to the object - const parentScope = new Scope(); - // eslint-disable-next-line deprecation/deprecation - parentScope.setRequestSession({ status: 'errored' }); - - const scope = parentScope.clone(); - // eslint-disable-next-line deprecation/deprecation - const requestSession = scope.getRequestSession(); - if (requestSession) { - requestSession.status = 'ok'; - } - - // eslint-disable-next-line deprecation/deprecation - expect(parentScope.getRequestSession()).toEqual({ status: 'ok' }); - // eslint-disable-next-line deprecation/deprecation - expect(scope.getRequestSession()).toEqual({ status: 'ok' }); - }); - test('should clone propagation context', () => { const parentScope = new Scope(); const scope = parentScope.clone(); @@ -322,12 +293,9 @@ describe('Scope', () => { scope.setUser({ id: '1' }); scope.setFingerprint(['abcd']); scope.addBreadcrumb({ message: 'test' }); - // eslint-disable-next-line deprecation/deprecation - scope.setRequestSession({ status: 'ok' }); expect(scope['_extra']).toEqual({ a: 2 }); scope.clear(); expect(scope['_extra']).toEqual({}); - expect(scope['_requestSession']).toEqual(undefined); expect(scope['_propagationContext']).toEqual({ traceId: expect.any(String), spanId: expect.any(String), @@ -356,8 +324,6 @@ describe('Scope', () => { scope.setUser({ id: '1337' }); scope.setLevel('info'); scope.setFingerprint(['foo']); - // eslint-disable-next-line deprecation/deprecation - scope.setRequestSession({ status: 'ok' }); }); test('given no data, returns the original scope', () => { @@ -405,7 +371,6 @@ describe('Scope', () => { localScope.setUser({ id: '42' }); localScope.setLevel('warning'); localScope.setFingerprint(['bar']); - (localScope as any)._requestSession = { status: 'ok' }; const updatedScope = scope.update(localScope) as any; @@ -427,7 +392,6 @@ describe('Scope', () => { expect(updatedScope._user).toEqual({ id: '42' }); expect(updatedScope._level).toEqual('warning'); expect(updatedScope._fingerprint).toEqual(['bar']); - expect(updatedScope._requestSession.status).toEqual('ok'); // @ts-expect-error accessing private property for test expect(updatedScope._propagationContext).toEqual(localScope._propagationContext); }); @@ -450,7 +414,6 @@ describe('Scope', () => { expect(updatedScope._user).toEqual({ id: '1337' }); expect(updatedScope._level).toEqual('info'); expect(updatedScope._fingerprint).toEqual(['foo']); - expect(updatedScope._requestSession.status).toEqual('ok'); }); test('given a plain object, it should merge two together, with the passed object having priority', () => { @@ -461,8 +424,6 @@ describe('Scope', () => { level: 'warning' as const, tags: { bar: '3', baz: '4' }, user: { id: '42' }, - // eslint-disable-next-line deprecation/deprecation - requestSession: { status: 'errored' as RequestSessionStatus }, propagationContext: { traceId: '8949daf83f4a4a70bee4c1eb9ab242ed', spanId: 'a024ad8fea82680e', @@ -490,7 +451,6 @@ describe('Scope', () => { expect(updatedScope._user).toEqual({ id: '42' }); expect(updatedScope._level).toEqual('warning'); expect(updatedScope._fingerprint).toEqual(['bar']); - expect(updatedScope._requestSession).toEqual({ status: 'errored' }); expect(updatedScope._propagationContext).toEqual({ traceId: '8949daf83f4a4a70bee4c1eb9ab242ed', spanId: 'a024ad8fea82680e', diff --git a/packages/node/src/utils/prepareEvent.ts b/packages/node/src/utils/prepareEvent.ts index b15b698550bc..11e3a9aff044 100644 --- a/packages/node/src/utils/prepareEvent.ts +++ b/packages/node/src/utils/prepareEvent.ts @@ -49,7 +49,6 @@ const captureContextKeys: readonly ScopeContextProperty[] = [ 'contexts', 'tags', 'fingerprint', - 'requestSession', 'propagationContext', ] as const; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 461870828f15..8fd49c5d7666 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -115,8 +115,6 @@ import type { ReplayRecordingMode as ReplayRecordingMode_imported, Request as Request_imported, RequestEventData as RequestEventData_imported, - RequestSession as RequestSession_imported, - RequestSessionStatus as RequestSessionStatus_imported, Runtime as Runtime_imported, SamplingContext as SamplingContext_imported, SanitizedRequestData as SanitizedRequestData_imported, @@ -422,12 +420,6 @@ export type SessionContext = SessionContext_imported; /** @deprecated This type has been moved to `@sentry/core`. */ export type SessionStatus = SessionStatus_imported; /** @deprecated This type has been moved to `@sentry/core`. */ -// eslint-disable-next-line deprecation/deprecation -export type RequestSession = RequestSession_imported; -/** @deprecated This type has been moved to `@sentry/core`. */ -// eslint-disable-next-line deprecation/deprecation -export type RequestSessionStatus = RequestSessionStatus_imported; -/** @deprecated This type has been moved to `@sentry/core`. */ export type SerializedSession = SerializedSession_imported; /** @deprecated This type has been moved to `@sentry/core`. */ export type SeverityLevel = SeverityLevel_imported; From 974b37b2d58f2f0b412a60d44c43ecc1de8e886d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 14:40:16 +0000 Subject: [PATCH 25/38] stuff --- .../suites/sessions/server.ts | 6 ++++++ .../http/SentryHttpInstrumentation.ts | 19 +++++++++++++++---- packages/node/src/integrations/http/index.ts | 10 +++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/sessions/server.ts b/dev-packages/node-integration-tests/suites/sessions/server.ts index b5696c5b1522..df2587aacfd4 100644 --- a/dev-packages/node-integration-tests/suites/sessions/server.ts +++ b/dev-packages/node-integration-tests/suites/sessions/server.ts @@ -5,6 +5,12 @@ Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', release: '1.0', transport: loggingTransport, + integrations: [ + Sentry.httpIntegration({ + // Flush after 2 seconds (to avoid waiting for the default 60s) + sessionFlushingDelayMS: 2_000, + }), + ], }); import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 506909e31575..88299128aa4a 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -59,6 +59,13 @@ type SentryHttpInstrumentationOptions = InstrumentationConfig & { * Defaults to `true`. */ trackIncomingRequestsAsSessions?: boolean; + + /** + * Number of milliseconds until sessions tracked with `trackIncomingRequestsAsSessions` will be flushed as a session aggregate. + * + * Defaults to `60000` (60s). + */ + sessionFlushingDelayMS?: number; }; // We only want to capture request bodies up to 1mb. @@ -212,6 +219,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase ({ started: timestamp, exited: value.exited, + errored: value.errored, crashed: value.crashed, }), ); @@ -222,10 +230,13 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, 60_000).unref(); + const timeout = setTimeout( + () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, + instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000, + ).unref(); } } }); diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 9af46c45804b..b63754bb017f 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -38,9 +38,15 @@ interface HttpOptions { * * Defaults to `true`. */ - // TODO(v9): Remove the note above. trackIncomingRequestsAsSessions?: boolean; + /** + * Number of milliseconds until sessions tracked with `trackIncomingRequestsAsSessions` will be flushed as a session aggregate. + * + * Defaults to `60000` (60s). + */ + sessionFlushingDelayMS?: number; + /** * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. @@ -95,11 +101,13 @@ const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: HttpOptions['breadcrumbs']; ignoreOutgoingRequests?: HttpOptions['ignoreOutgoingRequests']; trackIncomingRequestsAsSessions?: HttpOptions['trackIncomingRequestsAsSessions']; + sessionFlushingDelayMS?: HttpOptions['sessionFlushingDelayMS']; }>(`${INTEGRATION_NAME}.sentry`, options => { return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs, ignoreOutgoingRequests: options?.ignoreOutgoingRequests, trackIncomingRequestsAsSessions: options?.trackIncomingRequestsAsSessions, + sessionFlushingDelayMS: options?.sessionFlushingDelayMS, }); }); From c3b7e245b28396e40daf5f47bc3234c320427742 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 13 Dec 2024 14:51:04 +0000 Subject: [PATCH 26/38] formatting --- .../integrations/http/SentryHttpInstrumentation.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 88299128aa4a..b26aab7f31b7 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -230,13 +230,10 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, - instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000, - ).unref(); + const timeout = setTimeout(() => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000).unref(); } } }); From cde5a084e97114ad436f56f636017ee21bd758a9 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 12:24:44 +0000 Subject: [PATCH 27/38] Fix some tests --- packages/core/src/baseclient.ts | 2 +- packages/core/test/lib/prepareEvent.test.ts | 4 +- packages/node/test/sdk/init.test.ts | 46 --------------------- 3 files changed, 3 insertions(+), 49 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index f9505206c4d8..d93f03fe67dd 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -54,7 +54,7 @@ import { prepareEvent } from './utils/prepareEvent'; import { showSpanDropWarning } from './utils/spanUtils'; const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured."; -const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of miss ing or non-string release'; +const MISSING_RELEASE_FOR_SESSION_ERROR = 'Discarded session because of missing or non-string release'; /** * Base implementation for all JavaScript SDK clients. diff --git a/packages/core/test/lib/prepareEvent.test.ts b/packages/core/test/lib/prepareEvent.test.ts index 655b51b5c3ab..bfea2779b39c 100644 --- a/packages/core/test/lib/prepareEvent.test.ts +++ b/packages/core/test/lib/prepareEvent.test.ts @@ -174,9 +174,9 @@ describe('parseEventHintOrCaptureContext', () => { it('triggers a TS error if trying to mix ScopeContext & EventHint', () => { const actual = parseEventHintOrCaptureContext({ - user: { id: 'xxx' }, - // @ts-expect-error We are specifically testing that this errors! mechanism: { handled: false }, + // @ts-expect-error We are specifically testing that this errors! + user: { id: 'xxx' }, }); // ScopeContext takes presedence in this case, but this is actually not supported diff --git a/packages/node/test/sdk/init.test.ts b/packages/node/test/sdk/init.test.ts index 074de8296cd9..8827efa3e13f 100644 --- a/packages/node/test/sdk/init.test.ts +++ b/packages/node/test/sdk/init.test.ts @@ -148,52 +148,6 @@ describe('init()', () => { expect(client).toBeInstanceOf(NodeClient); }); - - describe('autoSessionTracking', () => { - it('does not track session by default if no release is set', () => { - // On CI, we always infer the release, so this does not work - if (process.env.CI) { - return; - } - init({ dsn: PUBLIC_DSN }); - - const session = getIsolationScope().getSession(); - expect(session).toBeUndefined(); - }); - - it('tracks session by default if release is set', () => { - init({ dsn: PUBLIC_DSN, release: '1.2.3' }); - - const session = getIsolationScope().getSession(); - expect(session).toBeDefined(); - }); - - it('does not track session if no release is set even if autoSessionTracking=true', () => { - // On CI, we always infer the release, so this does not work - if (process.env.CI) { - return; - } - - init({ dsn: PUBLIC_DSN, autoSessionTracking: true }); - - const session = getIsolationScope().getSession(); - expect(session).toBeUndefined(); - }); - - it('does not track session if autoSessionTracking=false', () => { - init({ dsn: PUBLIC_DSN, autoSessionTracking: false, release: '1.2.3' }); - - const session = getIsolationScope().getSession(); - expect(session).toBeUndefined(); - }); - - it('tracks session by default if autoSessionTracking=true & release is set', () => { - init({ dsn: PUBLIC_DSN, release: '1.2.3', autoSessionTracking: true }); - - const session = getIsolationScope().getSession(); - expect(session).toBeDefined(); - }); - }); }); describe('validateOpenTelemetrySetup', () => { From eb795ec96c6e8ea0b0b4b00ac722dd7822ffb7be Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 12:48:33 +0000 Subject: [PATCH 28/38] hm --- packages/node/src/sdk/index.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index b326301cbaa4..291e8704f468 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -2,9 +2,11 @@ import type { Integration, Options } from '@sentry/core'; import { consoleSandbox, dropUndefinedKeys, + endSession, functionToStringIntegration, getCurrentScope, getIntegrationsToSetup, + getIsolationScope, hasTracingEnabled, inboundFiltersIntegration, linkedErrorsIntegration, @@ -12,6 +14,7 @@ import { propagationContextFromHeaders, requestDataIntegration, stackParserFromStackParserOptions, + startSession, } from '@sentry/core'; import { enhanceDscWithOpenTelemetryRootSpanName, @@ -151,6 +154,9 @@ function _init( client.init(); logger.log(`Running in ${isCjs() ? 'CommonJS' : 'ESM'} mode.`); + + trackSessionForProcess(); + client.startClientReportTracking(); updateScopeFromEnvVariables(); @@ -301,3 +307,28 @@ function updateScopeFromEnvVariables(): void { getCurrentScope().setPropagationContext(propagationContext); } } + +/** + * Start a session for this process. + */ +// TODO(v9): This is still extremely funky because it's a session on the scope and therefore weirdly mutable by the user. +// Strawman proposal for v9: Either create a processSessionIntegration() or add functionality to the onunhandledexception/rejection integrations. +function trackSessionForProcess(): void { + startSession(); + + // Emitted in the case of healthy sessions, error of `mechanism.handled: true` and unhandledrejections because + // The 'beforeExit' event is not emitted for conditions causing explicit termination, + // such as calling process.exit() or uncaught exceptions. + // Ref: https://nodejs.org/api/process.html#process_event_beforeexit + process.on('beforeExit', () => { + const session = getIsolationScope().getSession(); + + // Only call endSession, if the Session exists on Scope and SessionStatus is not a + // Terminal Status i.e. Exited or Crashed because + // "When a session is moved away from ok it must not be updated anymore." + // Ref: https://develop.sentry.dev/sdk/sessions/ + if (session && session.status !== 'ok') { + endSession(); + } + }); +} From 31686ac7bbd7f37ecdf9a1c7fd342186ddd9222b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 12:55:57 +0000 Subject: [PATCH 29/38] desperately try to improve bundle size --- .size-limit.js | 2 +- packages/core/src/baseclient.ts | 12 ++++++------ packages/node/test/sdk/init.test.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.size-limit.js b/.size-limit.js index 6e73c9234c09..8e39e4b35a86 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -15,7 +15,7 @@ module.exports = [ path: 'packages/browser/build/npm/esm/index.js', import: createImport('init'), gzip: true, - limit: '24 KB', + limit: '24.1 KB', modifyWebpackConfig: function (config) { const webpack = require('webpack'); const TerserPlugin = require('terser-webpack-plugin'); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index d93f03fe67dd..09a090bd06c9 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -367,16 +367,16 @@ export abstract class BaseClient implements Client { * @inheritDoc */ public sendSession(session: Session | SessionAggregates): void { - const clientReleaseOption = this._options.release; - const clientEnvironmentOption = this._options.environment; + // Backfill release and environment on session + const { release: clientReleaseOption, environment: clientEnvironmentOption } = this._options; if ('aggregates' in session) { - if (!session.attrs?.release && !clientReleaseOption) { + const sessionAttrs = session.attrs || {}; + if (!sessionAttrs.release && !clientReleaseOption) { DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); return; } - session.attrs = session.attrs || {}; - session.attrs.release = session.attrs.release || clientReleaseOption; - session.attrs.environment = session.attrs.environment || clientEnvironmentOption; + sessionAttrs.release = sessionAttrs.release || clientReleaseOption; + sessionAttrs.environment = sessionAttrs.environment || clientEnvironmentOption; } else { if (!session.release && !clientReleaseOption) { DEBUG_BUILD && logger.warn(MISSING_RELEASE_FOR_SESSION_ERROR); diff --git a/packages/node/test/sdk/init.test.ts b/packages/node/test/sdk/init.test.ts index 8827efa3e13f..55536a972d77 100644 --- a/packages/node/test/sdk/init.test.ts +++ b/packages/node/test/sdk/init.test.ts @@ -2,7 +2,7 @@ import { logger } from '@sentry/core'; import type { Integration } from '@sentry/core'; import * as SentryOpentelemetry from '@sentry/opentelemetry'; -import { getClient, getIsolationScope } from '../../src/'; +import { getClient } from '../../src/'; import * as auto from '../../src/integrations/tracing'; import { init, validateOpenTelemetrySetup } from '../../src/sdk'; import { NodeClient } from '../../src/sdk/client'; From 2d084e8a47ab6ec0b2bc5794222be7062c12ff32 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 13:37:49 +0000 Subject: [PATCH 30/38] bundle? --- packages/core/src/baseclient.ts | 3 ++- packages/core/src/exports.ts | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 09a090bd06c9..a957bed317ed 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -32,6 +32,7 @@ import type { } from './types-hoist'; import { getEnvelopeEndpointWithUrlEncodedAuth } from './api'; +import { DEFAULT_ENVIRONMENT } from './constants'; import { getCurrentScope, getIsolationScope, getTraceContextFromScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import { createEventEnvelope, createSessionEnvelope } from './envelope'; @@ -368,7 +369,7 @@ export abstract class BaseClient implements Client { */ public sendSession(session: Session | SessionAggregates): void { // Backfill release and environment on session - const { release: clientReleaseOption, environment: clientEnvironmentOption } = this._options; + const { release: clientReleaseOption, environment: clientEnvironmentOption = DEFAULT_ENVIRONMENT } = this._options; if ('aggregates' in session) { const sessionAttrs = session.attrs || {}; if (!sessionAttrs.release && !clientReleaseOption) { diff --git a/packages/core/src/exports.ts b/packages/core/src/exports.ts index cf7e872fb001..2caf38b17631 100644 --- a/packages/core/src/exports.ts +++ b/packages/core/src/exports.ts @@ -15,7 +15,6 @@ import type { User, } from './types-hoist'; -import { DEFAULT_ENVIRONMENT } from './constants'; import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } from './currentScopes'; import { DEBUG_BUILD } from './debug-build'; import { closeSession, makeSession, updateSession } from './session'; @@ -265,18 +264,13 @@ export function addEventProcessor(callback: EventProcessor): void { * @returns the new active session */ export function startSession(context?: SessionContext): Session { - const client = getClient(); const isolationScope = getIsolationScope(); const currentScope = getCurrentScope(); - const { release, environment = DEFAULT_ENVIRONMENT } = (client && client.getOptions()) || {}; - // Will fetch userAgent if called from browser sdk const { userAgent } = GLOBAL_OBJ.navigator || {}; const session = makeSession({ - release, - environment, user: currentScope.getUser() || isolationScope.getUser(), ...(userAgent && { userAgent }), ...context, From 084781a602e46e7edc9a5528126f40db0fc4f72d Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 13:46:03 +0000 Subject: [PATCH 31/38] fun --- packages/core/src/server-runtime-client.ts | 8 ++------ packages/core/src/types-hoist/scope.ts | 14 ++++++++++---- .../http/SentryHttpInstrumentation.ts | 19 ++++++++----------- 3 files changed, 20 insertions(+), 21 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 1037b88f7db9..20e968b38839 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -206,16 +206,12 @@ export class ServerRuntimeClient< } function setCurrentRequestSessionErroredOrCrashed(eventHint?: EventHint): void { - const isHandledException = eventHint?.mechanism?.handled ?? true; - - const requestSession = getIsolationScope().getScopeData().sdkProcessingMetadata.requestSession as - | { status: 'ok' | 'errored' | 'crashed' } - | undefined; - + const requestSession = getIsolationScope().getScopeData().sdkProcessingMetadata.requestSession; if (requestSession) { // We mutate instead of doing `setSdkProcessingMetadata` because the http integration stores away a particular // isolationScope. If that isolation scope is forked, setting the processing metadata here will not mutate the // original isolation scope that the http integration stored away. + const isHandledException = eventHint?.mechanism?.handled ?? true; if (isHandledException) { // A request session can go from "errored" -> "crashed" but not "crashed" -> "errored". // Crashed (unhandled exception) is worse than errored (handled exception). diff --git a/packages/core/src/types-hoist/scope.ts b/packages/core/src/types-hoist/scope.ts index 106ebe978fd9..311532b1b70c 100644 --- a/packages/core/src/types-hoist/scope.ts +++ b/packages/core/src/types-hoist/scope.ts @@ -26,6 +26,14 @@ export interface ScopeContext { propagationContext: PropagationContext; } +// TODO(v9): Add `normalizedRequest` +export interface SdkProcessingMetadata { + [key: string]: unknown; + requestSession?: { + status: 'ok' | 'errored' | 'crashed'; + }; +} + export interface ScopeData { eventProcessors: EventProcessor[]; breadcrumbs: Breadcrumb[]; @@ -35,7 +43,7 @@ export interface ScopeData { contexts: Contexts; attachments: Attachment[]; propagationContext: PropagationContext; - sdkProcessingMetadata: { [key: string]: unknown }; + sdkProcessingMetadata: SdkProcessingMetadata; fingerprint: string[]; level?: SeverityLevel; transactionName?: string; @@ -207,10 +215,8 @@ export interface Scope { /** * Add data which will be accessible during event processing but won't get sent to Sentry. - * - * TODO(v9): We should type this stricter, so that e.g. `normalizedRequest` is strictly typed. */ - setSDKProcessingMetadata(newData: { [key: string]: unknown }): this; + setSDKProcessingMetadata(newData: SdkProcessingMetadata): this; /** * Add propagation context to the scope, used for distributed tracing diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index b26aab7f31b7..bfb85ad671ae 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -27,10 +27,6 @@ const clientToAggregatesMap = new Map< { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } >(); -interface RequestSession { - status: 'ok' | 'errored' | 'crashed'; -} - type Http = typeof http; type Https = typeof https; @@ -188,9 +184,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase { // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. const client = getClient(); - const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession as - | RequestSession - | undefined; + const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; if (client && requestSession) { DEBUG_BUILD && logger.debug(`Recorded request session with status: ${requestSession.status}`); @@ -230,10 +224,13 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000).unref(); + const timeout = setTimeout( + () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, + instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000, + ).unref(); } } }); From cae7fc80cb3f93bc708cf886c0eef99b12ff3b13 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 15:21:25 +0000 Subject: [PATCH 32/38] Add the most beautiful tests I have ever produced --- .../http/SentryHttpInstrumentation.ts | 144 ++++++++++------- .../request-session-tracking.test.ts | 151 ++++++++++++++++++ 2 files changed, 233 insertions(+), 62 deletions(-) create mode 100644 packages/node/test/integrations/request-session-tracking.test.ts diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index bfb85ad671ae..f864749a1539 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -2,6 +2,7 @@ import type * as http from 'node:http'; import type { IncomingMessage, RequestOptions } from 'node:http'; import type * as https from 'node:https'; +import { EventEmitter } from 'node:stream'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; @@ -22,11 +23,6 @@ import { DEBUG_BUILD } from '../../debug-build'; import { getRequestUrl } from '../../utils/getRequestUrl'; import { getRequestInfo } from './vendor/getRequestInfo'; -const clientToAggregatesMap = new Map< - Client, - { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } ->(); - type Http = typeof http; type Https = typeof https; @@ -50,7 +46,8 @@ type SentryHttpInstrumentationOptions = InstrumentationConfig & { ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; /** - * TODO + * Whether the integration should create [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for incoming requests to track the health and crash-free rate of your releases in Sentry. + * Read more about Release Health: https://docs.sentry.io/product/releases/health/ * * Defaults to `true`. */ @@ -178,62 +175,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase { - // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. - const client = getClient(); - const requestSession = isolationScope.getScopeData().sdkProcessingMetadata.requestSession; - - if (client && requestSession) { - DEBUG_BUILD && logger.debug(`Recorded request session with status: ${requestSession.status}`); - - const roundedDate = new Date(); - roundedDate.setSeconds(0, 0); - const dateBucketKey = roundedDate.toISOString(); - - const existingClientAggregate = clientToAggregatesMap.get(client); - const bucket = existingClientAggregate?.[dateBucketKey] || { exited: 0, crashed: 0, errored: 0 }; - bucket[({ ok: 'exited', crashed: 'crashed', errored: 'errored' } as const)[requestSession.status]]++; - - if (existingClientAggregate) { - existingClientAggregate[dateBucketKey] = bucket; - } else { - DEBUG_BUILD && logger.debug('Opened new request session aggregate.'); - const newClientAggregate = { [dateBucketKey]: bucket }; - clientToAggregatesMap.set(client, newClientAggregate); - - const flushPendingClientAggregates = (): void => { - clearTimeout(timeout); - unregisterClientFlushHook(); - clientToAggregatesMap.delete(client); - - const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( - ([timestamp, value]) => ({ - started: timestamp, - exited: value.exited, - errored: value.errored, - crashed: value.crashed, - }), - ); - client.sendSession({ aggregates: aggregatePayload }); - }; - - const unregisterClientFlushHook = client.on('flush', () => { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); - flushPendingClientAggregates(); - }); - const timeout = setTimeout( - () => { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, - instrumentation.getConfig().sessionFlushingDelayMS ?? 60_000, - ).unref(); - } - } - }); + recordRequestSession({ requestIsolationScope: isolationScope, response }); } return withIsolationScope(isolationScope, () => { @@ -505,3 +447,81 @@ function patchRequestToCaptureBody(req: IncomingMessage, isolationScope: Scope): // ignore errors if we can't patch stuff } } + +/** + * Starts a session and tracks it in the context of a given isolation scope. + * When the passed response is finished, the session is put into a task and is + * aggregated with other sessions that may happen in a certain time window + * (sessionFlushingDelayMs). + * + * The sessions are always aggregated by the client that is on the current scope + * at the time of ending the response (if there is one). + */ +// Exported for unit tests +export function recordRequestSession({ + requestIsolationScope, + response, + sessionFlushingDelayMS, +}: { requestIsolationScope: Scope; response: EventEmitter; sessionFlushingDelayMS?: number }): void { + requestIsolationScope.setSDKProcessingMetadata({ + requestSession: { status: 'ok' }, + }); + response.once('close', () => { + // We need to grab the client off the current scope instead of the isolation scope because the isolation scope doesn't hold any client out of the box. + const client = getClient(); + const requestSession = requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession; + + if (client && requestSession) { + DEBUG_BUILD && logger.debug(`Recorded request session with status: ${requestSession.status}`); + + const roundedDate = new Date(); + roundedDate.setSeconds(0, 0); + const dateBucketKey = roundedDate.toISOString(); + + const existingClientAggregate = clientToRequestSessionAggregatesMap.get(client); + const bucket = existingClientAggregate?.[dateBucketKey] || { exited: 0, crashed: 0, errored: 0 }; + bucket[({ ok: 'exited', crashed: 'crashed', errored: 'errored' } as const)[requestSession.status]]++; + + if (existingClientAggregate) { + existingClientAggregate[dateBucketKey] = bucket; + } else { + DEBUG_BUILD && logger.debug('Opened new request session aggregate.'); + const newClientAggregate = { [dateBucketKey]: bucket }; + clientToRequestSessionAggregatesMap.set(client, newClientAggregate); + + const flushPendingClientAggregates = (): void => { + clearTimeout(timeout); + unregisterClientFlushHook(); + clientToRequestSessionAggregatesMap.delete(client); + + const aggregatePayload: AggregationCounts[] = Object.entries(newClientAggregate).map( + ([timestamp, value]) => ({ + started: timestamp, + exited: value.exited, + errored: value.errored, + crashed: value.crashed, + }), + ); + client.sendSession({ aggregates: aggregatePayload }); + }; + + const unregisterClientFlushHook = client.on('flush', () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); + flushPendingClientAggregates(); + }); + const timeout = setTimeout( + () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, + sessionFlushingDelayMS ?? 60_000, + ).unref(); + } + } + }); +} + +const clientToRequestSessionAggregatesMap = new Map< + Client, + { [timestampRoundedToSeconds: string]: { exited: number; crashed: number; errored: number } } +>(); diff --git a/packages/node/test/integrations/request-session-tracking.test.ts b/packages/node/test/integrations/request-session-tracking.test.ts new file mode 100644 index 000000000000..3e83d2369ad3 --- /dev/null +++ b/packages/node/test/integrations/request-session-tracking.test.ts @@ -0,0 +1,151 @@ +import { EventEmitter } from 'stream'; +import { Client, Scope, ServerRuntimeClient, createTransport, withScope } from '@sentry/core'; +import { recordRequestSession } from '../../src/integrations/http/SentryHttpInstrumentation'; + +jest.useFakeTimers(); + +describe('recordRequestSession()', () => { + it('should send an "exited" session for an ok ended request', () => { + const client = createTestClient(); + const sendSessionSpy = jest.spyOn(client, 'sendSession'); + + jest.setSystemTime(new Date('March 19, 1999 06:12:34')); + + simulateRequest(client, 'ok'); + + jest.runAllTimers(); + + expect(sendSessionSpy).toBeCalledWith({ + aggregates: [{ crashed: 0, errored: 0, exited: 1, started: '1999-03-19T06:12:00.000Z' }], + }); + }); + + it('should send an "crashed" session when the session on the requestProcessingMetadata was overridden with crashed', () => { + const client = createTestClient(); + const sendSessionSpy = jest.spyOn(client, 'sendSession'); + + jest.setSystemTime(new Date('March 19, 1999 06:12:34')); + + simulateRequest(client, 'crashed'); + + jest.runAllTimers(); + + expect(sendSessionSpy).toBeCalledWith({ + aggregates: [{ crashed: 1, errored: 0, exited: 0, started: expect.stringMatching(/....-..-..T..:..:00.000Z/) }], + }); + }); + + it('should send an "errored" session when the session on the requestProcessingMetadata was overridden with errored', () => { + const client = createTestClient(); + const sendSessionSpy = jest.spyOn(client, 'sendSession'); + + jest.setSystemTime(new Date('March 19, 1999 06:12:34')); + + simulateRequest(client, 'errored'); + + jest.runAllTimers(); + + expect(sendSessionSpy).toBeCalledWith({ + aggregates: [{ crashed: 0, errored: 1, exited: 0, started: expect.stringMatching(/....-..-..T..:..:00.000Z/) }], + }); + }); + + it('should aggregate request sessions within a time frame', async () => { + const client = createTestClient(); + + const sendSessionSpy = jest.spyOn(client, 'sendSession'); + + jest.setSystemTime(new Date('March 19, 1999 06:00:00')); + + simulateRequest(client, 'ok'); + simulateRequest(client, 'ok'); + simulateRequest(client, 'crashed'); + simulateRequest(client, 'errored'); + + // "Wait" 1+ second to get into new bucket + jest.setSystemTime(new Date('March 19, 1999 06:01:01')); + + simulateRequest(client, 'ok'); + simulateRequest(client, 'errored'); + + jest.runAllTimers(); + + expect(sendSessionSpy).toBeCalledWith({ + aggregates: [ + { + crashed: 1, + errored: 1, + exited: 2, + started: '1999-03-19T06:00:00.000Z', + }, + { crashed: 0, errored: 1, exited: 1, started: '1999-03-19T06:01:00.000Z' }, + ], + }); + }); + + it('should flush pending sessions when the client emits a "flush" hook', async () => { + const client = createTestClient(); + + const sendSessionSpy = jest.spyOn(client, 'sendSession'); + + jest.setSystemTime(new Date('March 19, 1999 06:00:00')); + + simulateRequest(client, 'ok'); + + // "Wait" 1+ second to get into new bucket + jest.setSystemTime(new Date('March 19, 1999 06:01:01')); + + simulateRequest(client, 'ok'); + + client.emit('flush'); + + expect(sendSessionSpy).toBeCalledWith({ + aggregates: [ + { + crashed: 0, + errored: 0, + exited: 1, + started: '1999-03-19T06:00:00.000Z', + }, + { + crashed: 0, + errored: 0, + exited: 1, + started: '1999-03-19T06:01:00.000Z', + }, + ], + }); + }); +}); + +function simulateRequest(client: Client, status: 'ok' | 'errored' | 'crashed') { + const requestIsolationScope = new Scope(); + const response = new EventEmitter(); + + recordRequestSession({ + requestIsolationScope, + response, + }); + + requestIsolationScope.getScopeData().sdkProcessingMetadata.requestSession!.status = status; + + withScope(scope => { + scope.setClient(client); + // "end" request + response.emit('close'); + }); +} + +function createTestClient() { + return new ServerRuntimeClient({ + integrations: [], + transport: () => + createTransport( + { + recordDroppedEvent: () => undefined, + }, + () => Promise.resolve({}), + ), + stackParser: () => [], + }); +} From ad59b24b9e4c6371dba3ab901c9a9fbdac6a93bb Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 15:30:02 +0000 Subject: [PATCH 33/38] fix --- .../integrations/http/SentryHttpInstrumentation.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index f864749a1539..a56219d89bc6 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -509,13 +509,10 @@ export function recordRequestSession({ DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); flushPendingClientAggregates(); }); - const timeout = setTimeout( - () => { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, - sessionFlushingDelayMS ?? 60_000, - ).unref(); + const timeout = setTimeout(() => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, sessionFlushingDelayMS ?? 60_000).unref(); } } }); From 0c7f97ced96591c5877074589aaaca95016d5786 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 16:33:07 +0000 Subject: [PATCH 34/38] lint --- .../integrations/http/SentryHttpInstrumentation.ts | 13 ++++++++----- .../integrations/request-session-tracking.test.ts | 3 ++- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index a56219d89bc6..ffe80b819ef8 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -2,7 +2,7 @@ import type * as http from 'node:http'; import type { IncomingMessage, RequestOptions } from 'node:http'; import type * as https from 'node:https'; -import { EventEmitter } from 'node:stream'; +import type { EventEmitter } from 'node:stream'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; @@ -509,10 +509,13 @@ export function recordRequestSession({ DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); flushPendingClientAggregates(); }); - const timeout = setTimeout(() => { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, sessionFlushingDelayMS ?? 60_000).unref(); + const timeout = setTimeout( + () => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, + sessionFlushingDelayMS ?? 60_000, + ).unref(); } } }); diff --git a/packages/node/test/integrations/request-session-tracking.test.ts b/packages/node/test/integrations/request-session-tracking.test.ts index 3e83d2369ad3..27f9ffd336a0 100644 --- a/packages/node/test/integrations/request-session-tracking.test.ts +++ b/packages/node/test/integrations/request-session-tracking.test.ts @@ -1,5 +1,6 @@ import { EventEmitter } from 'stream'; -import { Client, Scope, ServerRuntimeClient, createTransport, withScope } from '@sentry/core'; +import { Scope, ServerRuntimeClient, createTransport, withScope } from '@sentry/core'; +import type { Client } from '@sentry/core'; import { recordRequestSession } from '../../src/integrations/http/SentryHttpInstrumentation'; jest.useFakeTimers(); From 845e45445d54ebe59ff4ee1388e0aa8be1ab470c Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 16:37:45 +0000 Subject: [PATCH 35/38] fix --- .../integrations/http/SentryHttpInstrumentation.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index ffe80b819ef8..67525ff5da85 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -509,13 +509,10 @@ export function recordRequestSession({ DEBUG_BUILD && logger.debug('Sending request session aggregate due to client flush'); flushPendingClientAggregates(); }); - const timeout = setTimeout( - () => { - DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); - flushPendingClientAggregates(); - }, - sessionFlushingDelayMS ?? 60_000, - ).unref(); + const timeout = setTimeout(() => { + DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); + flushPendingClientAggregates(); + }, sessionFlushingDelayMS ?? 60_000).unref(); } } }); From d93c35205faaa7c4e8c70b045c828f4ce6c03536 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 16 Dec 2024 16:46:47 +0000 Subject: [PATCH 36/38] facepalm --- .../src/integrations/http/SentryHttpInstrumentation.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 67525ff5da85..7e16856ff023 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -175,7 +175,11 @@ export class SentryHttpInstrumentation extends InstrumentationBase { @@ -512,7 +516,7 @@ export function recordRequestSession({ const timeout = setTimeout(() => { DEBUG_BUILD && logger.debug('Sending request session aggregate due to flushing schedule'); flushPendingClientAggregates(); - }, sessionFlushingDelayMS ?? 60_000).unref(); + }, sessionFlushingDelayMS).unref(); } } }); From 9ab36270a45a787f2fd65703fe4a650aa2471302 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 19 Dec 2024 08:35:04 +0000 Subject: [PATCH 37/38] Feedback --- packages/core/src/server-runtime-client.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index 20e968b38839..479682d06998 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -212,13 +212,11 @@ function setCurrentRequestSessionErroredOrCrashed(eventHint?: EventHint): void { // isolationScope. If that isolation scope is forked, setting the processing metadata here will not mutate the // original isolation scope that the http integration stored away. const isHandledException = eventHint?.mechanism?.handled ?? true; - if (isHandledException) { - // A request session can go from "errored" -> "crashed" but not "crashed" -> "errored". - // Crashed (unhandled exception) is worse than errored (handled exception). - if (requestSession.status !== 'crashed') { - requestSession.status = 'errored'; - } - } else { + // A request session can go from "errored" -> "crashed" but not "crashed" -> "errored". + // Crashed (unhandled exception) is worse than errored (handled exception). + if (isHandledException && requestSession.status !== 'crashed') { + requestSession.status = 'errored'; + } else if (!isHandledException) { requestSession.status = 'crashed'; } } From 120c34eb684966a945e285a965aa8188c8c04c9b Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 19 Dec 2024 09:07:36 +0000 Subject: [PATCH 38/38] messed up merge --- packages/core/src/scope.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 885fb34faf08..3f66d2053b34 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -272,9 +272,8 @@ export class Scope { } /** - * Get the request session from this scope. - * - * @deprecated Use `getSession()` and `setSession()` instead of `getRequestSession()` and `setRequestSession()`; + * Set an object that will be merged into existing tags on the scope, + * and will be sent as tags data with the event. */ public setTags(tags: { [key: string]: Primitive }): this { this._tags = {