feat(v8): Remove deprecated integration methods on client (getsentry#…

…11134) Removes `getIntegrationById` and `getIntegration` methods on client. Doesn't touch anything on the hub because we want to shim those methods.
cadesalaberry · Apr 19, 2024 · 658cab0 · 658cab0
1 parent ca29bb5
commit 658cab0
Show file tree

Hide file tree

Showing 15 changed files with 22 additions and 60 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -611,7 +611,7 @@ jobs:
     name: Playwright (${{ matrix.bundle }}${{ matrix.shard && format(' {0}/{1}', matrix.shard, matrix.shards) || ''}}) Tests
     needs: [job_get_metadata, job_build]
     if: needs.job_get_metadata.outputs.changed_browser_integration == 'true' || github.event_name != 'pull_request'
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-20.04-large-js
     timeout-minutes: 25
     strategy:
       fail-fast: false

diff --git a/MIGRATION.md b/MIGRATION.md
@@ -355,6 +355,8 @@ Removed top-level exports: `tracingOrigins`, `MetricsAggregator`, `metricsAggreg
 `Sentry.configureScope`, `Span`, `spanStatusfromHttpCode`, `makeMain`, `lastEventId`, `pushScope`, `popScope`,
 `addGlobalEventProcessor`, `timestampWithMs`, `addExtensionMethods`
 
+Remove util exports: `timestampWithMs`
+
 - [Deprecation of `Hub` and `getCurrentHub()`](./MIGRATION.md#deprecate-hub)
 - [Removal of class-based integrations](./MIGRATION.md#removal-of-class-based-integrations)
 - [`tracingOrigins` option replaced with `tracePropagationTargets`](./MIGRATION.md#tracingorigins-has-been-replaced-by-tracepropagationtargets)

diff --git a/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts b/dev-packages/browser-integration-tests/suites/replay/canvas/manualSnapshot/test.ts
@@ -31,7 +31,7 @@ sentryTest('can manually snapshot canvas', async ({ getLocalTestUrl, page, brows
   expect(incrementalSnapshots).toEqual([]);
 
   await page.evaluate(() => {
-    (window as any).Sentry.getClient().getIntegrationById('ReplayCanvas').snapshot();
+    (window as any).Sentry.getClient().getIntegrationByName('ReplayCanvas').snapshot();
   });
 
   const { incrementalSnapshots: incrementalSnapshotsManual } = getReplayRecordingContent(await reqPromise3);

diff --git a/...ntegration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts b/...ntegration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseBody/test.ts
@@ -254,7 +254,9 @@ sentryTest('captures non-text response body', async ({ getLocalTestPath, page, b
   ]);
 });
 
-sentryTest('does not capture response body when URL does not match', async ({ getLocalTestPath, page }) => {
+// This test is flaky
+// See: https://github.com/getsentry/sentry-javascript/issues/11136
+sentryTest.skip('does not capture response body when URL does not match', async ({ getLocalTestPath, page }) => {
   if (shouldSkipReplayTest()) {
     sentryTest.skip();
   }

diff --git a/...gration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts b/...gration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseHeaders/test.ts
@@ -157,7 +157,9 @@ sentryTest('captures response headers', async ({ getLocalTestPath, page }) => {
   ]);
 });
 
-sentryTest('does not capture response headers if URL does not match', async ({ getLocalTestPath, page }) => {
+// This test is flaky so it's skipped for now
+// See https://github.com/getsentry/sentry-javascript/issues/11139
+sentryTest.skip('does not capture response headers if URL does not match', async ({ getLocalTestPath, page }) => {
   if (shouldSkipReplayTest()) {
     sentryTest.skip();
   }

diff --git a/...ntegration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts b/...ntegration-tests/suites/replay/extendNetworkBreadcrumbs/fetch/captureResponseSize/test.ts
@@ -188,7 +188,9 @@ sentryTest('captures response size without Content-Length header', async ({ getL
   ]);
 });
 
-sentryTest('captures response size from non-text response body', async ({ getLocalTestPath, page }) => {
+// This test is flaky so it's skipped for now
+// See https://github.com/getsentry/sentry-javascript/issues/11137
+sentryTest.skip('captures response size from non-text response body', async ({ getLocalTestPath, page }) => {
   if (shouldSkipReplayTest()) {
     sentryTest.skip();
   }

diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/handlers-lcp/test.ts
@@ -7,7 +7,9 @@ import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../.
 
 const bundle = process.env.PW_BUNDLE || '';
 
-sentryTest(
+// This test is flaky so it's skipped for now
+// See https://github.com/getsentry/sentry-javascript/issues/11138
+sentryTest.skip(
   'should capture metrics for LCP instrumentation handlers',
   async ({ browserName, getLocalTestPath, page }) => {
     // This uses a utility that is not exported in CDN bundles

diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -15,7 +15,6 @@ import type {
   EventProcessor,
   FeedbackEvent,
   Integration,
-  IntegrationClass,
   Outcome,
   ParameterizedString,
   SdkMetadata,
@@ -317,16 +316,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     }
   }
 
-  /**
-   * Gets an installed integration by its `id`.
-   *
-   * @returns The installed integration or `undefined` if no integration with that `id` was installed.
-   * @deprecated Use `getIntegrationByName()` instead.
-   */
-  public getIntegrationById(integrationId: string): Integration | undefined {
-    return this.getIntegrationByName(integrationId);
-  }
-
   /**
    * Gets an installed integration by its name.
    *
@@ -336,19 +325,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     return this._integrations[integrationName] as T | undefined;
   }
 
-  /**
-   * Returns the client's instance of the given integration class, it any.
-   * @deprecated Use `getIntegrationByName()` instead.
-   */
-  public getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
-    try {
-      return (this._integrations[integration.id] as T) || null;
-    } catch (_oO) {
-      DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
-      return null;
-    }
-  }
-
   /**
    * @inheritDoc
    */

diff --git a/packages/core/src/hub.ts b/packages/core/src/hub.ts
@@ -428,8 +428,7 @@ export class Hub implements HubInterface {
     const client = this.getClient();
     if (!client) return null;
     try {
-      // eslint-disable-next-line deprecation/deprecation
-      return client.getIntegration(integration);
+      return client.getIntegrationByName<T>(integration.id) || null;
     } catch (_oO) {
       DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Hub`);
       return null;

diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -42,9 +42,6 @@ jest.mock('@sentry/utils', () => {
     truncate(str: string): string {
       return str;
     },
-    timestampWithMs(): number {
-      return 2020;
-    },
     dateTimestampInSeconds(): number {
       return 2020;
     },

diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts
@@ -170,8 +170,7 @@ export class Undici implements Integration {
   }
 
   private _onRequestCreate = (message: unknown): void => {
-    // eslint-disable-next-line deprecation/deprecation
-    if (!getClient()?.getIntegration(Undici)) {
+    if (!getClient()?.getIntegrationByName('Undici')) {
       return;
     }
 
@@ -229,8 +228,7 @@ export class Undici implements Integration {
   };
 
   private _onRequestEnd = (message: unknown): void => {
-    // eslint-disable-next-line deprecation/deprecation
-    if (!getClient()?.getIntegration(Undici)) {
+    if (!getClient()?.getIntegrationByName('Undici')) {
       return;
     }
 
@@ -269,8 +267,7 @@ export class Undici implements Integration {
   };
 
   private _onRequestError = (message: unknown): void => {
-    // eslint-disable-next-line deprecation/deprecation
-    if (!getClient()?.getIntegration(Undici)) {
+    if (!getClient()?.getIntegrationByName('Undici')) {
       return;
     }
 

diff --git a/packages/opentelemetry/src/custom/getCurrentHub.ts b/packages/opentelemetry/src/custom/getCurrentHub.ts
@@ -62,8 +62,7 @@ export function getCurrentHub(): Hub {
     setContext,
 
     getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null {
-      // eslint-disable-next-line deprecation/deprecation
-      return getClient()?.getIntegration(integration) || null;
+      return getClient()?.getIntegrationByName<T>(integration.id) || null;
     },
 
     startSession,

diff --git a/packages/sveltekit/src/server/index.ts b/packages/sveltekit/src/server/index.ts
@@ -20,8 +20,6 @@ export {
   getIsolationScope,
   Hub,
   NodeClient,
-  // eslint-disable-next-line deprecation/deprecation
-  makeMain,
   setCurrentClient,
   Scope,
   SDK_VERSION,

diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts
@@ -7,7 +7,7 @@ import type { DynamicSamplingContext, Envelope } from './envelope';
 import type { Event, EventHint } from './event';
 import type { EventProcessor } from './eventprocessor';
 import type { FeedbackEvent } from './feedback';
-import type { Integration, IntegrationClass } from './integration';
+import type { Integration } from './integration';
 import type { ClientOptions } from './options';
 import type { ParameterizedString } from './parameterize';
 import type { Scope } from './scope';
@@ -127,12 +127,6 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    */
   getEventProcessors(): EventProcessor[];
 
-  /**
-   * Returns the client's instance of the given integration class, it any.
-   * @deprecated Use `getIntegrationByName()` instead.
-   */
-  getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null;
-
   /** Get the instance of the integration with the given name on the client, if it was added. */
   getIntegrationByName<T extends Integration = Integration>(name: string): T | undefined;
 

diff --git a/packages/utils/src/time.ts b/packages/utils/src/time.ts
@@ -68,14 +68,6 @@ function createUnixTimestampInSecondsFunc(): () => number {
  */
 export const timestampInSeconds = createUnixTimestampInSecondsFunc();
 
-/**
- * Re-exported with an old name for backwards-compatibility.
- * TODO (v8): Remove this
- *
- * @deprecated Use `timestampInSeconds` instead.
- */
-export const timestampWithMs = timestampInSeconds;
-
 /**
  * Internal helper to store what is the source of browserPerformanceTimeOrigin below. For debugging only.
  */