Skip to content

Commit

Permalink
ref(browser): Refactor sentry breadcrumb to use hook (#8892)
Browse files Browse the repository at this point in the history
Noticed that this is a bit tightly coupled in the browser client, and
could be simplified by using a hook.
  • Loading branch information
mydea authored Aug 29, 2023
1 parent e3dda4c commit e72c047
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 42 deletions.
22 changes: 0 additions & 22 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { createClientReportEnvelope, dsnToString, getSDKSource, logger } from '@

import { eventFromException, eventFromMessage } from './eventbuilder';
import { WINDOW } from './helpers';
import type { Breadcrumbs } from './integrations';
import { BREADCRUMB_INTEGRATION_ID } from './integrations/breadcrumbs';
import type { BrowserTransportOptions } from './transports/types';
import { createUserFeedbackEnvelope } from './userfeedback';

Expand Down Expand Up @@ -91,26 +89,6 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
return eventFromMessage(this._options.stackParser, message, level, hint, this._options.attachStacktrace);
}

/**
* @inheritDoc
*/
public sendEvent(event: Event, hint?: EventHint): void {
// We only want to add the sentry event breadcrumb when the user has the breadcrumb integration installed and
// activated its `sentry` option.
// We also do not want to use the `Breadcrumbs` class here directly, because we do not want it to be included in
// bundles, if it is not used by the SDK.
// This all sadly is a bit ugly, but we currently don't have a "pre-send" hook on the integrations so we do it this
// way for now.
const breadcrumbIntegration = this.getIntegrationById(BREADCRUMB_INTEGRATION_ID) as Breadcrumbs | undefined;
// We check for definedness of `addSentryBreadcrumb` in case users provided their own integration with id
// "Breadcrumbs" that does not have this function.
if (breadcrumbIntegration && breadcrumbIntegration.addSentryBreadcrumb) {
breadcrumbIntegration.addSentryBreadcrumb(event);
}

super.sendEvent(event, hint);
}

/**
* Sends user feedback to Sentry.
*/
Expand Down
40 changes: 20 additions & 20 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ interface BreadcrumbsOptions {
/** maxStringLength gets capped to prevent 100 breadcrumbs exceeding 1MB event payload size */
const MAX_ALLOWED_STRING_LENGTH = 1024;

export const BREADCRUMB_INTEGRATION_ID = 'Breadcrumbs';

/**
* Default Breadcrumbs instrumentations
* TODO: Deprecated - with v6, this will be renamed to `Instrument`
Expand All @@ -51,7 +49,7 @@ export class Breadcrumbs implements Integration {
/**
* @inheritDoc
*/
public static id: string = BREADCRUMB_INTEGRATION_ID;
public static id: string = 'Breadcrumbs';

/**
* @inheritDoc
Expand Down Expand Up @@ -104,28 +102,30 @@ export class Breadcrumbs implements Integration {
if (this.options.history) {
addInstrumentationHandler('history', _historyBreadcrumb);
}
}

/**
* Adds a breadcrumb for Sentry events or transactions if this option is enabled.
*/
public addSentryBreadcrumb(event: SentryEvent): void {
if (this.options.sentry) {
getCurrentHub().addBreadcrumb(
{
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
event_id: event.event_id,
level: event.level,
message: getEventDescription(event),
},
{
event,
},
);
const client = getCurrentHub().getClient();
client && client.on && client.on('beforeSendEvent', addSentryBreadcrumb);
}
}
}

/**
* Adds a breadcrumb for Sentry events or transactions if this option is enabled.
*/
function addSentryBreadcrumb(event: SentryEvent): void {
getCurrentHub().addBreadcrumb(
{
category: `sentry.${event.type === 'transaction' ? 'transaction' : 'event'}`,
event_id: event.event_id,
level: event.level,
message: getEventDescription(event),
},
{
event,
},
);
}

/**
* A HOC that creaes a function that creates breadcrumbs from DOM API calls.
* This is a HOC so that we get access to dom options in the closure.
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* @inheritDoc
*/
public sendEvent(event: Event, hint: EventHint = {}): void {
this.emit('beforeSendEvent', event, hint);

if (this._dsn) {
let env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel);

Expand Down Expand Up @@ -381,6 +383,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public on(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;

/** @inheritdoc */
public on(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint) => void): void;

/** @inheritdoc */
public on(
hook: 'afterSendEvent',
Expand Down Expand Up @@ -412,6 +417,9 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
/** @inheritdoc */
public emit(hook: 'beforeEnvelope', envelope: Envelope): void;

/** @inheritdoc */
public emit(hook: 'beforeSendEvent', event: Event, hint?: EventHint): void;

/** @inheritdoc */
public emit(hook: 'afterSendEvent', event: Event, sendResponse: TransportMakeRequestResponse | void): void;

Expand Down
11 changes: 11 additions & 0 deletions packages/types/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
on?(hook: 'beforeEnvelope', callback: (envelope: Envelope) => void): void;

/**
* Register a callback for before an event is sent.
*/
on?(hook: 'beforeSendEvent', callback: (event: Event, hint?: EventHint | void) => void): void;

/**
* Register a callback for when an event has been sent.
*/
Expand Down Expand Up @@ -212,6 +217,12 @@ export interface Client<O extends ClientOptions = ClientOptions> {
*/
emit?(hook: 'beforeEnvelope', envelope: Envelope): void;

/*
* Fire a hook event before sending an event. Expects to be given an Event & EventHint as the
* second/third argument.
*/
emit?(hook: 'beforeSendEvent', event: Event, hint?: EventHint): void;

/*
* Fire a hook event after sending an event. Expects to be given an Event as the
* second argument.
Expand Down

0 comments on commit e72c047

Please sign in to comment.