Skip to content

ref(core): Make hint callback argument non-optional #5141

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
May 25, 2022
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
58 changes: 29 additions & 29 deletions packages/core/test/lib/integrations/inboundfilters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,16 +180,16 @@ describe('InboundFilters', () => {
describe('_isSentryError', () => {
it('should work as expected', () => {
const eventProcessor = createInboundFiltersEventProcessor();
expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT);
expect(eventProcessor(EXCEPTION_EVENT)).toBe(EXCEPTION_EVENT);
expect(eventProcessor(SENTRY_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(MESSAGE_EVENT);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(EXCEPTION_EVENT);
expect(eventProcessor(SENTRY_EVENT, {})).toBe(null);
});

it('should be configurable', () => {
const eventProcessor = createInboundFiltersEventProcessor({ ignoreInternal: false });
expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT);
expect(eventProcessor(EXCEPTION_EVENT)).toBe(EXCEPTION_EVENT);
expect(eventProcessor(SENTRY_EVENT)).toBe(SENTRY_EVENT);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(MESSAGE_EVENT);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(EXCEPTION_EVENT);
expect(eventProcessor(SENTRY_EVENT, {})).toBe(SENTRY_EVENT);
});
});

Expand All @@ -198,29 +198,29 @@ describe('InboundFilters', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: ['capture'],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(null);
});

it('string filter with exact match', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: ['captureMessage'],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(null);
});

it('regexp filter with partial match', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: [/capture/],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(null);
});

it('regexp filter with exact match', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: [/^captureMessage$/],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT_2)).toBe(MESSAGE_EVENT_2);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(null);
expect(eventProcessor(MESSAGE_EVENT_2, {})).toBe(MESSAGE_EVENT_2);
});

it('prefers message when both message and exception are available', () => {
Expand All @@ -231,42 +231,42 @@ describe('InboundFilters', () => {
...EXCEPTION_EVENT,
...MESSAGE_EVENT,
};
expect(eventProcessor(event)).toBe(null);
expect(eventProcessor(event, {})).toBe(null);
});

it('can use multiple filters', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: ['captureMessage', /SyntaxError/],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(null);
});

it('uses default filters', () => {
const eventProcessor = createInboundFiltersEventProcessor();
expect(eventProcessor(SCRIPT_ERROR_EVENT)).toBe(null);
expect(eventProcessor(SCRIPT_ERROR_EVENT, {})).toBe(null);
});

describe('on exception', () => {
it('uses exception data when message is unavailable', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: ['SyntaxError: unidentified ? at line 1337'],
});
expect(eventProcessor(EXCEPTION_EVENT)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(null);
});

it('can match on exception value', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: [/unidentified \?/],
});
expect(eventProcessor(EXCEPTION_EVENT)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(null);
});

it('can match on exception type', () => {
const eventProcessor = createInboundFiltersEventProcessor({
ignoreErrors: [/^SyntaxError/],
});
expect(eventProcessor(EXCEPTION_EVENT)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT, {})).toBe(null);
});
});
});
Expand All @@ -276,78 +276,78 @@ describe('InboundFilters', () => {
const eventProcessorDeny = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io'],
});
expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null);
expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE, {})).toBe(null);
});

it('should allow denyUrls to take precedence', () => {
const eventProcessorBoth = createInboundFiltersEventProcessor({
allowUrls: ['https://awesome-analytics.io'],
denyUrls: ['https://awesome-analytics.io'],
});
expect(eventProcessorBoth(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null);
expect(eventProcessorBoth(MESSAGE_EVENT_WITH_STACKTRACE, {})).toBe(null);
});

it('should filter captured message based on its stack trace using regexp filter', () => {
const eventProcessorDeny = createInboundFiltersEventProcessor({
denyUrls: [/awesome-analytics\.io/],
});
expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE)).toBe(null);
expect(eventProcessorDeny(MESSAGE_EVENT_WITH_STACKTRACE, {})).toBe(null);
});

it('should not filter captured messages with no stacktraces', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io'],
});
expect(eventProcessor(MESSAGE_EVENT)).toBe(MESSAGE_EVENT);
expect(eventProcessor(MESSAGE_EVENT, {})).toBe(MESSAGE_EVENT);
});

it('should filter captured exception based on its stack trace using string filter', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io'],
});
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES, {})).toBe(null);
});

it('should filter captured exception based on its stack trace using regexp filter', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: [/awesome-analytics\.io/],
});
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES, {})).toBe(null);
});

it("should not filter events that don't match the filtered values", () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['some-other-domain.com'],
});
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(EXCEPTION_EVENT_WITH_FRAMES);
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES, {})).toBe(EXCEPTION_EVENT_WITH_FRAMES);
});

it('should be able to use multiple filters', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['some-other-domain.com', /awesome-analytics\.io/],
});
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES)).toBe(null);
expect(eventProcessor(EXCEPTION_EVENT_WITH_FRAMES, {})).toBe(null);
});

it('should not fail with malformed event event', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io'],
});
expect(eventProcessor(MALFORMED_EVENT)).toBe(MALFORMED_EVENT);
expect(eventProcessor(MALFORMED_EVENT, {})).toBe(MALFORMED_EVENT);
});

it('should search for script names when there is an anonymous callback at the last frame', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io/some/file.js'],
});
expect(eventProcessor(MESSAGE_EVENT_WITH_ANON_LAST_FRAME)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT_WITH_ANON_LAST_FRAME, {})).toBe(null);
});

it('should search for script names when the last frame is from native code', () => {
const eventProcessor = createInboundFiltersEventProcessor({
denyUrls: ['https://awesome-analytics.io/some/file.js'],
});
expect(eventProcessor(MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME)).toBe(null);
expect(eventProcessor(MESSAGE_EVENT_WITH_NATIVE_LAST_FRAME, {})).toBe(null);
});
});
});
4 changes: 2 additions & 2 deletions packages/hub/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ export class Scope implements ScopeInterface {
* @param hint May contain additional information about the original exception.
* @hidden
*/
public applyToEvent(event: Event, hint?: EventHint): PromiseLike<Event | null> {
public applyToEvent(event: Event, hint: EventHint = {}): PromiseLike<Event | null> {
if (this._extra && Object.keys(this._extra).length) {
event.extra = { ...this._extra, ...event.extra };
}
Expand Down Expand Up @@ -491,7 +491,7 @@ export class Scope implements ScopeInterface {
protected _notifyEventProcessors(
processors: EventProcessor[],
event: Event | null,
hint?: EventHint,
hint: EventHint,
index: number = 0,
): PromiseLike<Event | null> {
return new SyncPromise<Event | null>((resolve, reject) => {
Expand Down
6 changes: 3 additions & 3 deletions packages/integrations/src/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class Debug implements Integration {
* @inheritDoc
*/
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
addGlobalEventProcessor((event: Event, hint?: EventHint) => {
addGlobalEventProcessor((event: Event, hint: EventHint) => {
const self = getCurrentHub().getIntegration(Debug);
if (self) {
if (self._options.debugger) {
Expand All @@ -49,12 +49,12 @@ export class Debug implements Integration {
consoleSandbox(() => {
if (self._options.stringify) {
console.log(JSON.stringify(event, null, 2));
if (hint) {
if (Object.keys(hint).length) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing these types to actually represent what is passed has highlighted that this would have always been logging an empty object for an empty hint.

console.log(JSON.stringify(hint, null, 2));
}
} else {
console.log(event);
if (hint) {
if (Object.keys(hint).length) {
console.log(hint);
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/integrations/src/extraerrordata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class ExtraErrorData implements Integration {
* @inheritDoc
*/
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
addGlobalEventProcessor((event: Event, hint?: EventHint) => {
addGlobalEventProcessor((event: Event, hint: EventHint) => {
const self = getCurrentHub().getIntegration(ExtraErrorData);
if (!self) {
return event;
Expand All @@ -49,8 +49,8 @@ export class ExtraErrorData implements Integration {
/**
* Attaches extracted information from the Error object to extra field in the Event
*/
public enhanceEventWithErrorData(event: Event, hint?: EventHint): Event {
if (!hint || !hint.originalException || !isError(hint.originalException)) {
public enhanceEventWithErrorData(event: Event, hint: EventHint = {}): Event {
if (!hint.originalException || !isError(hint.originalException)) {
return event;
}
const exceptionName = (hint.originalException as ExtendedError).name || hint.originalException.constructor.name;
Expand Down
4 changes: 2 additions & 2 deletions packages/integrations/test/debug.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Debug integration setup should register an event processor that', () =

const captureEventProcessor = (eventProcessor: EventProcessor) => {
const testEvent = { event_id: 'some event' };
void eventProcessor(testEvent);
void eventProcessor(testEvent, {});
expect(mockConsoleLog).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toBeCalledWith(testEvent);
};
Expand Down Expand Up @@ -55,7 +55,7 @@ describe('Debug integration setup should register an event processor that', () =

const captureEventProcessor = (eventProcessor: EventProcessor) => {
const testEvent = { event_id: 'some event' };
void eventProcessor(testEvent);
void eventProcessor(testEvent, {});
expect(mockConsoleLog).toHaveBeenCalledTimes(1);
expect(mockConsoleLog).toBeCalledWith(JSON.stringify(testEvent, null, 2));
};
Expand Down
2 changes: 1 addition & 1 deletion packages/integrations/test/offline.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ function processEventListeners(): void {
function processEvents(): void {
eventProcessors.forEach(processor => {
events.forEach(event => {
processor(event) as Event | null;
processor(event, {}) as Event | null;
});
});
}
Expand Down
6 changes: 3 additions & 3 deletions packages/node/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class LinkedErrors implements Integration {
* @inheritDoc
*/
public setupOnce(): void {
addGlobalEventProcessor(async (event: Event, hint?: EventHint) => {
addGlobalEventProcessor(async (event: Event, hint: EventHint) => {
const hub = getCurrentHub();
const self = hub.getIntegration(LinkedErrors);
const client = hub.getClient<NodeClient>();
Expand All @@ -57,8 +57,8 @@ export class LinkedErrors implements Integration {
/**
* @inheritDoc
*/
private _handler(stackParser: StackParser, event: Event, hint?: EventHint): PromiseLike<Event> {
if (!event.exception || !event.exception.values || !hint || !isInstanceOf(hint.originalException, Error)) {
private _handler(stackParser: StackParser, event: Event, hint: EventHint): PromiseLike<Event> {
if (!event.exception || !event.exception.values || !isInstanceOf(hint.originalException, Error)) {
return resolvedSyncPromise(event);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/node/test/integrations/linkederrors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('LinkedErrors', () => {
const event = {
message: 'foo',
};
return linkedErrors._handler(stackParser, event).then((result: any) => {
return linkedErrors._handler(stackParser, event, {}).then((result: any) => {
expect(spy.mock.calls.length).toEqual(0);
expect(result).toEqual(event);
});
Expand All @@ -36,7 +36,7 @@ describe('LinkedErrors', () => {
.eventFromException(one)
.then(eventFromException => {
event = eventFromException;
return linkedErrors._handler(stackParser, eventFromException);
return linkedErrors._handler(stackParser, eventFromException, {});
})
.then(result => {
expect(spy.mock.calls.length).toEqual(0);
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/eventprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ import { Event, EventHint } from './event';
*/
export interface EventProcessor {
id?: string; // This field can't be named "name" because functions already have this field natively
(event: Event, hint?: EventHint): PromiseLike<Event | null> | Event | null;
(event: Event, hint: EventHint): PromiseLike<Event | null> | Event | null;
}
2 changes: 1 addition & 1 deletion packages/types/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
* @param hint May contain additional information about the original exception.
* @returns A new event that will be sent | null.
*/
beforeSend?: (event: Event, hint?: EventHint) => PromiseLike<Event | null> | Event | null;
beforeSend?: (event: Event, hint: EventHint) => PromiseLike<Event | null> | Event | null;

/**
* A callback invoked when adding a breadcrumb, allowing to optionally modify
Expand Down