Skip to content

ref(integrations): Rewrite pluggable integrations to use functional style #9936

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 5 commits into from
Dec 21, 2023
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
26 changes: 15 additions & 11 deletions packages/core/src/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,24 +175,28 @@ function findIndex<T>(arr: T[], callback: (item: T) => boolean): number {
export function convertIntegrationFnToClass<Fn extends IntegrationFn>(
name: string,
fn: Fn,
): IntegrationClass<
Integration & {
setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
}
> {
): {
id: string;
new (...args: Parameters<Fn>): Integration &
ReturnType<Fn> & {
setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
};
} {
return Object.assign(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function ConvertedIntegration(...rest: any[]) {
function ConvertedIntegration(...rest: Parameters<Fn>) {
return {
// eslint-disable-next-line @typescript-eslint/no-empty-function
setupOnce: () => {},
...fn(...rest),
};
},
{ id: name },
) as unknown as IntegrationClass<
Integration & {
setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
}
>;
) as unknown as {
id: string;
new (...args: Parameters<Fn>): Integration &
ReturnType<Fn> & {
setupOnce: (addGlobalEventProcessor?: (callback: EventProcessor) => void, getCurrentHub?: () => Hub) => void;
};
};
}
17 changes: 17 additions & 0 deletions packages/core/test/lib/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,23 @@ describe('convertIntegrationFnToClass', () => {
});
});

it('works with options', () => {
const integrationFn = (_options: { num: number }) => ({ name: 'testName' });

const IntegrationClass = convertIntegrationFnToClass('testName', integrationFn);

expect(IntegrationClass.id).toBe('testName');

// @ts-expect-error This should fail TS without options
new IntegrationClass();

const integration = new IntegrationClass({ num: 3 });
expect(integration).toEqual({
name: 'testName',
setupOnce: expect.any(Function),
});
});

it('works with integration hooks', () => {
const setup = jest.fn();
const setupOnce = jest.fn();
Expand Down
6 changes: 3 additions & 3 deletions packages/deno/src/integrations/deno-cron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type CronParams = [string, string | Deno.CronSchedule, CronFn | CronOptions, Cro

const INTEGRATION_NAME = 'DenoCron';

const SETUP_CLIENTS: Client[] = [];
const SETUP_CLIENTS = new WeakMap<Client, boolean>();

const denoCronIntegration = (() => {
return {
Expand Down Expand Up @@ -37,7 +37,7 @@ const denoCronIntegration = (() => {
}

async function cronCalled(): Promise<void> {
if (SETUP_CLIENTS.includes(getClient() as Client)) {
if (SETUP_CLIENTS.has(getClient() as Client)) {
return;
}

Expand All @@ -55,7 +55,7 @@ const denoCronIntegration = (() => {
});
},
setup(client) {
SETUP_CLIENTS.push(client);
SETUP_CLIENTS.set(client, true);
},
};
}) satisfies IntegrationFn;
Expand Down
71 changes: 26 additions & 45 deletions packages/integrations/src/captureconsole.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { captureException, captureMessage, getClient, withScope } from '@sentry/core';
import type { CaptureContext, Client, EventProcessor, Hub, Integration } from '@sentry/types';
import { captureException, captureMessage, convertIntegrationFnToClass, getClient, withScope } from '@sentry/core';
import type { CaptureContext, IntegrationFn } from '@sentry/types';
import {
CONSOLE_LEVELS,
GLOBAL_OBJ,
Expand All @@ -9,55 +9,36 @@ import {
severityLevelFromString,
} from '@sentry/utils';

/** Send Console API calls as Sentry Events */
export class CaptureConsole implements Integration {
/**
* @inheritDoc
*/
public static id: string = 'CaptureConsole';

/**
* @inheritDoc
*/
public name: string;

/**
* @inheritDoc
*/
private readonly _levels: readonly string[];

/**
* @inheritDoc
*/
public constructor(options: { levels?: string[] } = {}) {
this.name = CaptureConsole.id;
this._levels = options.levels || CONSOLE_LEVELS;
}

/**
* @inheritDoc
*/
public setupOnce(_: (callback: EventProcessor) => void, _getCurrentHub: () => Hub): void {
// noop
}
interface CaptureConsoleOptions {
levels?: string[];
}

/** @inheritdoc */
public setup(client: Client): void {
if (!('console' in GLOBAL_OBJ)) {
return;
}
const INTEGRATION_NAME = 'CaptureConsole';

const levels = this._levels;
const captureConsoleIntegration = ((options: CaptureConsoleOptions = {}) => {
const levels = options.levels || CONSOLE_LEVELS;

addConsoleInstrumentationHandler(({ args, level }) => {
if (getClient() !== client || !levels.includes(level)) {
return {
name: INTEGRATION_NAME,
setup(client) {
if (!('console' in GLOBAL_OBJ)) {
return;
}

consoleHandler(args, level);
});
}
}
addConsoleInstrumentationHandler(({ args, level }) => {
if (getClient() !== client || !levels.includes(level)) {
return;
}

consoleHandler(args, level);
});
},
};
}) satisfies IntegrationFn;

/** Send Console API calls as Sentry Events */
// eslint-disable-next-line deprecation/deprecation
export const CaptureConsole = convertIntegrationFnToClass(INTEGRATION_NAME, captureConsoleIntegration);

function consoleHandler(args: unknown[], level: string): void {
const captureContext: CaptureContext = {
Expand Down
98 changes: 40 additions & 58 deletions packages/integrations/src/contextlines.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import type { Event, Integration, StackFrame } from '@sentry/types';
import { convertIntegrationFnToClass } from '@sentry/core';
import type { Event, IntegrationFn, StackFrame } from '@sentry/types';
import { GLOBAL_OBJ, addContextToFrame, stripUrlQueryAndFragment } from '@sentry/utils';

const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;

const DEFAULT_LINES_OF_CONTEXT = 7;

const INTEGRATION_NAME = 'ContextLines';

interface ContextLinesOptions {
/**
* Sets the number of context lines for each frame when loading a file.
Expand All @@ -15,6 +18,17 @@ interface ContextLinesOptions {
frameContextLines?: number;
}

const contextLinesIntegration: IntegrationFn = (options: ContextLinesOptions = {}) => {
const contextLines = options.frameContextLines != null ? options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;

return {
name: INTEGRATION_NAME,
processEvent(event) {
return addSourceContext(event, contextLines);
},
};
};

/**
* Collects source context lines around the lines of stackframes pointing to JS embedded in
* the current page's HTML.
Expand All @@ -26,73 +40,41 @@ interface ContextLinesOptions {
* Use this integration if you have inline JS code in HTML pages that can't be accessed
* by our backend (e.g. due to a login-protected page).
*/
export class ContextLines implements Integration {
/**
* @inheritDoc
*/
public static id: string = 'ContextLines';

/**
* @inheritDoc
*/
public name: string;
// eslint-disable-next-line deprecation/deprecation
export const ContextLines = convertIntegrationFnToClass(INTEGRATION_NAME, contextLinesIntegration);

public constructor(private readonly _options: ContextLinesOptions = {}) {
this.name = ContextLines.id;
/**
* Processes an event and adds context lines.
*/
function addSourceContext(event: Event, contextLines: number): Event {
const doc = WINDOW.document;
const htmlFilename = WINDOW.location && stripUrlQueryAndFragment(WINDOW.location.href);
if (!doc || !htmlFilename) {
return event;
}

/**
* @inheritDoc
*/
public setupOnce(_addGlobalEventProcessor: unknown, _getCurrentHub: unknown): void {
// noop
const exceptions = event.exception && event.exception.values;
if (!exceptions || !exceptions.length) {
return event;
}

/** @inheritDoc */
public processEvent(event: Event): Event {
return this.addSourceContext(event);
const html = doc.documentElement.innerHTML;
if (!html) {
return event;
}

/**
* Processes an event and adds context lines.
*
* TODO (v8): Make this internal/private
*/
public addSourceContext(event: Event): Event {
const doc = WINDOW.document;
const htmlFilename = WINDOW.location && stripUrlQueryAndFragment(WINDOW.location.href);
if (!doc || !htmlFilename) {
return event;
}

const exceptions = event.exception && event.exception.values;
if (!exceptions || !exceptions.length) {
return event;
}
const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];

const html = doc.documentElement.innerHTML;
if (!html) {
return event;
exceptions.forEach(exception => {
const stacktrace = exception.stacktrace;
if (stacktrace && stacktrace.frames) {
stacktrace.frames = stacktrace.frames.map(frame =>
applySourceContextToFrame(frame, htmlLines, htmlFilename, contextLines),
);
}
});

const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];

exceptions.forEach(exception => {
const stacktrace = exception.stacktrace;
if (stacktrace && stacktrace.frames) {
stacktrace.frames = stacktrace.frames.map(frame =>
applySourceContextToFrame(
frame,
htmlLines,
htmlFilename,
this._options.frameContextLines != null ? this._options.frameContextLines : DEFAULT_LINES_OF_CONTEXT,
),
);
}
});

return event;
}
return event;
}

/**
Expand Down
Loading