Skip to content
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

Use SyncPromise internally & Remove deprecated API & Trim Public API #1858

Merged
merged 17 commits into from
Feb 8, 2019
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
16 changes: 13 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,27 @@

## Unreleased

- **breaking** [node] fix: Events created from exception shouldnt have top-level message attribute
### Migration from v4

This major bump brings a lot of internal improvements. If you were using the SDK without any special abilities,
basically, the way we describe it in the docs, you should be fine by just updating it. This is a **breaking** release
since we removed some methods from the public API and removed some classes from the default export.

- **breaking** [node] fix: Events created from exception shouldn't have top-level message attribute
- [utils] ref: Update wrap method to hide internal sentry flags
- [utils] fix: Make internal Sentry flags non-enumerable in fill util
- [utils] ref: Move `SentryError` + `PromiseBuffer` to utils
- **breaking** [core] ref: Use `SyncPromise` internally, this reduces memory pressure by a lot.
- **breaking** [browser] ref: Removed `BrowserBackend` from default export.
- **breaking** [node] ref: Removed `BrowserBackend` from default export.

## 4.5.3

- [browser]: fix: Fix UnhandledPromise: [object Object]
- [core]: fix: Error in extraErrorData integration where event would not be send in case of non assignable object property.
- [core]: fix: Error in extraErrorData integration where event would not be send in case of non assignable object
property.
- [hub]: feat: Support non async event processors


## 4.5.2

- [utils] fix: Decycling for objects to no produce an endless loop
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export default [
interop: false,
sourcemap: true,
},
external: ['@sentry/core', '@sentry/hub', '@sentry/minimal', 'tslib'],
external: ['@sentry/core', '@sentry/hub', '@sentry/minimal'],
plugins: [
typescript({
tsconfig: 'tsconfig.build.json',
Expand Down
75 changes: 45 additions & 30 deletions packages/browser/src/backend.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { BaseBackend, Options, SentryError } from '@sentry/core';
import { BaseBackend, Options } from '@sentry/core';
import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types';
import { SentryError } from '@sentry/utils/error';
import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is';
import { supportsBeacon, supportsFetch } from '@sentry/utils/supports';
import { SyncPromise } from '@sentry/utils/syncpromise';
import { addExceptionTypeValue, eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
import { computeStackTrace } from './tracekit';
import { BeaconTransport, FetchTransport, XHRTransport } from './transports';
Expand All @@ -26,7 +28,10 @@ export interface BrowserOptions extends Options {
whitelistUrls?: Array<string | RegExp>;
}

/** The Sentry Browser SDK Backend. */
/**
* The Sentry Browser SDK Backend.
* @hidden
*/
export class BrowserBackend extends BaseBackend<BrowserOptions> {
/**
* @inheritDoc
Expand Down Expand Up @@ -69,48 +74,60 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
/**
* @inheritDoc
*/
public async eventFromException(exception: any, hint?: SentryEventHint): Promise<SentryEvent> {
let event;
public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise<SentryEvent> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fully convinced about this SyncPromise stuff tbh. It adds a level of indirection when in reality we could just use the fully procedural code here as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is correct, problem is that in node this needs to be "async" meaning that we can do file io in node. The function is defined in the interface of Backend, so even though this fully sync in browser, it isn't in node.

let event: SentryEvent;

if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) {
// If it is an ErrorEvent with `error` property, extract it to get actual Error
const ex = exception as ErrorEvent;
exception = ex.error; // tslint:disable-line:no-parameter-reassignment
const errorEvent = exception as ErrorEvent;
exception = errorEvent.error; // tslint:disable-line:no-parameter-reassignment
event = eventFromStacktrace(computeStackTrace(exception as Error));
return SyncPromise.resolve(this.buildEvent(event));
} else if (isDOMError(exception as DOMError) || isDOMException(exception as DOMException)) {
// If it is a DOMError or DOMException (which are legacy APIs, but still supported in some browsers)
// then we just extract the name and message, as they don't provide anything else
// https://developer.mozilla.org/en-US/docs/Web/API/DOMError
// https://developer.mozilla.org/en-US/docs/Web/API/DOMException
const ex = exception as DOMException;
const name = ex.name || (isDOMError(ex) ? 'DOMError' : 'DOMException');
const message = ex.message ? `${name}: ${ex.message}` : name;

event = await this.eventFromMessage(message, undefined, hint);
addExceptionTypeValue(event, message);
const domException = exception as DOMException;
const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException');
const message = domException.message ? `${name}: ${domException.message}` : name;

return this.eventFromMessage(message, Severity.Error, hint).then(messageEvent => {
addExceptionTypeValue(messageEvent, message);
return SyncPromise.resolve(this.buildEvent(messageEvent));
});
} else if (isError(exception as Error)) {
// we have a real Error object, do nothing
event = eventFromStacktrace(computeStackTrace(exception as Error));
return SyncPromise.resolve(this.buildEvent(event));
} else if (isPlainObject(exception as {}) && hint && hint.syntheticException) {
// If it is plain Object, serialize it manually and extract options
// This will allow us to group events based on top-level keys
// which is much better than creating new group when any key/value change
const ex = exception as {};
event = eventFromPlainObject(ex, hint.syntheticException);
const objectException = exception as {};
event = eventFromPlainObject(objectException, hint.syntheticException);
addExceptionTypeValue(event, 'Custom Object');
} else {
// If none of previous checks were valid, then it means that
// it's not a DOMError/DOMException
// it's not a plain Object
// it's not a valid ErrorEvent (one with an error property)
// it's not an Error
// So bail out and capture it as a simple message:
const ex = exception as string;
event = await this.eventFromMessage(ex, undefined, hint);
addExceptionTypeValue(event, `${ex}`);
return SyncPromise.resolve(this.buildEvent(event));
}

event = {
// If none of previous checks were valid, then it means that
// it's not a DOMError/DOMException
// it's not a plain Object
// it's not a valid ErrorEvent (one with an error property)
// it's not an Error
// So bail out and capture it as a simple message:
const stringException = exception as string;
return this.eventFromMessage(stringException, undefined, hint).then(messageEvent => {
addExceptionTypeValue(messageEvent, `${stringException}`);
return SyncPromise.resolve(this.buildEvent(messageEvent));
});
}

/**
* This is an internal helper function that creates an event.
*/
private buildEvent(event: SentryEvent, hint?: SentryEventHint): SentryEvent {
return {
...event,
event_id: hint && hint.event_id,
exception: {
Expand All @@ -121,18 +138,16 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
},
},
};

return event;
}

/**
* @inheritDoc
*/
public async eventFromMessage(
public eventFromMessage(
message: string,
level: Severity = Severity.Info,
hint?: SentryEventHint,
): Promise<SentryEvent> {
): SyncPromise<SentryEvent> {
const event: SentryEvent = {
event_id: hint && hint.event_id,
level,
Expand All @@ -147,6 +162,6 @@ export class BrowserBackend extends BaseBackend<BrowserOptions> {
};
}

return event;
return SyncPromise.resolve(event);
}
}
6 changes: 4 additions & 2 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { API, BaseClient, Scope, SentryError } from '@sentry/core';
import { API, BaseClient, Scope } from '@sentry/core';
import { DsnLike, SentryEvent, SentryEventHint } from '@sentry/types';
import { SentryError } from '@sentry/utils/error';
import { getGlobalObject } from '@sentry/utils/misc';
import { SyncPromise } from '@sentry/utils/syncpromise';
import { BrowserBackend, BrowserOptions } from './backend';
import { SDK_NAME, SDK_VERSION } from './version';

Expand Down Expand Up @@ -48,7 +50,7 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
/**
* @inheritDoc
*/
protected async prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): Promise<SentryEvent | null> {
protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SyncPromise<SentryEvent | null> {
event.platform = event.platform || 'javascript';
event.sdk = {
...event.sdk,
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export {
Scope,
} from '@sentry/core';

export { BrowserBackend, BrowserOptions } from './backend';
export { BrowserOptions } from './backend';
export { BrowserClient, ReportDialogOptions } from './client';
export { defaultIntegrations, forceLoad, init, lastEventId, onLoad, showReportDialog } from './sdk';
export { SDK_NAME, SDK_VERSION } from './version';
Expand Down
4 changes: 3 additions & 1 deletion packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import { breadcrumbEventHandler, keypressEventHandler, wrap } from './helpers';
const global = getGlobalObject() as Window;
let lastHref: string | undefined;

/** JSDoc */
/**
* @hidden
*/
export interface SentryWrappedXMLHttpRequest extends XMLHttpRequest {
[key: string]: any;
__sentry_xhr__?: {
Expand Down
16 changes: 12 additions & 4 deletions packages/browser/src/integrations/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,16 @@ let keypressTimeout: number | undefined;
let lastCapturedEvent: Event | undefined;
let ignoreOnError: number = 0;

/** JSDoc */
/**
* @hidden
*/
export function shouldIgnoreOnError(): boolean {
return ignoreOnError > 0;
}
/** JSDoc */

/**
* @hidden
*/
export function ignoreNextOnError(): void {
// onerror should trigger before setTimeout
ignoreOnError += 1;
Expand All @@ -28,6 +33,7 @@ export function ignoreNextOnError(): void {
*
* @param fn A function to wrap.
* @returns The wrapped function.
* @hidden
*/
export function wrap(
fn: SentryWrappedFunction,
Expand Down Expand Up @@ -79,8 +85,8 @@ export function wrap(
} catch (ex) {
ignoreNextOnError();

withScope(async scope => {
scope.addEventProcessor(async (event: SentryEvent) => {
withScope(scope => {
scope.addEventProcessor((event: SentryEvent) => {
const processedEvent = { ...event };

if (options.mechanism) {
Expand Down Expand Up @@ -141,6 +147,7 @@ export function wrap(
* Wraps addEventListener to capture UI breadcrumbs
* @param eventName the event name (e.g. "click")
* @returns wrapped breadcrumb events handler
* @hidden
*/
export function breadcrumbEventHandler(eventName: string): (event: Event) => void {
return (event: Event) => {
Expand Down Expand Up @@ -185,6 +192,7 @@ export function breadcrumbEventHandler(eventName: string): (event: Event) => voi
/**
* Wraps addEventListener to capture keypress UI events
* @returns wrapped keypress events handler
* @hidden
*/
export function keypressEventHandler(): (event: Event) => void {
// TODO: if somehow user switches keypress target before
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/linkederrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class LinkedErrors implements Integration {
* @inheritDoc
*/
public setupOnce(): void {
addGlobalEventProcessor(async (event: SentryEvent, hint?: SentryEventHint) => {
addGlobalEventProcessor((event: SentryEvent, hint?: SentryEventHint) => {
const self = getCurrentHub().getIntegration(LinkedErrors);
if (self) {
return self.handler(event, hint);
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/pluggable/ember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export class Ember implements Integration {
* @param scope The scope currently used.
*/
private addIntegrationToSdkInfo(scope: Scope): void {
scope.addEventProcessor(async (event: SentryEvent) => {
scope.addEventProcessor((event: SentryEvent) => {
if (event.sdk) {
const integrations = event.sdk.integrations || [];
event.sdk = {
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/pluggable/vue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class Vue implements Integration {
scope.setExtra(key, metadata[key]);
});

scope.addEventProcessor(async (event: SentryEvent) => {
scope.addEventProcessor((event: SentryEvent) => {
if (event.sdk) {
const integrations = event.sdk.integrations || [];
event.sdk = {
Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/useragent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class UserAgent implements Integration {
* @inheritDoc
*/
public setupOnce(): void {
addGlobalEventProcessor(async (event: SentryEvent) => {
addGlobalEventProcessor((event: SentryEvent) => {
if (getCurrentHub().getIntegration(UserAgent)) {
if (!global.navigator || !global.location) {
return event;
Expand Down
14 changes: 11 additions & 3 deletions packages/browser/src/parsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const STACKTRACE_LIMIT = 50;
/**
* This function creates an exception from an TraceKitStackTrace
* @param stacktrace TraceKitStackTrace that will be converted to an exception
* @hidden
*/
export function exceptionFromStacktrace(stacktrace: TraceKitStackTrace): SentryException {
const frames = prepareFramesForEvent(stacktrace.stack);
Expand All @@ -30,7 +31,9 @@ export function exceptionFromStacktrace(stacktrace: TraceKitStackTrace): SentryE
return exception;
}

/** JSDoc */
/**
* @hidden
*/
export function eventFromPlainObject(exception: {}, syntheticException: Error | null): SentryEvent {
const exceptionKeys = Object.keys(exception).sort();
const event: SentryEvent = {
Expand All @@ -52,7 +55,9 @@ export function eventFromPlainObject(exception: {}, syntheticException: Error |
return event;
}

/** JSDoc */
/**
* @hidden
*/
export function eventFromStacktrace(stacktrace: TraceKitStackTrace): SentryEvent {
const exception = exceptionFromStacktrace(stacktrace);

Expand All @@ -63,7 +68,9 @@ export function eventFromStacktrace(stacktrace: TraceKitStackTrace): SentryEvent
};
}

/** JSDoc */
/**
* @hidden
*/
export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[] {
if (!stack || !stack.length) {
return [];
Expand Down Expand Up @@ -104,6 +111,7 @@ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[]
* @param event The event to modify.
* @param value Value of the exception.
* @param type Type of the exception.
* @hidden
*/
export function addExceptionTypeValue(event: SentryEvent, value?: string, type?: string): void {
event.exception = event.exception || {};
Expand Down
Loading