feat(core): Streamline integration function results to be compatible (#10135)

mydea · c298lee · commit 15c00acc9e05 · 2024-01-10T14:12:04.000-05:00
For the v7 cycle, in order to make this easier to handle. in v8, we need
to remove the unused `setupOnce()` calls again.

It would be nicer if we could do away with this, but it is pretty hard
as the options for a client currently are types as `Integration[]`, and
we use these both for setting &amp; getting, making it hard to change this
without breaking anything. Now, this is much easier (it basically just
works), with the small downsides of shipping a few more bytes - but we
can simply remove this in v8 then.
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -10,6 +10,15 @@ npx @sentry/migr8@latest
 This will let you select which updates to run, and automatically update your code. Make sure to still review all code
 changes!
 
+## Changed integration interface
+
+In v8, integrations passed to a client will have an optional `setupOnce()` hook.
+Currently, this hook is always present, but in v8 you will not be able to rely on this always existing anymore -
+any integration _may_ have a `setup` and/or a `setupOnce` hook. Additionally, `setupOnce()` will not receive any arguments anymore.
+
+This should not affect most people, but in the case that you are manually calling `integration.setupOnce()` right now,
+make sure to guard it's existence properly.
+
 ## Deprecate `Hub`
 
 The `Hub` has been a very important part of the Sentry SDK API up until now. Hubs were the SDK's "unit of concurrency"
diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts
@@ -68,6 +68,8 @@ const breadcrumbsIntegration: IntegrationFn = (options: Partial<BreadcrumbsOptio
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (_options.console) {
         addConsoleInstrumentationHandler(_getConsoleBreadcrumbHandler(client));
diff --git a/packages/browser/src/integrations/dedupe.ts b/packages/browser/src/integrations/dedupe.ts
@@ -11,6 +11,8 @@ const dedupeIntegration: IntegrationFn = () => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(currentEvent) {
       // We want to ignore any non-error type events, e.g. transactions or replays
       // These should never be deduped, and also not be compared against as _previousEvent.
diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts
@@ -8,6 +8,8 @@ const INTEGRATION_NAME = 'HttpContext';
 const httpContextIntegration: IntegrationFn = () => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     preprocessEvent(event) {
       // if none of the information we want exists, don't bother
       if (!WINDOW.navigator && !WINDOW.location && !WINDOW.document) {
diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts
@@ -19,6 +19,8 @@ const linkedErrorsIntegration: IntegrationFn = (options: LinkedErrorsOptions = {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     preprocessEvent(event, hint, client) {
       const options = client.getOptions();
 
diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts
@@ -21,6 +21,8 @@ const INTEGRATION_NAME = 'BrowserProfiling';
 const browserProfilingIntegration: IntegrationFn = () => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       const scope = getCurrentScope();
 
diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts
@@ -1,4 +1,4 @@
-import type { Client, Event, EventHint, EventProcessor, Hub, Integration, IntegrationFn, Options } from '@sentry/types';
+import type { Client, Event, EventHint, Integration, IntegrationFn, Options } from '@sentry/types';
 import { arrayify, logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from './debug-build';
@@ -171,26 +171,16 @@ export function convertIntegrationFnToClass<Fn extends IntegrationFn>(
   fn: Fn,
 ): Integration & {
   id: string;
-  new (...args: Parameters<Fn>): Integration &
-    ReturnType<Fn> & {
-      setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
-    };
+  new (...args: Parameters<Fn>): Integration & ReturnType<Fn>;
 } {
   return Object.assign(
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     function ConvertedIntegration(...rest: Parameters<Fn>) {
-      return {
-        // eslint-disable-next-line @typescript-eslint/no-empty-function
-        setupOnce: () => {},
-        ...fn(...rest),
-      };
+      return fn(...rest);
     },
     { id: name },
   ) as unknown as Integration & {
     id: string;
-    new (...args: Parameters<Fn>): Integration &
-      ReturnType<Fn> & {
-        setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
-      };
+    new (...args: Parameters<Fn>): Integration & ReturnType<Fn>;
   };
 }
diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts
@@ -33,6 +33,8 @@ const INTEGRATION_NAME = 'InboundFilters';
 const inboundFiltersIntegration: IntegrationFn = (options: Partial<InboundFiltersOptions>) => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event, _hint, client) {
       const clientOptions = client.getOptions();
       const mergedOptions = _mergeOptions(options, clientOptions);
diff --git a/packages/core/src/integrations/linkederrors.ts b/packages/core/src/integrations/linkederrors.ts
@@ -18,6 +18,8 @@ const linkedErrorsIntegration: IntegrationFn = (options: LinkedErrorsOptions = {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     preprocessEvent(event, hint, client) {
       const options = client.getOptions();
 
diff --git a/packages/core/src/integrations/metadata.ts b/packages/core/src/integrations/metadata.ts
@@ -9,6 +9,8 @@ const INTEGRATION_NAME = 'ModuleMetadata';
 const moduleMetadataIntegration: IntegrationFn = () => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (typeof client.on !== 'function') {
         return;
diff --git a/packages/core/src/integrations/requestdata.ts b/packages/core/src/integrations/requestdata.ts
@@ -71,7 +71,8 @@ const requestDataIntegration: IntegrationFn = (options: RequestDataIntegrationOp
 
   return {
     name: INTEGRATION_NAME,
-
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event, _hint, client) {
       // Note: In the long run, most of the logic here should probably move into the request data utility functions. For
       // the moment it lives here, though, until https://github.com/getsentry/sentry-javascript/issues/5718 is addressed.
diff --git a/packages/core/src/metrics/integration.ts b/packages/core/src/metrics/integration.ts
@@ -8,6 +8,8 @@ const INTEGRATION_NAME = 'MetricsAggregator';
 const metricsAggregatorIntegration: IntegrationFn = () => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client: BaseClient<ClientOptions>) {
       client.metricsAggregator = new BrowserMetricsAggregator(client);
     },
diff --git a/packages/core/test/lib/integration.test.ts b/packages/core/test/lib/integration.test.ts
@@ -649,7 +649,10 @@ describe('addIntegration', () => {
 describe('convertIntegrationFnToClass', () => {
   /* eslint-disable deprecation/deprecation */
   it('works with a minimal integration', () => {
-    const integrationFn = () => ({ name: 'testName' });
+    const integrationFn = () => ({
+      name: 'testName',
+      setupOnce: () => {},
+    });
 
     const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);
 
@@ -663,7 +666,10 @@ describe('convertIntegrationFnToClass', () => {
   });
 
   it('works with options', () => {
-    const integrationFn = (_options: { num: number }) => ({ name: 'testName' });
+    const integrationFn = (_options: { num: number }) => ({
+      name: 'testName',
+      setupOnce: () => {},
+    });
 
     const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);
 
diff --git a/packages/deno/src/integrations/context.ts b/packages/deno/src/integrations/context.ts
@@ -55,6 +55,8 @@ async function addDenoRuntimeContext(event: Event): Promise<Event> {
 const denoContextIntegration: IntegrationFn = () => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       return addDenoRuntimeContext(event);
     },
diff --git a/packages/deno/src/integrations/contextlines.ts b/packages/deno/src/integrations/contextlines.ts
@@ -52,6 +52,8 @@ const denoContextLinesIntegration: IntegrationFn = (options: ContextLinesOptions
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       return addSourceContext(event, contextLines);
     },
diff --git a/packages/deno/src/integrations/globalhandlers.ts b/packages/deno/src/integrations/globalhandlers.ts
@@ -22,6 +22,8 @@ const globalHandlersIntegration: IntegrationFn = (options?: GlobalHandlersIntegr
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (_options.error) {
         installGlobalErrorHandler(client);
diff --git a/packages/deno/src/integrations/normalizepaths.ts b/packages/deno/src/integrations/normalizepaths.ts
@@ -69,6 +69,8 @@ const normalizePathsIntegration: IntegrationFn = () => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       // This error.stack hopefully contains paths that traverse the app cwd
       const error = new Error();
diff --git a/packages/integrations/src/captureconsole.ts b/packages/integrations/src/captureconsole.ts
@@ -20,6 +20,8 @@ const captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (!('console' in GLOBAL_OBJ)) {
         return;
diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts
@@ -23,6 +23,8 @@ const contextLinesIntegration: IntegrationFn = (options: ContextLinesOptions = {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       return addSourceContext(event, contextLines);
     },
diff --git a/packages/integrations/src/debug.ts b/packages/integrations/src/debug.ts
@@ -20,6 +20,8 @@ const debugIntegration = ((options: DebugOptions = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (!client.on) {
         return;
diff --git a/packages/integrations/src/dedupe.ts b/packages/integrations/src/dedupe.ts
@@ -11,6 +11,8 @@ const dedupeIntegration = (() => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(currentEvent) {
       // We want to ignore any non-error type events, e.g. transactions or replays
       // These should never be deduped, and also not be compared against as _previousEvent.
diff --git a/packages/integrations/src/extraerrordata.ts b/packages/integrations/src/extraerrordata.ts
@@ -28,6 +28,8 @@ const extraErrorDataIntegration = ((options: Partial<ExtraErrorDataOptions> = {}
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event, hint) {
       return _enhanceEventWithErrorData(event, hint, depth, captureErrorCause);
     },
diff --git a/packages/integrations/src/httpclient.ts b/packages/integrations/src/httpclient.ts
@@ -47,6 +47,8 @@ const httpClientIntegration = ((options: Partial<HttpClientOptions> = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client): void {
       _wrapFetch(client, _options);
       _wrapXHR(client, _options);
diff --git a/packages/integrations/src/rewriteframes.ts b/packages/integrations/src/rewriteframes.ts
@@ -69,6 +69,8 @@ const rewriteFramesIntegration = ((options: RewriteFramesOptions = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(originalEvent) {
       let processedEvent = originalEvent;
 
diff --git a/packages/integrations/src/sessiontiming.ts b/packages/integrations/src/sessiontiming.ts
@@ -8,6 +8,8 @@ const sessionTimingIntegration = (() => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       const now = Date.now();
 
diff --git a/packages/integrations/src/transaction.ts b/packages/integrations/src/transaction.ts
@@ -6,6 +6,8 @@ const INTEGRATION_NAME = 'Transaction';
 const transactionIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       const frames = _getFramesFromEvent(event);
 
diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts
@@ -55,6 +55,8 @@ const INTEGRATION_NAME = 'Anr';
 const anrIntegration = ((options: Partial<Options> = {}) => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client: NodeClient) {
       if (NODE_VERSION.major < 16 || (NODE_VERSION.major === 16 && NODE_VERSION.minor < 17)) {
         throw new Error('ANR detection requires Node 16.17.0 or later');
diff --git a/packages/node/src/integrations/console.ts b/packages/node/src/integrations/console.ts
@@ -8,6 +8,8 @@ const INTEGRATION_NAME = 'Console';
 const consoleIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       addConsoleInstrumentationHandler(({ args, level }) => {
         if (getClient() !== client) {
diff --git a/packages/node/src/integrations/context.ts b/packages/node/src/integrations/context.ts
@@ -98,6 +98,8 @@ const contextIntegration = ((options: ContextOptions = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       return addContext(event);
     },
diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
@@ -40,6 +40,8 @@ const contextLinesIntegration = ((options: ContextLinesOptions = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       return addSourceContext(event, contextLines);
     },
diff --git a/packages/node/src/integrations/local-variables/local-variables-async.ts b/packages/node/src/integrations/local-variables/local-variables-async.ts
@@ -214,6 +214,8 @@ export const localVariablesAsync: IntegrationFn = (options: Options = {}) => {
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client: NodeClient) {
       const clientOptions = client.getOptions();
 
diff --git a/packages/node/src/integrations/local-variables/local-variables-sync.ts b/packages/node/src/integrations/local-variables/local-variables-sync.ts
@@ -326,6 +326,8 @@ export const localVariablesSync: IntegrationFn = (
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client: NodeClient) {
       const clientOptions = client.getOptions();
 
diff --git a/packages/node/src/integrations/modules.ts b/packages/node/src/integrations/modules.ts
@@ -79,6 +79,8 @@ function _getModules(): { [key: string]: string } {
 const modulesIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     processEvent(event) {
       event.modules = {
         ...event.modules,
diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts
@@ -48,6 +48,8 @@ const onUncaughtExceptionIntegration = ((options: Partial<OnUncaughtExceptionOpt
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client: NodeClient) {
       global.process.on('uncaughtException', makeErrorHandler(client, _options));
     },
diff --git a/packages/node/src/integrations/onunhandledrejection.ts b/packages/node/src/integrations/onunhandledrejection.ts
@@ -21,6 +21,8 @@ const onUnhandledRejectionIntegration = ((options: Partial<OnUnhandledRejectionO
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       global.process.on('unhandledRejection', makeUnhandledPromiseHandler(client, { mode }));
     },
diff --git a/packages/node/src/integrations/spotlight.ts b/packages/node/src/integrations/spotlight.ts
@@ -21,6 +21,8 @@ const spotlightIntegration = ((options: Partial<SpotlightConnectionOptions> = {}
 
   return {
     name: INTEGRATION_NAME,
+    // TODO v8: Remove this
+    setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
     setup(client) {
       if (typeof process === 'object' && process.env && process.env.NODE_ENV !== 'development') {
         logger.warn("[Spotlight] It seems you're not in dev mode. Do you really want to have Spotlight enabled?");
diff --git a/packages/types/src/integration.ts b/packages/types/src/integration.ts
