Skip to content

ref(core): Make remaining client methods required #10605

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ function _sendSessionUpdate(): void {
// TODO (v8): Remove currentScope and only use the isolation scope(?).
// For v7 though, we can't "soft-break" people using getCurrentHub().getScope().setSession()
const session = currentScope.getSession() || isolationScope.getSession();
if (session && client && client.captureSession) {
if (session && client) {
client.captureSession(session);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ export function setupIntegration(client: Client, integration: Integration, integ
client.on('preprocessEvent', (event, hint) => callback(event, hint, client));
}

if (client.addEventProcessor && typeof integration.processEvent === 'function') {
if (typeof integration.processEvent === 'function') {
const callback = integration.processEvent.bind(integration) as typeof integration.processEvent;

const processor = Object.assign((event: Event, hint: EventHint) => callback(event, hint, client), {
Expand All @@ -162,7 +162,7 @@ export function setupIntegration(client: Client, integration: Integration, integ
export function addIntegration(integration: Integration): void {
const client = getClient();

if (!client || !client.addIntegration) {
if (!client) {
DEBUG_BUILD && logger.warn(`Cannot add integration "${integration.name}" because no SDK Client is available.`);
return;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/metrics/aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export class MetricsAggregator implements MetricsAggregatorBase {
* @param flushedBuckets
*/
private _captureMetrics(flushedBuckets: MetricBucket): void {
if (flushedBuckets.size > 0 && this._client.captureAggregateMetrics) {
if (flushedBuckets.size > 0) {
// TODO(@anonrig): Optimization opportunity.
// This copy operation can be avoided if we store the key in the bucketItem.
const buckets = Array.from(flushedBuckets).map(([, bucketItem]) => bucketItem);
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/metrics/browser-aggregator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ export class BrowserMetricsAggregator implements MetricsAggregator {
if (this._buckets.size === 0) {
return;
}
if (this._client.captureAggregateMetrics) {
// TODO(@anonrig): Use Object.values() when we support ES6+
const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem);
this._client.captureAggregateMetrics(metricBuckets);
}

// TODO(@anonrig): Use Object.values() when we support ES6+
const metricBuckets = Array.from(this._buckets).map(([, bucketItem]) => bucketItem);
this._client.captureAggregateMetrics(metricBuckets);

this._buckets.clear();
}

Expand Down
17 changes: 1 addition & 16 deletions packages/core/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function initAndBind<F extends Client, O extends ClientOptions>(

const client = new clientClass(options);
setCurrentClient(client);
initializeClient(client);
client.init();
}

/**
Expand All @@ -49,18 +49,3 @@ export function setCurrentClient(client: Client): void {
top.client = client;
top.scope.setClient(client);
}

/**
* Initialize the client for the current scope.
* Make sure to call this after `setCurrentClient()`.
*/
function initializeClient(client: Client): void {
if (client.init) {
client.init();
// TODO v8: Remove this fallback
// eslint-disable-next-line deprecation/deprecation
} else if (client.setupIntegrations) {
// eslint-disable-next-line deprecation/deprecation
client.setupIntegrations();
}
}
2 changes: 1 addition & 1 deletion packages/core/src/utils/prepareEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function prepareEvent(
addExceptionMechanism(prepared, hint.mechanism);
}

const clientEventProcessors = client && client.getEventProcessors ? client.getEventProcessors() : [];
const clientEventProcessors = client ? client.getEventProcessors() : [];

// This should be the last thing called, since we want that
// {@link Hub.addEventProcessor} gets the finished prepared event.
Expand Down
2 changes: 1 addition & 1 deletion packages/node-experimental/src/sdk/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function _sendSessionUpdate(): void {
const client = getClient();

const session = scope.getSession();
if (session && client.captureSession) {
if (session) {
client.captureSession(session);
}
}
Expand Down
23 changes: 11 additions & 12 deletions packages/node-experimental/src/sdk/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,18 @@ export function init(options: NodeExperimentalOptions | undefined = {}): void {

if (options.spotlight) {
const client = getClient();
if (client.addIntegration) {
// force integrations to be setup even if no DSN was set
// If they have already been added before, they will be ignored anyhow
const integrations = client.getOptions().integrations;
for (const integration of integrations) {
client.addIntegration(integration);
}
client.addIntegration(
spotlightIntegration({
sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined,
}),
);

// force integrations to be setup even if no DSN was set
// If they have already been added before, they will be ignored anyhow
const integrations = client.getOptions().integrations;
for (const integration of integrations) {
client.addIntegration(integration);
}
client.addIntegration(
spotlightIntegration({
sidecarUrl: typeof options.spotlight === 'string' ? options.spotlight : undefined,
}),
);
}

// Always init Otel, even if tracing is disabled, because we need it for trace propagation & the HTTP integration
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export function init(options: NodeOptions = {}): void {

if (options.spotlight) {
const client = getClient();
if (client && client.addIntegration) {
if (client) {
// force integrations to be setup even if no DSN was set
// If they have already been added before, they will be ignored anyhow
const integrations = client.getOptions().integrations;
Expand Down
4 changes: 0 additions & 4 deletions packages/opentelemetry/src/setupEventContextTrace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import { spanHasParentId } from './utils/spanTypes';

/** Ensure the `trace` context is set on all events. */
export function setupEventContextTrace(client: Client): void {
if (!client.addEventProcessor) {
return;
}

client.addEventProcessor(event => {
const span = getActiveSpan();
if (!span) {
Expand Down
2 changes: 1 addition & 1 deletion packages/replay/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ Sentry.init({ replaysOnErrorSampleRate: ${errorSampleRate} })`,
/* eslint-disable @typescript-eslint/no-non-null-assertion */
try {
const client = getClient()!;
const canvasIntegration = client.getIntegrationByName!('ReplayCanvas') as Integration & {
const canvasIntegration = client.getIntegrationByName('ReplayCanvas') as Integration & {
getOptions(): ReplayCanvasIntegrationOptions;
};
if (!canvasIntegration) {
Expand Down
6 changes: 1 addition & 5 deletions packages/replay/src/util/addGlobalListeners.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ export function addGlobalListeners(replay: ReplayContainer): void {
// Tag all (non replay) events that get sent to Sentry with the current
// replay ID so that we can reference them later in the UI
const eventProcessor = handleGlobalEventListener(replay);
if (client && client.addEventProcessor) {
client.addEventProcessor(eventProcessor);
} else {
addEventProcessor(eventProcessor);
}
addEventProcessor(eventProcessor);

// If a custom client has no hooks yet, we continue to use the "old" implementation
if (client) {
Expand Down
4 changes: 1 addition & 3 deletions packages/replay/src/util/getReplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,5 @@ import type { replayIntegration } from '../integration';
// eslint-disable-next-line deprecation/deprecation
export function getReplay(): ReturnType<typeof replayIntegration> | undefined {
const client = getClient();
return (
client && client.getIntegrationByName && client.getIntegrationByName<ReturnType<typeof replayIntegration>>('Replay')
);
return client && client.getIntegrationByName<ReturnType<typeof replayIntegration>>('Replay');
}
2 changes: 1 addition & 1 deletion packages/replay/src/util/prepareReplayEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export async function prepareReplayEvent({
preparedEvent.platform = preparedEvent.platform || 'javascript';

// extract the SDK name because `client._prepareEvent` doesn't add it to the event
const metadata = client.getSdkMetadata && client.getSdkMetadata();
const metadata = client.getSdkMetadata();
const { name, version } = (metadata && metadata.sdk) || {};

preparedEvent.sdk = {
Expand Down
22 changes: 8 additions & 14 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*
* @param session Session to be delivered
*/
captureSession?(session: Session): void;
captureSession(session: Session): void;

/**
* Create a cron monitor check in and send it to Sentry. This method is not available on all clients.
Expand All @@ -87,9 +87,8 @@ export interface Client<O extends ClientOptions = ClientOptions> {
/**
* @inheritdoc
*
* TODO (v8): Make this a required method.
*/
getSdkMetadata?(): SdkMetadata | undefined;
getSdkMetadata(): SdkMetadata | undefined;

/**
* Returns the transport that is used by the client.
Expand Down Expand Up @@ -121,17 +120,13 @@ export interface Client<O extends ClientOptions = ClientOptions> {

/**
* Adds an event processor that applies to any event processed by this client.
*
* TODO (v8): Make this a required method.
*/
addEventProcessor?(eventProcessor: EventProcessor): void;
addEventProcessor(eventProcessor: EventProcessor): void;

/**
* Get all added event processors for this client.
*
* TODO (v8): Make this a required method.
*/
getEventProcessors?(): EventProcessor[];
getEventProcessors(): EventProcessor[];

/**
* Returns the client's instance of the given integration class, it any.
Expand All @@ -140,17 +135,16 @@ export interface Client<O extends ClientOptions = ClientOptions> {
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;
getIntegrationByName<T extends Integration = Integration>(name: string): T | undefined;

/**
* Add an integration to the client.
* This can be used to e.g. lazy load integrations.
* In most cases, this should not be necessary, and you're better off just passing the integrations via `integrations: []` at initialization time.
* However, if you find the need to conditionally load & add an integration, you can use `addIntegration` to do so.
*
* TODO (v8): Make this a required method.
* */
addIntegration?(integration: Integration): void;
addIntegration(integration: Integration): void;

/**
* This is an internal function to setup all integrations that should run on the client.
Expand All @@ -162,7 +156,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
* Initialize this client.
* Call this after the client was set on a scope.
*/
init?(): void;
init(): void;

/** Creates an {@link Event} from all inputs to `captureException` and non-primitive inputs to `captureMessage`. */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -191,7 +185,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*
* @experimental This API is experimental and might experience breaking changes
*/
captureAggregateMetrics?(metricBucketItems: Array<MetricBucketItem>): void;
captureAggregateMetrics(metricBucketItems: Array<MetricBucketItem>): void;

// HOOKS
// TODO(v8): Make the hooks non-optional.
Expand Down