Skip to content

ref(core): Make on and emit required on client #10603

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 1 commit 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
4 changes: 2 additions & 2 deletions packages/angular/src/errorhandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class SentryErrorHandler implements AngularErrorHandler {
if (this._options.showDialog) {
const client = Sentry.getClient();

if (client && client.on && !this._registeredAfterSendEventHandler) {
if (client && !this._registeredAfterSendEventHandler) {
client.on('afterSendEvent', (event: Event) => {
if (!event.type) {
// eslint-disable-next-line deprecation/deprecation
Expand All @@ -128,7 +128,7 @@ class SentryErrorHandler implements AngularErrorHandler {

// We only want to register this hook once in the lifetime of the error handler
this._registeredAfterSendEventHandler = true;
} else if (!client || !client.on) {
} else if (!client) {
Sentry.showReportDialog({ ...this._options.dialogOptions, eventId });
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ const _breadcrumbsIntegration = ((options: Partial<BreadcrumbsOptions> = {}) =>
if (_options.history) {
addHistoryInstrumentationHandler(_getHistoryBreadcrumbHandler(client));
}
if (_options.sentry && client.on) {
if (_options.sentry) {
client.on('beforeSendEvent', _getSentryBreadcrumbHandler(client));
}
},
Expand Down
5 changes: 0 additions & 5 deletions packages/browser/src/profiling/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ const _browserProfilingIntegration = (() => {
}
}

if (typeof client.on !== 'function') {
logger.warn('[Profiling] Client does not support hooks, profiling will be disabled');
return;
}

client.on('startTransaction', (transaction: Transaction) => {
if (shouldProfileTransaction(transaction)) {
startProfileForTransaction(transaction);
Expand Down
4 changes: 1 addition & 3 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,7 @@ export class Hub implements HubInterface {

if (finalBreadcrumb === null) return;

if (client.emit) {
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
}
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);

// TODO(v8): I know this comment doesn't make much sense because the hub will be deprecated but I still wanted to
// write it down. In theory, we would have to add the breadcrumbs to the isolation scope here, however, that would
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export function setupIntegration(client: Client, integration: Integration, integ
integration.setup(client);
}

if (client.on && typeof integration.preprocessEvent === 'function') {
if (typeof integration.preprocessEvent === 'function') {
const callback = integration.preprocessEvent.bind(integration) as typeof integration.preprocessEvent;
client.on('preprocessEvent', (event, hint) => callback(event, hint, client));
}
Expand Down
4 changes: 0 additions & 4 deletions packages/core/src/integrations/metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ const _moduleMetadataIntegration = (() => {
// TODO v8: Remove this
setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
setup(client) {
if (typeof client.on !== 'function') {
return;
}

// We need to strip metadata from stack frames before sending them to Sentry since these are client side only.
client.on('beforeEnvelope', envelope => {
forEachEnvelopeItem(envelope, (item, type) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tracing/dynamicSamplingContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function getDynamicSamplingContextFromClient(trace_id: string, client: Cl
trace_id,
}) as DynamicSamplingContext;

client.emit && client.emit('createDsc', dsc);
client.emit('createDsc', dsc);

return dsc;
}
Expand Down Expand Up @@ -81,7 +81,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<

dsc.sampled = String(spanIsSampled(txn));

client.emit && client.emit('createDsc', dsc);
client.emit('createDsc', dsc);

return dsc;
}
4 changes: 2 additions & 2 deletions packages/core/src/tracing/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ The transaction will not be sampled. Please use the ${configInstrumenter} instru
if (transaction.isRecording()) {
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
}
if (client && client.emit) {
if (client) {
client.emit('startTransaction', transaction);
}
return transaction;
Expand Down Expand Up @@ -125,7 +125,7 @@ export function startIdleTransaction(
if (transaction.isRecording()) {
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
}
if (client && client.emit) {
if (client) {
client.emit('startTransaction', transaction);
}
return transaction;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ export class Transaction extends SpanClass implements TransactionInterface {

// eslint-disable-next-line deprecation/deprecation
const client = this._hub.getClient();
if (client && client.emit) {
if (client) {
client.emit('finishTransaction', this);
}

Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1957,11 +1957,11 @@ describe('BaseClient', () => {
traceId: '86f39e84263a4de99c326acab3bfe3bd',
} as Transaction;

client.on?.('startTransaction', transaction => {
client.on('startTransaction', transaction => {
expect(transaction).toEqual(mockTransaction);
});

client.emit?.('startTransaction', mockTransaction);
client.emit('startTransaction', mockTransaction);
});

it('should call a beforeEnvelope hook', () => {
Expand All @@ -1974,11 +1974,11 @@ describe('BaseClient', () => {
{},
] as Envelope;

client.on?.('beforeEnvelope', envelope => {
client.on('beforeEnvelope', envelope => {
expect(envelope).toEqual(mockEnvelope);
});

client.emit?.('beforeEnvelope', mockEnvelope);
client.emit('beforeEnvelope', mockEnvelope);
});
});
});
Expand Down
4 changes: 1 addition & 3 deletions packages/feedback/src/util/prepareFeedbackEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ export async function prepareFeedbackEvent({
event,
}: PrepareFeedbackEventParams): Promise<FeedbackEvent | null> {
const eventHint = {};
if (client.emit) {
client.emit('preprocessEvent', event, eventHint);
}
client.emit('preprocessEvent', event, eventHint);

const preparedEvent = (await prepareEvent(
client.getOptions(),
Expand Down
4 changes: 1 addition & 3 deletions packages/feedback/src/util/sendFeedbackRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ export async function sendFeedbackRequest(
return;
}

if (client.emit) {
client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
}
client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });

const envelope = createEventEnvelope(feedbackEvent, dsn, client.getOptions()._metadata, client.getOptions().tunnel);

Expand Down
4 changes: 0 additions & 4 deletions packages/integrations/src/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ const _debugIntegration = ((options: DebugOptions = {}) => {
// TODO v8: Remove this
setupOnce() {}, // eslint-disable-line @typescript-eslint/no-empty-function
setup(client) {
if (!client.on) {
return;
}

client.on('beforeSendEvent', (event: Event, hint?: EventHint) => {
if (_options.debugger) {
// eslint-disable-next-line no-debugger
Expand Down
4 changes: 1 addition & 3 deletions packages/node-experimental/src/sdk/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,7 @@ export function addBreadcrumb(breadcrumb: Breadcrumb, hint?: BreadcrumbHint): vo

if (finalBreadcrumb === null) return;

if (client.emit) {
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);
}
client.emit('beforeAddBreadcrumb', finalBreadcrumb, hint);

getIsolationScope().addBreadcrumb(finalBreadcrumb, maxBreadcrumbs);
}
Expand Down
5 changes: 0 additions & 5 deletions packages/node/src/integrations/spotlight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ function connectToSpotlight(client: Client, options: Required<SpotlightConnectio

let failedRequests = 0;

if (typeof client.on !== 'function') {
logger.warn('[Spotlight] Cannot connect to spotlight due to missing method on SDK client (`client.on`)');
return;
}

client.on('beforeEnvelope', (envelope: Envelope) => {
if (failedRequests > 3) {
logger.warn('[Spotlight] Disabled Sentry -> Spotlight integration due to too many failed requests');
Expand Down
10 changes: 0 additions & 10 deletions packages/node/test/integrations/spotlight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,6 @@ describe('Spotlight', () => {
integration.setup!(client);
expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid sidecar URL: invalid-url'));
});

it("the client doesn't support life cycle hooks", () => {
const integration = spotlightIntegration({ sidecarUrl: 'http://mylocalhost:8969' });
const clientWithoutHooks = { ...client };
// @ts-expect-error - this is fine in tests
delete client.on;
// @ts-expect-error - this is fine in tests
integration.setup(clientWithoutHooks);
expect(loggerSpy).toHaveBeenCalledWith(expect.stringContaining(' missing method on SDK client (`client.on`)'));
});
});

it('warns if the NODE_ENV variable doesn\'t equal "development"', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-node/src/spanprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
const client = getClient();

const mutableOptions = { drop: false };
client && client.emit && client?.emit('otelSpanEnd', otelSpan, mutableOptions);
client && client.emit('otelSpanEnd', otelSpan, mutableOptions);

if (mutableOptions.drop) {
clearSpan(otelSpanId);
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-node/test/propagator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('SentryPropagator', () => {
getDsn: () => ({
publicKey: 'abc',
}),
emit: () => {},
};
// @ts-expect-error Use mock client for unit tests
// eslint-disable-next-line deprecation/deprecation
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry/src/custom/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function startTransaction(hub: HubInterface, transactionContext: Transact
// Any sampling decision happens in OpenTelemetry's sampler
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));

if (client && client.emit) {
if (client) {
client.emit('startTransaction', transaction);
}
return transaction;
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry/test/propagator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ describe('SentryPropagator', () => {
getDsn: () => ({
publicKey: 'abc',
}),
emit: () => {},
};

// @ts-expect-error Use mock client for unit tests
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/errorboundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundarySta
this._openFallbackReportDialog = true;

const client = getClient();
if (client && client.on && props.showDialog) {
if (client && props.showDialog) {
this._openFallbackReportDialog = false;
client.on('afterSendEvent', event => {
if (!event.type && this._lastEventId && event.event_id === this._lastEventId) {
Expand Down
2 changes: 1 addition & 1 deletion packages/replay/src/coreHandlers/handleBreadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type BreadcrumbWithCategory = Required<Pick<Breadcrumb, 'category'>>;
export function handleBreadcrumbs(replay: ReplayContainer): void {
const client = getClient();

if (!client || !client.on) {
if (!client) {
return;
}

Expand Down
42 changes: 0 additions & 42 deletions packages/replay/src/coreHandlers/handleFetch.ts

This file was deleted.

15 changes: 1 addition & 14 deletions packages/replay/src/coreHandlers/handleGlobalEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,13 @@ import { DEBUG_BUILD } from '../debug-build';
import type { ReplayContainer } from '../types';
import { isErrorEvent, isFeedbackEvent, isReplayEvent, isTransactionEvent } from '../util/eventUtils';
import { isRrwebError } from '../util/isRrwebError';
import { handleAfterSendEvent } from './handleAfterSendEvent';
import { addFeedbackBreadcrumb } from './util/addFeedbackBreadcrumb';
import { shouldSampleForBufferEvent } from './util/shouldSampleForBufferEvent';

/**
* Returns a listener to be added to `addEventProcessor(listener)`.
*/
export function handleGlobalEventListener(
replay: ReplayContainer,
includeAfterSendEventHandling = false,
): (event: Event, hint: EventHint) => Event | null {
const afterSendHandler = includeAfterSendEventHandling ? handleAfterSendEvent(replay) : undefined;

export function handleGlobalEventListener(replay: ReplayContainer): (event: Event, hint: EventHint) => Event | null {
return Object.assign(
(event: Event, hint: EventHint) => {
// Do nothing if replay has been disabled
Expand Down Expand Up @@ -73,13 +67,6 @@ export function handleGlobalEventListener(
event.tags = { ...event.tags, replayId: replay.getSessionId() };
}

// In cases where a custom client is used that does not support the new hooks (yet),
// we manually call this hook method here
if (afterSendHandler) {
// Pretend the error had a 200 response so we always capture it
afterSendHandler(event, { statusCode: 200 });
}

return event;
},
{ id: 'Replay' },
Expand Down
10 changes: 2 additions & 8 deletions packages/replay/src/coreHandlers/handleNetworkBreadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ import type {
TextEncoderInternal,
XhrBreadcrumbData,
} from '@sentry/types';
import { addFetchInstrumentationHandler, addXhrInstrumentationHandler, logger } from '@sentry/utils';
import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
import type { FetchHint, ReplayContainer, ReplayNetworkOptions, XhrHint } from '../types';
import { handleFetchSpanListener } from './handleFetch';
import { handleXhrSpanListener } from './handleXhr';
import { captureFetchBreadcrumbToReplay, enrichFetchBreadcrumb } from './util/fetchUtils';
import { captureXhrBreadcrumbToReplay, enrichXhrBreadcrumb } from './util/xhrUtils';

Expand Down Expand Up @@ -50,12 +48,8 @@ export function handleNetworkBreadcrumbs(replay: ReplayContainer): void {
networkResponseHeaders,
};

if (client && client.on) {
if (client) {
client.on('beforeAddBreadcrumb', (breadcrumb, hint) => beforeAddNetworkBreadcrumb(options, breadcrumb, hint));
} else {
// Fallback behavior
addFetchInstrumentationHandler(handleFetchSpanListener(replay));
addXhrInstrumentationHandler(handleXhrSpanListener(replay));
}
} catch {
// Do nothing
Expand Down
Loading