From 200e0460150fff1355a2c4afce9497691eb169e0 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 21 Jan 2019 09:04:25 +0100 Subject: [PATCH 01/16] ref: Move PromiseBuffer and Error to utils package --- packages/browser/src/backend.ts | 3 ++- packages/browser/src/client.ts | 3 ++- packages/browser/src/integrations/helpers.ts | 4 ++-- packages/browser/src/integrations/pluggable/ember.ts | 2 +- packages/browser/src/integrations/pluggable/vue.ts | 2 +- packages/browser/src/transports/base.ts | 4 +++- packages/core/src/basebackend.ts | 2 +- packages/core/src/baseclient.ts | 2 +- packages/core/src/dsn.ts | 2 +- packages/core/src/index.ts | 2 -- packages/node/src/handlers.ts | 2 +- packages/node/src/integrations/onuncaughtexception.ts | 2 +- packages/node/src/transports/base.ts | 4 +++- packages/node/src/transports/http.ts | 2 +- packages/node/src/transports/https.ts | 2 +- packages/{core => utils}/src/error.ts | 0 packages/{core => utils}/src/promisebuffer.ts | 0 17 files changed, 21 insertions(+), 17 deletions(-) rename packages/{core => utils}/src/error.ts (100%) rename packages/{core => utils}/src/promisebuffer.ts (100%) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index 83fb1835ad11..aed7d156e296 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -1,5 +1,6 @@ -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 { addExceptionTypeValue, eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers'; diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index bca2bd077f5b..bd9e5ea291d8 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,5 +1,6 @@ -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 { BrowserBackend, BrowserOptions } from './backend'; import { SDK_NAME, SDK_VERSION } from './version'; diff --git a/packages/browser/src/integrations/helpers.ts b/packages/browser/src/integrations/helpers.ts index a2aa210c37c2..bf795dbea8f4 100644 --- a/packages/browser/src/integrations/helpers.ts +++ b/packages/browser/src/integrations/helpers.ts @@ -79,8 +79,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) { diff --git a/packages/browser/src/integrations/pluggable/ember.ts b/packages/browser/src/integrations/pluggable/ember.ts index e57ba041cc5c..3966c5c1bd53 100644 --- a/packages/browser/src/integrations/pluggable/ember.ts +++ b/packages/browser/src/integrations/pluggable/ember.ts @@ -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 = { diff --git a/packages/browser/src/integrations/pluggable/vue.ts b/packages/browser/src/integrations/pluggable/vue.ts index 1c92bbfd2c42..d9c920013fda 100644 --- a/packages/browser/src/integrations/pluggable/vue.ts +++ b/packages/browser/src/integrations/pluggable/vue.ts @@ -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 = { diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index 14c6a42de6cc..17d9f5bd2b1a 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -1,5 +1,7 @@ -import { API, PromiseBuffer, SentryError } from '@sentry/core'; +import { API } from '@sentry/core'; import { SentryResponse, Transport, TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; +import { PromiseBuffer } from '@sentry/utils/promisebuffer'; /** Base Transport class implementation */ export abstract class BaseTransport implements Transport { diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index b42439622175..4b8c0424fddc 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -1,8 +1,8 @@ import { Scope } from '@sentry/hub'; import { Breadcrumb, SentryEvent, SentryEventHint, SentryResponse, Severity, Transport } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import { logger } from '@sentry/utils/logger'; import { serialize } from '@sentry/utils/object'; -import { SentryError } from './error'; import { Backend, Options } from './interfaces'; import { NoopTransport } from './transports/noop'; diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 0daccbae7b97..1594cc720b24 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -14,12 +14,12 @@ import { forget } from '@sentry/utils/async'; import { isPrimitive } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; +import { PromiseBuffer } from '@sentry/utils/promisebuffer'; import { truncate } from '@sentry/utils/string'; import { BackendClass } from './basebackend'; import { Dsn } from './dsn'; import { IntegrationIndex, setupIntegrations } from './integration'; import { Backend, Client, Options } from './interfaces'; -import { PromiseBuffer } from './promisebuffer'; /** * Default maximum number of breadcrumbs added to an event. Can be overwritten diff --git a/packages/core/src/dsn.ts b/packages/core/src/dsn.ts index 9287b3c98ed5..364cf2187ca0 100644 --- a/packages/core/src/dsn.ts +++ b/packages/core/src/dsn.ts @@ -1,7 +1,7 @@ import { DsnComponents, DsnLike, DsnProtocol } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import { isNaN } from '@sentry/utils/is'; import { assign } from '@sentry/utils/object'; -import { SentryError } from './error'; /** Regular expression used to parse a Dsn. */ const DSN_REGEX = /^(?:(\w+):)\/\/(?:(\w+)(?::(\w+))?@)([\w\.-]+)(?::(\d+))?\/(.+)/; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index aa425970895f..020e0c5d115a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -11,8 +11,6 @@ export { API } from './api'; export { BaseClient } from './baseclient'; export { BackendClass, BaseBackend } from './basebackend'; export { Dsn } from './dsn'; -export { SentryError } from './error'; -export { PromiseBuffer } from './promisebuffer'; export { Backend, Client, LogLevel, Options } from './interfaces'; export { initAndBind, ClientClass } from './sdk'; export { NoopTransport } from './transports/noop'; diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index dd2abcd64f82..9519eb211ef5 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -230,7 +230,7 @@ export function requestHandler(options?: { local.on('error', next); local.run(() => { getCurrentHub().configureScope(scope => - scope.addEventProcessor(async (event: SentryEvent) => parseRequest(event, req, options)), + scope.addEventProcessor((event: SentryEvent) => parseRequest(event, req, options)), ); next(); }); diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index f099e5cf6d14..c28fb36470f8 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -58,7 +58,7 @@ export function makeErrorHandler( caughtFirstError = true; if (hub.getIntegration(OnUncaughtException)) { - hub.withScope(async (scope: Scope) => { + hub.withScope((scope: Scope) => { scope.setLevel(Severity.Fatal); hub.captureException(error, { originalException: error }); if (!calledFatalError) { diff --git a/packages/node/src/transports/base.ts b/packages/node/src/transports/base.ts index c1b828757922..1359903b4cad 100644 --- a/packages/node/src/transports/base.ts +++ b/packages/node/src/transports/base.ts @@ -1,5 +1,7 @@ -import { API, PromiseBuffer, SentryError } from '@sentry/core'; +import { API } from '@sentry/core'; import { SentryResponse, Status, Transport, TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; +import { PromiseBuffer } from '@sentry/utils/promisebuffer'; import * as fs from 'fs'; import * as http from 'http'; import * as https from 'https'; diff --git a/packages/node/src/transports/http.ts b/packages/node/src/transports/http.ts index 4be6ce3ca8f8..4b1f557b6f14 100644 --- a/packages/node/src/transports/http.ts +++ b/packages/node/src/transports/http.ts @@ -1,5 +1,5 @@ -import { SentryError } from '@sentry/core'; import { SentryResponse, TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import * as http from 'http'; import * as HttpsProxyAgent from 'https-proxy-agent'; import { BaseTransport } from './base'; diff --git a/packages/node/src/transports/https.ts b/packages/node/src/transports/https.ts index 35ca3353bde8..3117845b1ff1 100644 --- a/packages/node/src/transports/https.ts +++ b/packages/node/src/transports/https.ts @@ -1,5 +1,5 @@ -import { SentryError } from '@sentry/core'; import { SentryResponse, TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import * as https from 'https'; import * as HttpsProxyAgent from 'https-proxy-agent'; import { BaseTransport } from './base'; diff --git a/packages/core/src/error.ts b/packages/utils/src/error.ts similarity index 100% rename from packages/core/src/error.ts rename to packages/utils/src/error.ts diff --git a/packages/core/src/promisebuffer.ts b/packages/utils/src/promisebuffer.ts similarity index 100% rename from packages/core/src/promisebuffer.ts rename to packages/utils/src/promisebuffer.ts From 5a82777e6c13ebc9c9f0d2707ee3bbbfc6621caf Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 21 Jan 2019 12:59:03 +0100 Subject: [PATCH 02/16] ref: Remove all async api --- packages/browser/src/backend.ts | 12 +- packages/browser/src/client.ts | 2 +- .../browser/src/integrations/linkederrors.ts | 2 +- .../browser/src/integrations/useragent.ts | 2 +- packages/core/src/basebackend.ts | 14 +- packages/core/src/baseclient.ts | 127 +++++------------- packages/core/src/integrations/dedupe.ts | 2 +- .../core/src/integrations/extraerrordata.ts | 2 +- .../core/src/integrations/inboundfilters.ts | 2 +- .../core/src/integrations/pluggable/debug.ts | 2 +- .../integrations/pluggable/rewriteframes.ts | 10 +- packages/core/src/interfaces.ts | 15 +-- packages/core/test/mocks/integration.ts | 2 +- packages/hub/src/scope.ts | 15 +-- packages/node/src/backend.ts | 14 +- packages/node/src/client.ts | 2 +- .../node/src/integrations/linkederrors.ts | 14 +- .../src/integrations/pluggable/modules.ts | 2 +- .../src/integrations/pluggable/transaction.ts | 4 +- packages/node/src/parsers.ts | 34 +++-- 20 files changed, 101 insertions(+), 178 deletions(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index aed7d156e296..ec4605410443 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -70,7 +70,7 @@ export class BrowserBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromException(exception: any, hint?: SentryEventHint): Promise { + public eventFromException(exception: any, hint?: SentryEventHint): SentryEvent { let event; if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) { @@ -87,7 +87,7 @@ export class BrowserBackend extends BaseBackend { const name = ex.name || (isDOMError(ex) ? 'DOMError' : 'DOMException'); const message = ex.message ? `${name}: ${ex.message}` : name; - event = await this.eventFromMessage(message, undefined, hint); + event = this.eventFromMessage(message, undefined, hint); addExceptionTypeValue(event, message); } else if (isError(exception as Error)) { // we have a real Error object, do nothing @@ -107,7 +107,7 @@ export class BrowserBackend extends BaseBackend { // 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); + event = this.eventFromMessage(ex, undefined, hint); addExceptionTypeValue(event, `${ex}`); } @@ -129,11 +129,7 @@ export class BrowserBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromMessage( - message: string, - level: Severity = Severity.Info, - hint?: SentryEventHint, - ): Promise { + public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: SentryEventHint): SentryEvent { const event: SentryEvent = { event_id: hint && hint.event_id, level, diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index bd9e5ea291d8..a74e8d9a826b 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -49,7 +49,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - protected async prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): Promise { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { event.platform = event.platform || 'javascript'; event.sdk = { ...event.sdk, diff --git a/packages/browser/src/integrations/linkederrors.ts b/packages/browser/src/integrations/linkederrors.ts index 6b7538ebcfca..9433760c9928 100644 --- a/packages/browser/src/integrations/linkederrors.ts +++ b/packages/browser/src/integrations/linkederrors.ts @@ -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); diff --git a/packages/browser/src/integrations/useragent.ts b/packages/browser/src/integrations/useragent.ts index bd6611e53764..8f8eea74ea53 100644 --- a/packages/browser/src/integrations/useragent.ts +++ b/packages/browser/src/integrations/useragent.ts @@ -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; diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index 4b8c0424fddc..6785e47da19d 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -1,5 +1,5 @@ import { Scope } from '@sentry/hub'; -import { Breadcrumb, SentryEvent, SentryEventHint, SentryResponse, Severity, Transport } from '@sentry/types'; +import { Breadcrumb, SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types'; import { SentryError } from '@sentry/utils/error'; import { logger } from '@sentry/utils/logger'; import { serialize } from '@sentry/utils/object'; @@ -40,29 +40,31 @@ export abstract class BaseBackend implements Backend { /** * @inheritDoc */ - public async eventFromException(_exception: any, _hint?: SentryEventHint): Promise { + public eventFromException(_exception: any, _hint?: SentryEventHint): SentryEvent { throw new SentryError('Backend has to implement `eventFromException` method'); } /** * @inheritDoc */ - public async eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): Promise { + public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): SentryEvent { throw new SentryError('Backend has to implement `eventFromMessage` method'); } /** * @inheritDoc */ - public async sendEvent(event: SentryEvent): Promise { + public sendEvent(event: SentryEvent): void { + // TODO: Floating promise // TODO: Remove with v5 // tslint:disable-next-line if (this.transport.captureEvent) { // tslint:disable-next-line - return this.transport.captureEvent(event); + this.transport.captureEvent(event); } // -------------------- - return this.transport.sendEvent(serialize(event)); + // tslint:disable-next-line + this.transport.sendEvent(serialize(event)); } /** diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 1594cc720b24..3753ee5cc6ed 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -6,15 +6,11 @@ import { SentryBreadcrumbHint, SentryEvent, SentryEventHint, - SentryResponse, Severity, - Status, } from '@sentry/types'; -import { forget } from '@sentry/utils/async'; import { isPrimitive } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; -import { PromiseBuffer } from '@sentry/utils/promisebuffer'; import { truncate } from '@sentry/utils/string'; import { BackendClass } from './basebackend'; import { Dsn } from './dsn'; @@ -96,9 +92,6 @@ export abstract class BaseClient implement /** Array of used integrations. */ private readonly integrations: IntegrationIndex; - /** A simple buffer holding all requests. */ - protected readonly buffer: PromiseBuffer = new PromiseBuffer(); - /** * Initializes this client instance. * @@ -136,46 +129,27 @@ export abstract class BaseClient implement /** * @inheritDoc */ - public async captureException(exception: any, hint?: SentryEventHint, scope?: Scope): Promise { - return this.buffer.add( - (async () => { - const event = await this.getBackend().eventFromException(exception, hint); - return this.captureEvent(event, hint, scope); - })(), - ); + public captureException(exception: any, hint?: SentryEventHint, scope?: Scope): void { + const event = this.getBackend().eventFromException(exception, hint); + this.captureEvent(event, hint, scope); } /** * @inheritDoc */ - public async captureMessage( - message: string, - level?: Severity, - hint?: SentryEventHint, - scope?: Scope, - ): Promise { - return this.buffer.add( - (async () => { - const event = isPrimitive(message) - ? await this.getBackend().eventFromMessage(`${message}`, level, hint) - : await this.getBackend().eventFromException(message, hint); - - return this.captureEvent(event, hint, scope); - })(), - ); + public captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): void { + const event = isPrimitive(message) + ? this.getBackend().eventFromMessage(`${message}`, level, hint) + : this.getBackend().eventFromException(message, hint); + + this.captureEvent(event, hint, scope); } /** * @inheritDoc */ - public async captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): Promise { - // Adding this here is technically not correct since if you call captureMessage/captureException it's already - // buffered. But since we not really need the count and we only need to know if the buffer is full or not, - // This is fine... - return this.buffer.add( - (async () => - this.processEvent(event, async finalEvent => this.getBackend().sendEvent(finalEvent), hint, scope))(), - ); + public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void { + this.processEvent(event, hint, scope); } /** @@ -241,7 +215,7 @@ export abstract class BaseClient implement * @param scope A scope containing event metadata. * @returns A new event with more information. */ - protected async prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): Promise { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { const { environment, maxBreadcrumbs = DEFAULT_BREADCRUMBS, release, dist } = this.getOptions(); const prepared = { ...event }; @@ -301,16 +275,10 @@ export abstract class BaseClient implement * @param hint May contain additional informartion about the original exception. * @returns A Promise that resolves with the event status. */ - protected async processEvent( - event: SentryEvent, - send: (finalEvent: SentryEvent) => Promise, - hint?: SentryEventHint, - scope?: Scope, - ): Promise { + protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void { if (!this.isEnabled()) { - return { - status: Status.Skipped, - }; + logger.log('SDK not enabled, will not send event.'); + return; } const { beforeSend, sampleRate } = this.getOptions(); @@ -318,16 +286,14 @@ export abstract class BaseClient implement // 1.0 === 100% events are sent // 0.0 === 0% events are sent if (typeof sampleRate === 'number' && Math.random() > sampleRate) { - return { - status: Status.Skipped, - }; + logger.log('This event has been sampled, will not send event.'); + return; } - const prepared = await this.prepareEvent(event, scope, hint); + const prepared = this.prepareEvent(event, scope, hint); if (prepared === null) { - return { - status: Status.Skipped, - }; + logger.log('An event processor returned null, will not send event.'); + return; } let finalEvent: SentryEvent | null = prepared; @@ -335,51 +301,33 @@ export abstract class BaseClient implement try { const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; if (!isInternalException && beforeSend) { - finalEvent = await beforeSend(prepared, hint); + finalEvent = beforeSend(prepared, hint); if ((typeof finalEvent as any) === 'undefined') { - logger.error('`beforeSend` method has to return `null` or a valid event'); + logger.error('`beforeSend` method has to return `null` or a valid event.'); } } } catch (exception) { - forget( - this.captureException(exception, { - data: { - __sentry__: true, - }, - originalException: exception as Error, - }), - ); - - return { - reason: 'Event processing in beforeSend method threw an exception', - status: Status.Invalid, - }; + this.captureException(exception, { + data: { + __sentry__: true, + }, + originalException: exception as Error, + }); + logger.error('`beforeSend` throw an error, will not send event.'); + return; } if (finalEvent === null) { - return { - reason: 'Event dropped due to being discarded by beforeSend method', - status: Status.Skipped, - }; + logger.log('`beforeSend` returned `null`, will not send event.'); + return; } try { - const response = await send(finalEvent); - response.event = finalEvent; - - if (response.status === Status.RateLimit) { - // TODO: Handle rate limits and maintain a queue. For now, we require SDK - // implementors to override this method and handle it themselves. - } - return response; + this.getBackend().sendEvent(finalEvent); } catch (error) { // We have a catch here since the transport can reject the request internally. // If we do not catch this here, we will run into an endless loop. logger.error(`${error}`); - return { - reason: `${error}`, - status: Status.Failed, - }; } } @@ -387,12 +335,9 @@ export abstract class BaseClient implement * @inheritDoc */ public async close(timeout?: number): Promise { - return (await Promise.all([ - this.getBackend() - .getTransport() - .close(timeout), - this.buffer.drain(timeout), - ])).reduce((prev, current) => prev && current); + return this.getBackend() + .getTransport() + .close(timeout); } /** diff --git a/packages/core/src/integrations/dedupe.ts b/packages/core/src/integrations/dedupe.ts index fafa4ca8ce70..34b492c184dc 100644 --- a/packages/core/src/integrations/dedupe.ts +++ b/packages/core/src/integrations/dedupe.ts @@ -24,7 +24,7 @@ export class Dedupe implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async (currentEvent: SentryEvent) => { + addGlobalEventProcessor((currentEvent: SentryEvent) => { const self = getCurrentHub().getIntegration(Dedupe); if (self) { // Juuust in case something goes wrong diff --git a/packages/core/src/integrations/extraerrordata.ts b/packages/core/src/integrations/extraerrordata.ts index 263cd96f2a03..650adeffa2a2 100644 --- a/packages/core/src/integrations/extraerrordata.ts +++ b/packages/core/src/integrations/extraerrordata.ts @@ -27,7 +27,7 @@ export class ExtraErrorData implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async (event: SentryEvent, hint?: SentryEventHint) => { + addGlobalEventProcessor((event: SentryEvent, hint?: SentryEventHint) => { const self = getCurrentHub().getIntegration(ExtraErrorData); if (!self) { return event; diff --git a/packages/core/src/integrations/inboundfilters.ts b/packages/core/src/integrations/inboundfilters.ts index e1cfff837b1a..36138c224afa 100644 --- a/packages/core/src/integrations/inboundfilters.ts +++ b/packages/core/src/integrations/inboundfilters.ts @@ -35,7 +35,7 @@ export class InboundFilters implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async (event: SentryEvent) => { + addGlobalEventProcessor((event: SentryEvent) => { const hub = getCurrentHub(); if (!hub) { return event; diff --git a/packages/core/src/integrations/pluggable/debug.ts b/packages/core/src/integrations/pluggable/debug.ts index 424969ac7c84..4b440dd16e08 100644 --- a/packages/core/src/integrations/pluggable/debug.ts +++ b/packages/core/src/integrations/pluggable/debug.ts @@ -37,7 +37,7 @@ export class Debug implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async (event: SentryEvent, hint?: SentryEventHint) => { + addGlobalEventProcessor((event: SentryEvent, hint?: SentryEventHint) => { const self = getCurrentHub().getIntegration(Debug); if (self) { // tslint:disable:no-console diff --git a/packages/core/src/integrations/pluggable/rewriteframes.ts b/packages/core/src/integrations/pluggable/rewriteframes.ts index 976df048048a..c803d99ed5ea 100644 --- a/packages/core/src/integrations/pluggable/rewriteframes.ts +++ b/packages/core/src/integrations/pluggable/rewriteframes.ts @@ -2,7 +2,7 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub'; import { Integration, SentryEvent, StackFrame } from '@sentry/types'; import { basename, relative } from '@sentry/utils/path'; -type StackFrameIteratee = (frame: StackFrame) => Promise; +type StackFrameIteratee = (frame: StackFrame) => StackFrame; /** Rewrite event frames paths */ export class RewriteFrames implements Integration { @@ -24,7 +24,7 @@ export class RewriteFrames implements Integration { /** * @inheritDoc */ - private readonly iteratee: StackFrameIteratee = async (frame: StackFrame) => { + private readonly iteratee: StackFrameIteratee = (frame: StackFrame) => { if (frame.filename && frame.filename.startsWith('/')) { const base = this.root ? relative(this.root, frame.filename) : basename(frame.filename); frame.filename = `app:///${base}`; @@ -48,7 +48,7 @@ export class RewriteFrames implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async event => { + addGlobalEventProcessor(event => { const self = getCurrentHub().getIntegration(RewriteFrames); if (self) { return self.process(event); @@ -58,12 +58,12 @@ export class RewriteFrames implements Integration { } /** JSDoc */ - public async process(event: SentryEvent): Promise { + public process(event: SentryEvent): SentryEvent { const frames = this.getFramesFromEvent(event); if (frames) { for (const i in frames) { // tslint:disable-next-line - frames[i] = await this.iteratee(frames[i]); + frames[i] = this.iteratee(frames[i]); } } return event; diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 45f1540dfa6f..9139d6b5e02e 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -6,7 +6,6 @@ import { SentryBreadcrumbHint, SentryEvent, SentryEventHint, - SentryResponse, Severity, Transport, TransportClass, @@ -112,7 +111,7 @@ export interface Options { * @param hint May contain additional information about the original exception. * @returns A new event that will be sent | null. */ - beforeSend?(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null | Promise; + beforeSend?(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null; /** * A callback invoked when adding a breadcrumb, allowing to optionally modify @@ -164,7 +163,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ - captureException(exception: any, hint?: SentryEventHint, scope?: Scope): Promise; + captureException(exception: any, hint?: SentryEventHint, scope?: Scope): void; /** * Captures a message event and sends it to Sentry. @@ -175,7 +174,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ - captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): Promise; + captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): void; /** * Captures a manually created event and sends it to Sentry. @@ -185,7 +184,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns SentryResponse status and event */ - captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): Promise; + captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void; /** * Records a new breadcrumb which will be attached to future events. @@ -244,13 +243,13 @@ export interface Backend { install?(): boolean; /** Creates a {@link SentryEvent} from an exception. */ - eventFromException(exception: any, hint?: SentryEventHint): Promise; + eventFromException(exception: any, hint?: SentryEventHint): SentryEvent; /** Creates a {@link SentryEvent} from a plain message. */ - eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): Promise; + eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): SentryEvent; /** Submits the event to Sentry */ - sendEvent(event: SentryEvent): Promise; + sendEvent(event: SentryEvent): void; /** * Receives a breadcrumb and stores it in a platform-dependent way. diff --git a/packages/core/test/mocks/integration.ts b/packages/core/test/mocks/integration.ts index 4f89a04fd78f..a12e7a107e69 100644 --- a/packages/core/test/mocks/integration.ts +++ b/packages/core/test/mocks/integration.ts @@ -8,7 +8,7 @@ export class TestIntegration implements Integration { public setupOnce(): void { configureScope(scope => { - scope.addEventProcessor(async (event: SentryEvent) => { + scope.addEventProcessor((event: SentryEvent) => { if (!getCurrentHub().getIntegration(TestIntegration)) { return event; } diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 2f30f16a182d..d30a016f6067 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -2,10 +2,7 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, User } from '@sentr import { getGlobalObject } from '@sentry/utils/misc'; import { assign, safeNormalize } from '@sentry/utils/object'; -export type EventProcessor = ( - event: SentryEvent, - hint?: SentryEventHint, -) => Promise | SentryEvent | null; +export type EventProcessor = (event: SentryEvent, hint?: SentryEventHint) => SentryEvent | null; /** * Holds additional event information. {@link Scope.applyToEvent} will be @@ -68,11 +65,11 @@ export class Scope { /** * This will be called after {@link applyToEvent} is finished. */ - protected async notifyEventProcessors(event: SentryEvent, hint?: SentryEventHint): Promise { + protected notifyEventProcessors(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null { let processedEvent: SentryEvent | null = event; for (const processor of [...getGlobalEventProcessors(), ...this.eventProcessors]) { try { - processedEvent = await processor({ ...processedEvent }, hint); + processedEvent = processor({ ...processedEvent }, hint); if (processedEvent === null) { return null; } @@ -209,11 +206,7 @@ export class Scope { * @param hint May contain additional informartion about the original exception. * @param maxBreadcrumbs number of max breadcrumbs to merged into event. */ - public async applyToEvent( - event: SentryEvent, - hint?: SentryEventHint, - maxBreadcrumbs?: number, - ): Promise { + public applyToEvent(event: SentryEvent, hint?: SentryEventHint, maxBreadcrumbs?: number): SentryEvent | null { if (this.extra && Object.keys(this.extra).length) { event.extra = { ...this.extra, ...event.extra }; } diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index 5423686e1aa5..b8e7f56ea7fd 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -69,7 +69,7 @@ export class NodeBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromException(exception: any, hint?: SentryEventHint): Promise { + public eventFromException(exception: any, hint?: SentryEventHint): SentryEvent { let ex: any = exception; if (!isError(exception)) { @@ -97,7 +97,7 @@ export class NodeBackend extends BaseBackend { } } - const event: SentryEvent = await parseError(ex as Error, this.options); + const event: SentryEvent = parseError(ex as Error, this.options); return { ...event, @@ -108,11 +108,7 @@ export class NodeBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromMessage( - message: string, - level: Severity = Severity.Info, - hint?: SentryEventHint, - ): Promise { + public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: SentryEventHint): SentryEvent { const event: SentryEvent = { event_id: hint && hint.event_id, level, @@ -120,8 +116,8 @@ export class NodeBackend extends BaseBackend { }; if (this.options.attachStacktrace && hint && hint.syntheticException) { - const stack = hint.syntheticException ? await extractStackFromError(hint.syntheticException) : []; - const frames = await parseStack(stack, this.options); + const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; + const frames = parseStack(stack, this.options); event.stacktrace = { frames: prepareFramesForEvent(frames), }; diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index e76ea420fd25..8a4ccdae99bf 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -21,7 +21,7 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected async prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): Promise { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { event.platform = event.platform || 'node'; event.sdk = { ...event.sdk, diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index 727bd0f29454..7685c8b4a8c0 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -45,7 +45,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); @@ -57,11 +57,11 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - public async handler(event: SentryEvent, hint?: SentryEventHint): Promise { + public handler(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null { if (!event.exception || !event.exception.values || !hint || !(hint.originalException instanceof Error)) { return event; } - const linkedErrors = await this.walkErrorTree(hint.originalException, this.key); + const linkedErrors = this.walkErrorTree(hint.originalException, this.key); event.exception.values = [...linkedErrors, ...event.exception.values]; return event; } @@ -69,15 +69,11 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - public async walkErrorTree( - error: ExtendedError, - key: string, - stack: SentryException[] = [], - ): Promise { + public walkErrorTree(error: ExtendedError, key: string, stack: SentryException[] = []): SentryException[] { if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) { return stack; } - const exception = await getExceptionFromError(error[key]); + const exception = getExceptionFromError(error[key]); return this.walkErrorTree(error[key], key, [exception, ...stack]); } } diff --git a/packages/node/src/integrations/pluggable/modules.ts b/packages/node/src/integrations/pluggable/modules.ts index a689a0cbcf70..8d1a508c116f 100644 --- a/packages/node/src/integrations/pluggable/modules.ts +++ b/packages/node/src/integrations/pluggable/modules.ts @@ -19,7 +19,7 @@ export class Modules implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async event => { + addGlobalEventProcessor(event => { if (!getCurrentHub().getIntegration(Modules)) { return event; } diff --git a/packages/node/src/integrations/pluggable/transaction.ts b/packages/node/src/integrations/pluggable/transaction.ts index eba0c315c2e3..1944c5b1e12c 100644 --- a/packages/node/src/integrations/pluggable/transaction.ts +++ b/packages/node/src/integrations/pluggable/transaction.ts @@ -16,7 +16,7 @@ export class Transaction implements Integration { * @inheritDoc */ public setupOnce(): void { - addGlobalEventProcessor(async event => { + addGlobalEventProcessor(event => { const self = getCurrentHub().getIntegration(Transaction); if (self) { return self.process(event); @@ -28,7 +28,7 @@ export class Transaction implements Integration { /** * @inheritDoc */ - public async process(event: SentryEvent): Promise { + public process(event: SentryEvent): SentryEvent { const frames = this.getFramesFromEvent(event); // use for loop so we don't have to reverse whole frames array diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index 9dd135bef2d2..313f2e08da33 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,7 +1,7 @@ import { SentryEvent, SentryException, StackFrame } from '@sentry/types'; -import { readFileAsync } from '@sentry/utils/fs'; import { basename, dirname } from '@sentry/utils/path'; import { snipLine } from '@sentry/utils/string'; +import { readFileSync } from 'fs'; import { LRUMap } from 'lru_map'; import * as stacktrace from 'stack-trace'; import { NodeOptions } from './backend'; @@ -73,11 +73,11 @@ function getModule(filename: string, base?: string): string { * * @param filenames Array of filepaths to read content from. */ -async function readSourceFiles( +function readSourceFiles( filenames: string[], -): Promise<{ +): { [key: string]: string; -}> { +} { // we're relying on filenames being de-duped already if (filenames.length === 0) { return {}; @@ -106,7 +106,7 @@ async function readSourceFiles( let content = null; try { - content = await readFileAsync(filename); + content = readFileSync(filename, 'utf8'); sourceFiles[filename] = content; } catch (_) { // unsure what to add here as the file is unreadable @@ -121,7 +121,7 @@ async function readSourceFiles( } /** JSDoc */ -export async function extractStackFromError(error: Error): Promise { +export function extractStackFromError(error: Error): stacktrace.StackFrame[] { const stack = stacktrace.parse(error); if (!stack) { return []; @@ -130,7 +130,7 @@ export async function extractStackFromError(error: Error): Promise { +export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): StackFrame[] { const filesToRead: string[] = []; const linesOfContext = @@ -175,7 +175,7 @@ export async function parseStack(stack: stacktrace.StackFrame[], options?: NodeO } try { - return await addPrePostContext(filesToRead, frames, linesOfContext); + return addPrePostContext(filesToRead, frames, linesOfContext); } catch (_) { // This happens in electron for example where we are not able to read files from asar. // So it's fine, we recover be just returning all frames without pre/post context. @@ -189,12 +189,8 @@ export async function parseStack(stack: stacktrace.StackFrame[], options?: NodeO * @param filesToRead string[] of filepaths * @param frames StackFrame[] containg all frames */ -async function addPrePostContext( - filesToRead: string[], - frames: StackFrame[], - linesOfContext: number, -): Promise { - const sourceFiles = await readSourceFiles(filesToRead); +function addPrePostContext(filesToRead: string[], frames: StackFrame[], linesOfContext: number): StackFrame[] { + const sourceFiles = readSourceFiles(filesToRead); return frames.map(frame => { if (frame.filename && sourceFiles[frame.filename]) { try { @@ -219,10 +215,10 @@ async function addPrePostContext( } /** JSDoc */ -export async function getExceptionFromError(error: Error, options?: NodeOptions): Promise { +export function getExceptionFromError(error: Error, options?: NodeOptions): SentryException { const name = error.name || error.constructor.name; - const stack = await extractStackFromError(error); - const frames = await parseStack(stack, options); + const stack = extractStackFromError(error); + const frames = parseStack(stack, options); return { stacktrace: { @@ -234,9 +230,9 @@ export async function getExceptionFromError(error: Error, options?: NodeOptions) } /** JSDoc */ -export async function parseError(error: ExtendedError, options?: NodeOptions): Promise { +export function parseError(error: ExtendedError, options?: NodeOptions): SentryEvent { const name = error.name || error.constructor.name; - const exception = await getExceptionFromError(error, options); + const exception = getExceptionFromError(error, options); return { exception: { values: [exception], From 8cdee6779d67be324fd452ba87623e71cee133b0 Mon Sep 17 00:00:00 2001 From: HazA Date: Wed, 23 Jan 2019 22:44:22 +0100 Subject: [PATCH 03/16] ref: Remove invokeClientAsync function --- packages/hub/src/hub.ts | 22 +++------------------ packages/node/test/transports/http.test.ts | 2 +- packages/node/test/transports/https.test.ts | 2 +- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index eaafd54d9657..f68238221f38 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -66,22 +66,6 @@ export class Hub { } } - /** - * Internal helper function to call an async method on the top client if it - * exists. - * - * @param method The method to call on the client/client. - * @param args Arguments to pass to the client/frontend. - */ - private invokeClientAsync(method: string, ...args: any[]): void { - const top = this.getStackTop(); - if (top && top.client && top.client[method]) { - top.client[method](...args, top.scope).catch((err: any) => { - logger.error(err); - }); - } - } - /** * Checks if this hub's version is older than the given version. * @@ -196,7 +180,7 @@ export class Hub { */ public captureException(exception: any, hint?: SentryEventHint): string { const eventId = (this._lastEventId = uuid4()); - this.invokeClientAsync('captureException', exception, { + this.invokeClient('captureException', exception, { ...hint, event_id: eventId, }); @@ -213,7 +197,7 @@ export class Hub { */ public captureMessage(message: string, level?: Severity, hint?: SentryEventHint): string { const eventId = (this._lastEventId = uuid4()); - this.invokeClientAsync('captureMessage', message, level, { + this.invokeClient('captureMessage', message, level, { ...hint, event_id: eventId, }); @@ -228,7 +212,7 @@ export class Hub { */ public captureEvent(event: SentryEvent, hint?: SentryEventHint): string { const eventId = (this._lastEventId = uuid4()); - this.invokeClientAsync('captureEvent', event, { + this.invokeClient('captureEvent', event, { ...hint, event_id: eventId, }); diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts index a38952233968..be99f0117d69 100644 --- a/packages/node/test/transports/http.test.ts +++ b/packages/node/test/transports/http.test.ts @@ -1,5 +1,5 @@ -import { SentryError } from '@sentry/core'; import { TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import * as HttpsProxyAgent from 'https-proxy-agent'; import { HTTPTransport } from '../../src/transports/http'; diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts index 23ec9063ac55..18d759a926ab 100644 --- a/packages/node/test/transports/https.test.ts +++ b/packages/node/test/transports/https.test.ts @@ -1,5 +1,5 @@ -import { SentryError } from '@sentry/core'; import { TransportOptions } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import * as HttpsProxyAgent from 'https-proxy-agent'; import { HTTPSTransport } from '../../src/transports/https'; From 3ada698e1386328e60580748b2cad34b2605da8f Mon Sep 17 00:00:00 2001 From: HazA Date: Fri, 25 Jan 2019 09:27:39 +0100 Subject: [PATCH 04/16] feat: Finish Node SDK --- packages/core/src/basebackend.ts | 14 +- packages/core/src/baseclient.ts | 94 +++++---- packages/core/src/interfaces.ts | 17 +- packages/hub/src/scope.ts | 54 +++-- packages/node/src/backend.ts | 45 +++-- packages/node/src/client.ts | 3 +- .../node/src/integrations/linkederrors.ts | 34 +++- packages/node/src/parsers.ts | 187 ++++++++++-------- packages/utils/src/is.ts | 8 + packages/utils/src/quickpromise.ts | 58 ++++++ packages/utils/test/quickpromise.test.ts | 89 +++++++++ 11 files changed, 417 insertions(+), 186 deletions(-) create mode 100644 packages/utils/src/quickpromise.ts create mode 100644 packages/utils/test/quickpromise.test.ts diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index 6785e47da19d..f3c85d36fa93 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -3,6 +3,7 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, Transport } from '@ import { SentryError } from '@sentry/utils/error'; import { logger } from '@sentry/utils/logger'; import { serialize } from '@sentry/utils/object'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { Backend, Options } from './interfaces'; import { NoopTransport } from './transports/noop'; @@ -40,14 +41,14 @@ export abstract class BaseBackend implements Backend { /** * @inheritDoc */ - public eventFromException(_exception: any, _hint?: SentryEventHint): SentryEvent { + public eventFromException(_exception: any, _hint?: SentryEventHint): QuickPromise { throw new SentryError('Backend has to implement `eventFromException` method'); } /** * @inheritDoc */ - public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): SentryEvent { + public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): QuickPromise { throw new SentryError('Backend has to implement `eventFromMessage` method'); } @@ -55,15 +56,6 @@ export abstract class BaseBackend implements Backend { * @inheritDoc */ public sendEvent(event: SentryEvent): void { - // TODO: Floating promise - // TODO: Remove with v5 - // tslint:disable-next-line - if (this.transport.captureEvent) { - // tslint:disable-next-line - this.transport.captureEvent(event); - } - // -------------------- - // tslint:disable-next-line this.transport.sendEvent(serialize(event)); } diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3753ee5cc6ed..92d4494a2f6a 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -11,6 +11,7 @@ import { import { isPrimitive } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { truncate } from '@sentry/utils/string'; import { BackendClass } from './basebackend'; import { Dsn } from './dsn'; @@ -129,27 +130,35 @@ export abstract class BaseClient implement /** * @inheritDoc */ - public captureException(exception: any, hint?: SentryEventHint, scope?: Scope): void { - const event = this.getBackend().eventFromException(exception, hint); - this.captureEvent(event, hint, scope); + public captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string { + const promisedEvent = this.getBackend().eventFromException(exception, hint); + promisedEvent.then(event => { + this.captureEvent(event, hint, scope); + }); + return (hint && hint.event_id) || ''; } /** * @inheritDoc */ - public captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): void { - const event = isPrimitive(message) + public captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string { + const promisedEvent = isPrimitive(message) ? this.getBackend().eventFromMessage(`${message}`, level, hint) : this.getBackend().eventFromException(message, hint); - this.captureEvent(event, hint, scope); + promisedEvent.then(event => { + this.captureEvent(event, hint, scope); + }); + + return (hint && hint.event_id) || ''; } /** * @inheritDoc */ - public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void { + public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string { this.processEvent(event, hint, scope); + return (hint && hint.event_id) || ''; } /** @@ -215,7 +224,7 @@ export abstract class BaseClient implement * @param scope A scope containing event metadata. * @returns A new event with more information. */ - protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): QuickPromise { const { environment, maxBreadcrumbs = DEFAULT_BREADCRUMBS, release, dist } = this.getOptions(); const prepared = { ...event }; @@ -254,7 +263,7 @@ export abstract class BaseClient implement return scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS)); } - return prepared; + return QuickPromise.resolve(prepared); } /** @@ -290,45 +299,46 @@ export abstract class BaseClient implement return; } - const prepared = this.prepareEvent(event, scope, hint); - if (prepared === null) { - logger.log('An event processor returned null, will not send event.'); - return; - } + this.prepareEvent(event, scope, hint).then(prepared => { + if (prepared === null) { + logger.log('An event processor returned null, will not send event.'); + return; + } - let finalEvent: SentryEvent | null = prepared; + let finalEvent: SentryEvent | null = prepared; - try { - const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; - if (!isInternalException && beforeSend) { - finalEvent = beforeSend(prepared, hint); - if ((typeof finalEvent as any) === 'undefined') { - logger.error('`beforeSend` method has to return `null` or a valid event.'); + try { + const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; + if (!isInternalException && beforeSend) { + finalEvent = beforeSend(prepared, hint); + if ((typeof finalEvent as any) === 'undefined') { + logger.error('`beforeSend` method has to return `null` or a valid event.'); + } } + } catch (exception) { + this.captureException(exception, { + data: { + __sentry__: true, + }, + originalException: exception as Error, + }); + logger.error('`beforeSend` throw an error, will not send event.'); + return; } - } catch (exception) { - this.captureException(exception, { - data: { - __sentry__: true, - }, - originalException: exception as Error, - }); - logger.error('`beforeSend` throw an error, will not send event.'); - return; - } - if (finalEvent === null) { - logger.log('`beforeSend` returned `null`, will not send event.'); - return; - } + if (finalEvent === null) { + logger.log('`beforeSend` returned `null`, will not send event.'); + return; + } - try { - this.getBackend().sendEvent(finalEvent); - } catch (error) { - // We have a catch here since the transport can reject the request internally. - // If we do not catch this here, we will run into an endless loop. - logger.error(`${error}`); - } + try { + this.getBackend().sendEvent(finalEvent); + } catch (error) { + // We have a catch here since the transport can reject the request internally. + // If we do not catch this here, we will run into an endless loop. + logger.error(`${error}`); + } + }); } /** diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 9139d6b5e02e..1c951728041f 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -11,6 +11,7 @@ import { TransportClass, TransportOptions, } from '@sentry/types'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { Dsn } from './dsn'; /** Console logging verbosity for the SDK. */ @@ -161,9 +162,9 @@ export interface Client { * @param exception An exception-like object. * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. - * @returns SentryResponse status and event + * @returns The event id */ - captureException(exception: any, hint?: SentryEventHint, scope?: Scope): void; + captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string; /** * Captures a message event and sends it to Sentry. @@ -172,9 +173,9 @@ export interface Client { * @param level Define the level of the message. * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. - * @returns SentryResponse status and event + * @returns The event id */ - captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): void; + captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string; /** * Captures a manually created event and sends it to Sentry. @@ -182,9 +183,9 @@ export interface Client { * @param event The event to send to Sentry. * @param hint May contain additional information about the original exception. * @param scope An optional scope containing event metadata. - * @returns SentryResponse status and event + * @returns The event id */ - captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void; + captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string; /** * Records a new breadcrumb which will be attached to future events. @@ -243,10 +244,10 @@ export interface Backend { install?(): boolean; /** Creates a {@link SentryEvent} from an exception. */ - eventFromException(exception: any, hint?: SentryEventHint): SentryEvent; + eventFromException(exception: any, hint?: SentryEventHint): QuickPromise; /** Creates a {@link SentryEvent} from a plain message. */ - eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): SentryEvent; + eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): QuickPromise; /** Submits the event to Sentry */ sendEvent(event: SentryEvent): void; diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index d30a016f6067..7d94e677a105 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,8 +1,13 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, User } from '@sentry/types'; +import { isPromise } from '@sentry/utils/is'; import { getGlobalObject } from '@sentry/utils/misc'; import { assign, safeNormalize } from '@sentry/utils/object'; +import { QuickPromise } from '@sentry/utils/quickpromise'; -export type EventProcessor = (event: SentryEvent, hint?: SentryEventHint) => SentryEvent | null; +export type EventProcessor = ( + event: SentryEvent, + hint?: SentryEventHint, +) => Promise | SentryEvent | null; /** * Holds additional event information. {@link Scope.applyToEvent} will be @@ -65,19 +70,36 @@ export class Scope { /** * This will be called after {@link applyToEvent} is finished. */ - protected notifyEventProcessors(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null { - let processedEvent: SentryEvent | null = event; - for (const processor of [...getGlobalEventProcessors(), ...this.eventProcessors]) { - try { - processedEvent = processor({ ...processedEvent }, hint); - if (processedEvent === null) { - return null; + protected notifyEventProcessors( + processors: EventProcessor[], + event: SentryEvent | null, + hint?: SentryEventHint, + index: number = 0, + callback?: (result: SentryEvent | null) => void, + ): QuickPromise { + return new QuickPromise(resolve => { + if (event === null) { + resolve(null); + } else { + const processor = processors[index]; + if (isPromise(processor)) { + (processor({ ...event }, hint) as Promise).then((processedEvent: SentryEvent | null) => { + if (index === processors.length - 1 && callback) { + callback(processedEvent); + } else { + this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve); + } + }); + } else { + const processedEvent = processor({ ...event }, hint) as SentryEvent | null; + if (index === processors.length - 1 && callback) { + callback(processedEvent); + } else { + this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve); + } } - } catch (e) { - continue; } - } - return processedEvent; + }); } /** @@ -206,7 +228,11 @@ export class Scope { * @param hint May contain additional informartion about the original exception. * @param maxBreadcrumbs number of max breadcrumbs to merged into event. */ - public applyToEvent(event: SentryEvent, hint?: SentryEventHint, maxBreadcrumbs?: number): SentryEvent | null { + public applyToEvent( + event: SentryEvent, + hint?: SentryEventHint, + maxBreadcrumbs?: number, + ): QuickPromise { if (this.extra && Object.keys(this.extra).length) { event.extra = { ...this.extra, ...event.extra }; } @@ -230,7 +256,7 @@ export class Scope { : this.breadcrumbs; } - return this.notifyEventProcessors(event, hint); + return this.notifyEventProcessors([...getGlobalEventProcessors(), ...this.eventProcessors], event, hint); } } diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index b8e7f56ea7fd..c400b40cd181 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -2,6 +2,7 @@ import { BaseBackend, Dsn, getCurrentHub, Options } from '@sentry/core'; import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types'; import { isError, isPlainObject } from '@sentry/utils/is'; import { limitObjectDepthToSize, serializeKeysToEventMessage } from '@sentry/utils/object'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { createHash } from 'crypto'; import { extractStackFromError, parseError, parseStack, prepareFramesForEvent } from './parsers'; import { HTTPSTransport, HTTPTransport } from './transports'; @@ -69,7 +70,7 @@ export class NodeBackend extends BaseBackend { /** * @inheritDoc */ - public eventFromException(exception: any, hint?: SentryEventHint): SentryEvent { + public eventFromException(exception: any, hint?: SentryEventHint): QuickPromise { let ex: any = exception; if (!isError(exception)) { @@ -97,32 +98,42 @@ export class NodeBackend extends BaseBackend { } } - const event: SentryEvent = parseError(ex as Error, this.options); - - return { - ...event, - event_id: hint && hint.event_id, - }; + return new QuickPromise(resolve => + parseError(ex as Error, this.options).then(event => { + resolve({ + ...event, + event_id: hint && hint.event_id, + }); + }), + ); } /** * @inheritDoc */ - public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: SentryEventHint): SentryEvent { + public eventFromMessage( + message: string, + level: Severity = Severity.Info, + hint?: SentryEventHint, + ): QuickPromise { const event: SentryEvent = { event_id: hint && hint.event_id, level, message, }; - if (this.options.attachStacktrace && hint && hint.syntheticException) { - const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; - const frames = parseStack(stack, this.options); - event.stacktrace = { - frames: prepareFramesForEvent(frames), - }; - } - - return event; + return new QuickPromise(resolve => { + if (this.options.attachStacktrace && hint && hint.syntheticException) { + const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; + parseStack(stack, this.options).then(frames => { + event.stacktrace = { + frames: prepareFramesForEvent(frames), + }; + resolve(event); + }); + } else { + resolve(event); + } + }); } } diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 8a4ccdae99bf..090c7cf4248e 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,5 +1,6 @@ import { BaseClient, Scope } from '@sentry/core'; import { SentryEvent, SentryEventHint } from '@sentry/types'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { NodeBackend, NodeOptions } from './backend'; import { SDK_NAME, SDK_VERSION } from './version'; @@ -21,7 +22,7 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): QuickPromise { event.platform = event.platform || 'node'; event.sdk = { ...event.sdk, diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index 7685c8b4a8c0..fec6c49d80d0 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -1,5 +1,6 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import { Integration, SentryEvent, SentryEventHint, SentryException } from '@sentry/types'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { getExceptionFromError } from '../parsers'; const DEFAULT_KEY = 'cause'; @@ -48,7 +49,7 @@ export class LinkedErrors implements Integration { addGlobalEventProcessor((event: SentryEvent, hint?: SentryEventHint) => { const self = getCurrentHub().getIntegration(LinkedErrors); if (self) { - return self.handler(event, hint); + return (self.handler(event, hint) as unknown) as Promise; } return event; }); @@ -57,23 +58,36 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - public handler(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null { + public handler(event: SentryEvent, hint?: SentryEventHint): QuickPromise { if (!event.exception || !event.exception.values || !hint || !(hint.originalException instanceof Error)) { - return event; + return QuickPromise.resolve(event); } - const linkedErrors = this.walkErrorTree(hint.originalException, this.key); - event.exception.values = [...linkedErrors, ...event.exception.values]; - return event; + + return new QuickPromise(resolve => { + this.walkErrorTree(hint.originalException as ExtendedError, this.key).then((linkedErrors: SentryException[]) => { + if (event && event.exception) { + event.exception.values = [...linkedErrors, ...event.exception.values]; + } + resolve(event); + }); + }); } /** * @inheritDoc */ - public walkErrorTree(error: ExtendedError, key: string, stack: SentryException[] = []): SentryException[] { + public walkErrorTree( + error: ExtendedError, + key: string, + stack: SentryException[] = [], + ): QuickPromise { if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) { - return stack; + return QuickPromise.resolve(stack); } - const exception = getExceptionFromError(error[key]); - return this.walkErrorTree(error[key], key, [exception, ...stack]); + return new QuickPromise(resolve => { + getExceptionFromError(error[key]).then((exception: SentryException) => { + this.walkErrorTree(error[key], key, [exception, ...stack]).then(resolve); + }); + }); } } diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index 313f2e08da33..f1142917f5af 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,7 +1,8 @@ import { SentryEvent, SentryException, StackFrame } from '@sentry/types'; import { basename, dirname } from '@sentry/utils/path'; +import { QuickPromise } from '@sentry/utils/quickpromise'; import { snipLine } from '@sentry/utils/string'; -import { readFileSync } from 'fs'; +import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; import * as stacktrace from 'stack-trace'; import { NodeOptions } from './backend'; @@ -73,51 +74,55 @@ function getModule(filename: string, base?: string): string { * * @param filenames Array of filepaths to read content from. */ -function readSourceFiles( - filenames: string[], -): { - [key: string]: string; -} { +function readSourceFiles(filenames: string[]): QuickPromise<{ [key: string]: string | null }> { // we're relying on filenames being de-duped already if (filenames.length === 0) { - return {}; + return QuickPromise.resolve({}); } - const sourceFiles: { - [key: string]: string; - } = {}; - - // tslint:disable-next-line:prefer-for-of - for (let i = 0; i < filenames.length; i++) { - const filename = filenames[i]; - - const cache = FILE_CONTENT_CACHE.get(filename); - // We have a cache hit - if (cache !== undefined) { - // If it's not null (which means we found a file and have a content) - // we set the content and return it later. - if (cache !== null) { - sourceFiles[filename] = cache; + return new QuickPromise<{ + [key: string]: string | null; + }>(resolve => { + const sourceFiles: { + [key: string]: string | null; + } = {}; + + let count = 0; + // tslint:disable-next-line:prefer-for-of + for (let i = 0; i < filenames.length; i++) { + const filename = filenames[i]; + + const cache = FILE_CONTENT_CACHE.get(filename); + // We have a cache hit + if (cache !== undefined) { + // If it's not null (which means we found a file and have a content) + // we set the content and return it later. + if (cache !== null) { + sourceFiles[filename] = cache; + } + count++; + // In any case we want to skip here then since we have a content already or we couldn't + // read the file and don't want to try again. + if (count === filenames.length) { + resolve(sourceFiles); + } + continue; } - // In any case we want to skip here then since we have a content already or we couldn't - // read the file and don't want to try again. - continue; - } - let content = null; - try { - content = readFileSync(filename, 'utf8'); - sourceFiles[filename] = content; - } catch (_) { - // unsure what to add here as the file is unreadable + readFile(filename, (err: Error, data: Buffer) => { + const content = err ? null : data.toString(); + sourceFiles[filename] = content; + + // We always want to set the cache, even to null which means there was an error reading the file. + // We do not want to try to read the file again. + FILE_CONTENT_CACHE.set(filename, content); + count++; + if (count === filenames.length) { + resolve(sourceFiles); + } + }); } - - // We always want to set the cache, even to null which means there was an error reading the file. - // We do not want to try to read the file again. - FILE_CONTENT_CACHE.set(filename, content); - } - - return sourceFiles; + }); } /** JSDoc */ @@ -130,7 +135,7 @@ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { } /** JSDoc */ -export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): StackFrame[] { +export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): QuickPromise { const filesToRead: string[] = []; const linesOfContext = @@ -171,7 +176,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions // We do an early return if we do not want to fetch context liens if (linesOfContext <= 0) { - return frames; + return QuickPromise.resolve(frames); } try { @@ -179,7 +184,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions } catch (_) { // This happens in electron for example where we are not able to read files from asar. // So it's fine, we recover be just returning all frames without pre/post context. - return frames; + return QuickPromise.resolve(frames); } } @@ -189,56 +194,72 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions * @param filesToRead string[] of filepaths * @param frames StackFrame[] containg all frames */ -function addPrePostContext(filesToRead: string[], frames: StackFrame[], linesOfContext: number): StackFrame[] { - const sourceFiles = readSourceFiles(filesToRead); - return frames.map(frame => { - if (frame.filename && sourceFiles[frame.filename]) { - try { - const lines = sourceFiles[frame.filename].split('\n'); - - frame.pre_context = lines - .slice(Math.max(0, (frame.lineno || 0) - (linesOfContext + 1)), (frame.lineno || 0) - 1) - .map((line: string) => snipLine(line, 0)); - - frame.context_line = snipLine(lines[(frame.lineno || 0) - 1], frame.colno || 0); - - frame.post_context = lines - .slice(frame.lineno || 0, (frame.lineno || 0) + linesOfContext) - .map((line: string) => snipLine(line, 0)); - } catch (e) { - // anomaly, being defensive in case - // unlikely to ever happen in practice but can definitely happen in theory - } - } - return frame; - }); +function addPrePostContext( + filesToRead: string[], + frames: StackFrame[], + linesOfContext: number, +): QuickPromise { + return new QuickPromise(resolve => + readSourceFiles(filesToRead).then(sourceFiles => { + const result = frames.map(frame => { + if (frame.filename && sourceFiles[frame.filename]) { + try { + const lines = (sourceFiles[frame.filename] as string).split('\n'); + + frame.pre_context = lines + .slice(Math.max(0, (frame.lineno || 0) - (linesOfContext + 1)), (frame.lineno || 0) - 1) + .map((line: string) => snipLine(line, 0)); + + frame.context_line = snipLine(lines[(frame.lineno || 0) - 1], frame.colno || 0); + + frame.post_context = lines + .slice(frame.lineno || 0, (frame.lineno || 0) + linesOfContext) + .map((line: string) => snipLine(line, 0)); + } catch (e) { + // anomaly, being defensive in case + // unlikely to ever happen in practice but can definitely happen in theory + } + } + return frame; + }); + + resolve(result); + }), + ); } /** JSDoc */ -export function getExceptionFromError(error: Error, options?: NodeOptions): SentryException { +export function getExceptionFromError(error: Error, options?: NodeOptions): QuickPromise { const name = error.name || error.constructor.name; const stack = extractStackFromError(error); - const frames = parseStack(stack, options); - - return { - stacktrace: { - frames: prepareFramesForEvent(frames), - }, - type: name, - value: error.message, - }; + return new QuickPromise(resolve => + parseStack(stack, options).then(frames => { + const result = { + stacktrace: { + frames: prepareFramesForEvent(frames), + }, + type: name, + value: error.message, + }; + resolve(result); + }), + ); } /** JSDoc */ -export function parseError(error: ExtendedError, options?: NodeOptions): SentryEvent { +export function parseError(error: ExtendedError, options?: NodeOptions): QuickPromise { const name = error.name || error.constructor.name; - const exception = getExceptionFromError(error, options); - return { - exception: { - values: [exception], - }, - message: `${name}: ${error.message || ''}`, - }; + return new QuickPromise(resolve => + getExceptionFromError(error, options).then((exception: SentryException) => { + const result = { + exception: { + values: [exception], + }, + message: `${name}: ${error.message || ''}`, + }; + resolve(result); + }), + ); } /** JSDoc */ diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts index 2228c56eccec..5be6f6261225 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -138,3 +138,11 @@ export function isRegExp(wat: any): boolean { export function isNaN(wat: any): boolean { return wat !== wat; } + +/** + * Checks wheter given value is a Promise. + * @param wat A value to be checked. + */ +export function isPromise(wat: any): boolean { + return Boolean(wat && typeof wat.then === 'function'); +} diff --git a/packages/utils/src/quickpromise.ts b/packages/utils/src/quickpromise.ts new file mode 100644 index 000000000000..844273874dc9 --- /dev/null +++ b/packages/utils/src/quickpromise.ts @@ -0,0 +1,58 @@ +/** JSDoc */ +export enum PromiseState { + /** JSDoc */ + INITIAL, + /** JSDoc */ + RESOLVED, +} + +/** JSDoc */ +export class QuickPromise { + /** JSDoc */ + private result?: T | QuickPromise; + /** JSDoc */ + private state: PromiseState = PromiseState.INITIAL; + /** JSDoc */ + private callbacks: any[] = []; + + /** JSDoc */ + public constructor(callback: (resolve: (result: T) => void | QuickPromise) => void) { + callback((result: T) => { + this.result = result; + this.state = PromiseState.RESOLVED; + this.triggerHandlers(); + }); + } + + /** JSDoc */ + public then(callback: (result: T) => void | QuickPromise): void { + if (this.state !== PromiseState.INITIAL) { + this.triggerHandler(callback); + } else { + this.callbacks.push([callback]); + } + } + + /** JSDoc */ + private triggerHandler(callback: (result: T) => void | QuickPromise): void { + if (this.state === PromiseState.RESOLVED) { + callback(this.result as T); + } + } + + /** JSDoc */ + private triggerHandlers(): void { + const callbacks = this.callbacks; + this.callbacks = []; + callbacks.forEach(call => { + this.triggerHandler(call[0]); + }); + } + + /** JSDoc */ + public static resolve(result: T): QuickPromise { + return new QuickPromise(resolve => { + resolve(result); + }); + } +} diff --git a/packages/utils/test/quickpromise.test.ts b/packages/utils/test/quickpromise.test.ts new file mode 100644 index 000000000000..f2a989529a87 --- /dev/null +++ b/packages/utils/test/quickpromise.test.ts @@ -0,0 +1,89 @@ +import { QuickPromise } from '../src/quickpromise'; + +describe('QuickPromise', () => { + test('with callback', () => { + expect.assertions(1); + + new QuickPromise(done => done(42)).done(val => { + expect(val).toEqual(42); + }); + }); + + test('with new Promise', finished => { + expect.assertions(2); + + new QuickPromise(done => { + new Promise(resolve => { + expect(true).toBe(true); + resolve(41); + }).then(done); + }).done(val => { + expect(val).toEqual(41); + finished(); + }); + }); + + test('with setTimeout', finished => { + jest.useFakeTimers(); + expect.assertions(1); + + new QuickPromise(done => { + setTimeout(() => done(41), 10); + jest.runAllTimers(); + }).done(val => { + expect(val).toEqual(41); + finished(); + }); + }); + + test('calling the callback immediatly', () => { + expect.assertions(1); + + let foo: number = 1; + + new QuickPromise(_ => { + foo = 2; + }); + + expect(foo).toEqual(2); + }); + + test('calling the callback not immediatly', () => { + jest.useFakeTimers(); + expect.assertions(5); + + const qp = new QuickPromise(done => setTimeout(() => done(2))); + qp.done(_ => { + // + }) + .done(_ => { + // + }) + .done(_ => { + // + }); + expect((qp as any).callbacks).toHaveLength(3); + expect(qp).not.toHaveProperty('result'); + qp.done(result => { + expect(result).toEqual(2); + }); + expect((qp as any).callbacks).toHaveLength(4); + jest.runAllTimers(); + expect(qp).toHaveProperty('result'); + }); + + test('multi then', () => { + expect.assertions(3); + + new QuickPromise(done => done(2)) + .done(result => { + expect(result).toEqual(2); + }) + .done(result => { + expect(result).toEqual(2); + }) + .done(result => { + expect(result).toEqual(2); + }); + }); +}); From afcdb4c8f422af04f264328d4a038cb13d053d3e Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 28 Jan 2019 14:17:36 +0100 Subject: [PATCH 05/16] feat: SyncPromise implementation --- packages/core/src/basebackend.ts | 10 +- packages/core/src/baseclient.ts | 15 +- packages/core/src/interfaces.ts | 6 +- packages/core/test/lib/base.test.ts | 2 +- packages/core/test/lib/dsn.test.ts | 2 +- .../lib/integrations/rewriteframes.test.ts | 22 +- packages/hub/package.json | 2 +- packages/hub/src/scope.ts | 45 +- packages/hub/test/hub.test.ts | 43 +- packages/hub/test/scope.test.ts | 100 +++-- packages/minimal/package.json | 2 +- packages/node/src/backend.ts | 10 +- packages/node/src/client.ts | 4 +- .../node/src/integrations/linkederrors.ts | 14 +- packages/node/src/parsers.ts | 26 +- packages/utils/package.json | 2 +- packages/utils/src/is.ts | 8 +- packages/utils/src/quickpromise.ts | 58 --- packages/utils/src/syncpromise.ts | 413 ++++++++++++++++++ packages/utils/test/is.test.ts | 19 +- packages/utils/test/quickpromise.test.ts | 89 ---- packages/utils/test/syncpromise.test.ts | 142 ++++++ 22 files changed, 729 insertions(+), 305 deletions(-) delete mode 100644 packages/utils/src/quickpromise.ts create mode 100644 packages/utils/src/syncpromise.ts delete mode 100644 packages/utils/test/quickpromise.test.ts create mode 100644 packages/utils/test/syncpromise.test.ts diff --git a/packages/core/src/basebackend.ts b/packages/core/src/basebackend.ts index f3c85d36fa93..fe214cab84be 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -3,7 +3,7 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, Transport } from '@ import { SentryError } from '@sentry/utils/error'; import { logger } from '@sentry/utils/logger'; import { serialize } from '@sentry/utils/object'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { Backend, Options } from './interfaces'; import { NoopTransport } from './transports/noop'; @@ -41,14 +41,14 @@ export abstract class BaseBackend implements Backend { /** * @inheritDoc */ - public eventFromException(_exception: any, _hint?: SentryEventHint): QuickPromise { + public eventFromException(_exception: any, _hint?: SentryEventHint): SyncPromise { throw new SentryError('Backend has to implement `eventFromException` method'); } /** * @inheritDoc */ - public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): QuickPromise { + public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): SyncPromise { throw new SentryError('Backend has to implement `eventFromMessage` method'); } @@ -56,7 +56,9 @@ export abstract class BaseBackend implements Backend { * @inheritDoc */ public sendEvent(event: SentryEvent): void { - this.transport.sendEvent(serialize(event)); + this.transport.sendEvent(serialize(event)).catch(reason => { + logger.error(`Error while sending event: ${reason}`); + }); } /** diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 92d4494a2f6a..511d89e6171f 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -11,7 +11,7 @@ import { import { isPrimitive } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { truncate } from '@sentry/utils/string'; import { BackendClass } from './basebackend'; import { Dsn } from './dsn'; @@ -224,7 +224,7 @@ export abstract class BaseClient implement * @param scope A scope containing event metadata. * @returns A new event with more information. */ - protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): QuickPromise { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SyncPromise { const { environment, maxBreadcrumbs = DEFAULT_BREADCRUMBS, release, dist } = this.getOptions(); const prepared = { ...event }; @@ -263,7 +263,7 @@ export abstract class BaseClient implement return scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS)); } - return QuickPromise.resolve(prepared); + return SyncPromise.resolve(prepared); } /** @@ -331,13 +331,8 @@ export abstract class BaseClient implement return; } - try { - this.getBackend().sendEvent(finalEvent); - } catch (error) { - // We have a catch here since the transport can reject the request internally. - // If we do not catch this here, we will run into an endless loop. - logger.error(`${error}`); - } + // From here on we are really async + this.getBackend().sendEvent(finalEvent); }); } diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 1c951728041f..7ca1dd31f754 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -11,7 +11,7 @@ import { TransportClass, TransportOptions, } from '@sentry/types'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { Dsn } from './dsn'; /** Console logging verbosity for the SDK. */ @@ -244,10 +244,10 @@ export interface Backend { install?(): boolean; /** Creates a {@link SentryEvent} from an exception. */ - eventFromException(exception: any, hint?: SentryEventHint): QuickPromise; + eventFromException(exception: any, hint?: SentryEventHint): SyncPromise; /** Creates a {@link SentryEvent} from a plain message. */ - eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): QuickPromise; + eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): SyncPromise; /** Submits the event to Sentry */ sendEvent(event: SentryEvent): void; diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 5698c353bd94..b5c3ea6c88a2 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,6 +1,6 @@ import { Scope } from '@sentry/hub'; import { SentryEvent, Status } from '@sentry/types'; -import { SentryError } from '../../src/error'; +import { SentryError } from '@sentry/utils/error'; import { TestBackend } from '../mocks/backend'; import { TestClient } from '../mocks/client'; import { TestIntegration } from '../mocks/integration'; diff --git a/packages/core/test/lib/dsn.test.ts b/packages/core/test/lib/dsn.test.ts index fc91da7bf3df..b7251b7ec804 100644 --- a/packages/core/test/lib/dsn.test.ts +++ b/packages/core/test/lib/dsn.test.ts @@ -1,5 +1,5 @@ +import { SentryError } from '@sentry/utils/error'; import { Dsn } from '../../src/dsn'; -import { SentryError } from '../../src/error'; describe('Dsn', () => { describe('fromComponents', () => { diff --git a/packages/core/test/lib/integrations/rewriteframes.test.ts b/packages/core/test/lib/integrations/rewriteframes.test.ts index e524d835448a..e825139877b6 100644 --- a/packages/core/test/lib/integrations/rewriteframes.test.ts +++ b/packages/core/test/lib/integrations/rewriteframes.test.ts @@ -44,14 +44,14 @@ describe('RewriteFrames', () => { rewriteFrames = new RewriteFrames(); }); - it('transforms messageEvent frames', async () => { - const event = await rewriteFrames.process(messageEvent); + it('transforms messageEvent frames', () => { + const event = rewriteFrames.process(messageEvent); expect(event.stacktrace!.frames![0].filename).toEqual('app:///file1.js'); expect(event.stacktrace!.frames![1].filename).toEqual('app:///file2.js'); }); - it('transforms exceptionEvent frames', async () => { - const event = await rewriteFrames.process(exceptionEvent); + it('transforms exceptionEvent frames', () => { + const event = rewriteFrames.process(exceptionEvent); expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js'); expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js'); }); @@ -64,14 +64,14 @@ describe('RewriteFrames', () => { }); }); - it('transforms messageEvent frames', async () => { - const event = await rewriteFrames.process(messageEvent); + it('transforms messageEvent frames', () => { + const event = rewriteFrames.process(messageEvent); expect(event.stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js'); expect(event.stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js'); }); - it('transforms exceptionEvent frames', async () => { - const event = await rewriteFrames.process(exceptionEvent); + it('transforms exceptionEvent frames', () => { + const event = rewriteFrames.process(exceptionEvent); expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js'); expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js'); }); @@ -80,7 +80,7 @@ describe('RewriteFrames', () => { describe('can use custom iteratee', () => { beforeEach(() => { rewriteFrames = new RewriteFrames({ - iteratee: async (frame: StackFrame) => ({ + iteratee: (frame: StackFrame) => ({ ...frame, function: 'whoops', }), @@ -88,7 +88,7 @@ describe('RewriteFrames', () => { }); it('transforms messageEvent frames', async () => { - const event = await rewriteFrames.process(messageEvent); + const event = rewriteFrames.process(messageEvent); expect(event.stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js'); expect(event.stacktrace!.frames![0].function).toEqual('whoops'); expect(event.stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js'); @@ -96,7 +96,7 @@ describe('RewriteFrames', () => { }); it('transforms exceptionEvent frames', async () => { - const event = await rewriteFrames.process(exceptionEvent); + const event = rewriteFrames.process(exceptionEvent); expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js'); expect(event.exception!.values![0].stacktrace!.frames![0].function).toEqual('whoops'); expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js'); diff --git a/packages/hub/package.json b/packages/hub/package.json index 0df484fd3218..ed53a785fc92 100644 --- a/packages/hub/package.json +++ b/packages/hub/package.json @@ -44,7 +44,7 @@ "fix:prettier": "prettier --write \"{src,test}/**/*.ts\"", "fix:tslint": "tslint --fix -t stylish -p .", "test": "jest", - "test:watch": "jest --watch --notify" + "test:watch": "jest --watch" }, "jest": { "collectCoverage": true, diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 7d94e677a105..4d94b9a0cdeb 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,8 +1,8 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, User } from '@sentry/types'; -import { isPromise } from '@sentry/utils/is'; +import { isFunction, isThenable } from '@sentry/utils/is'; import { getGlobalObject } from '@sentry/utils/misc'; import { assign, safeNormalize } from '@sentry/utils/object'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; export type EventProcessor = ( event: SentryEvent, @@ -75,28 +75,29 @@ export class Scope { event: SentryEvent | null, hint?: SentryEventHint, index: number = 0, - callback?: (result: SentryEvent | null) => void, - ): QuickPromise { - return new QuickPromise(resolve => { - if (event === null) { - resolve(null); + ): SyncPromise { + return new SyncPromise((resolve, reject) => { + const processor = processors[index]; + if (event === null || !isFunction(processor)) { + resolve(event); } else { - const processor = processors[index]; - if (isPromise(processor)) { - (processor({ ...event }, hint) as Promise).then((processedEvent: SentryEvent | null) => { - if (index === processors.length - 1 && callback) { - callback(processedEvent); - } else { - this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve); - } - }); - } else { - const processedEvent = processor({ ...event }, hint) as SentryEvent | null; - if (index === processors.length - 1 && callback) { - callback(processedEvent); + try { + const result = processor({ ...event }, hint) as SentryEvent | null; + if (isThenable(result)) { + (result as Promise) + .then((final: SentryEvent | null) => { + this.notifyEventProcessors(processors, final, hint, index + 1) + .then(resolve) + .catch(reject); + }) + .catch(reject); } else { - this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve); + this.notifyEventProcessors(processors, result, hint, index + 1) + .then(resolve) + .catch(reject); } + } catch (e) { + reject(e); } } }); @@ -232,7 +233,7 @@ export class Scope { event: SentryEvent, hint?: SentryEventHint, maxBreadcrumbs?: number, - ): QuickPromise { + ): SyncPromise { if (this.extra && Object.keys(this.extra).length) { event.extra = { ...this.extra, ...event.extra }; } diff --git a/packages/hub/test/hub.test.ts b/packages/hub/test/hub.test.ts index 7992a3de89fd..75e455c9261b 100644 --- a/packages/hub/test/hub.test.ts +++ b/packages/hub/test/hub.test.ts @@ -3,7 +3,6 @@ import { logger } from '@sentry/utils/logger'; import { getCurrentHub, Hub, Scope } from '../src'; const clientFn = jest.fn(); -const asyncClientFn = async () => Promise.reject('error'); const scope = new Scope(); describe('Hub', () => { @@ -19,7 +18,6 @@ describe('Hub', () => { test('pass in filled layer', () => { const hub = new Hub({ - asyncClientFn, clientFn, }); expect(hub.getStack()).toHaveLength(1); @@ -28,7 +26,6 @@ describe('Hub', () => { test('invoke client sync', () => { const hub = new Hub( { - asyncClientFn, clientFn, }, scope, @@ -42,7 +39,6 @@ describe('Hub', () => { test("don't invoke client sync with wrong func", () => { const hub = new Hub({ - asyncClientFn, clientFn, }); // @ts-ignore @@ -50,20 +46,6 @@ describe('Hub', () => { expect(clientFn).not.toHaveBeenCalled(); }); - test('invoke client async catch error in case', done => { - jest.spyOn(logger, 'error'); - const hub = new Hub({ - asyncClientFn, - clientFn, - }); - (hub as any).invokeClientAsync('asyncClientFn', true); - setTimeout(() => { - // tslint:disable-next-line - expect(logger.error).toHaveBeenCalled(); - done(); - }); - }); - test('isOlderThan', () => { const hub = new Hub(); expect(hub.isOlderThan(0)).toBeFalsy(); @@ -190,7 +172,7 @@ describe('Hub', () => { test('captureException', () => { const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureException('a'); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0]).toBe('captureException'); @@ -199,7 +181,7 @@ describe('Hub', () => { test('captureMessage', () => { const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureMessage('a'); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0]).toBe('captureMessage'); @@ -211,7 +193,7 @@ describe('Hub', () => { extra: { b: 3 }, }; const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureEvent(event); expect(spy).toHaveBeenCalled(); expect(spy.mock.calls[0][0]).toBe('captureEvent'); @@ -245,37 +227,38 @@ describe('Hub', () => { }); }); - test('pushScope inherit processors', async () => { + test('pushScope inherit processors', () => { + expect.assertions(1); const event: SentryEvent = { extra: { b: 3 }, }; const localScope = new Scope(); localScope.setExtra('a', 'b'); const hub = new Hub({ a: 'b' }, localScope); - const callCounter = jest.fn(); + localScope.addEventProcessor(async (processedEvent: SentryEvent) => { - callCounter(1); processedEvent.dist = '1'; return processedEvent; }); + hub.pushScope(); const pushedScope = hub.getStackTop().scope; - if (pushedScope) { - const final = await pushedScope.applyToEvent(event); + + return pushedScope!.applyToEvent(event).then(final => { expect(final!.dist).toEqual('1'); - } + }); }); test('captureException should set event_id in hint', () => { const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureException('a'); expect(spy.mock.calls[0][2].event_id).toBeTruthy(); }); test('captureMessage should set event_id in hint', () => { const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureMessage('a'); expect(spy.mock.calls[0][3].event_id).toBeTruthy(); }); @@ -285,7 +268,7 @@ describe('Hub', () => { extra: { b: 3 }, }; const hub = new Hub(); - const spy = jest.spyOn(hub as any, 'invokeClientAsync'); + const spy = jest.spyOn(hub as any, 'invokeClient'); hub.captureEvent(event); expect(spy.mock.calls[0][2].event_id).toBeTruthy(); }); diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 4b223e868631..9707983b5b8c 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -86,7 +86,8 @@ describe('Scope', () => { expect(listener.mock.calls[0][0].extra).toEqual({ a: 2 }); }); - test('applyToEvent', async () => { + test('applyToEvent', () => { + expect.assertions(6); const scope = new Scope(); scope.setExtra('a', 2); scope.setTag('a', 'b'); @@ -95,16 +96,18 @@ describe('Scope', () => { scope.setLevel(Severity.Warning); scope.addBreadcrumb({ message: 'test' }, 100); const event: SentryEvent = {}; - await scope.applyToEvent(event); - expect(event.extra).toEqual({ a: 2 }); - expect(event.tags).toEqual({ a: 'b' }); - expect(event.user).toEqual({ id: '1' }); - expect(event.fingerprint).toEqual(['abcd']); - expect(event.level).toEqual('warning'); - expect(event.breadcrumbs).toEqual([{ message: 'test' }]); + return scope.applyToEvent(event).then(processedEvent => { + expect(processedEvent!.extra).toEqual({ a: 2 }); + expect(processedEvent!.tags).toEqual({ a: 'b' }); + expect(processedEvent!.user).toEqual({ id: '1' }); + expect(processedEvent!.fingerprint).toEqual(['abcd']); + expect(processedEvent!.level).toEqual('warning'); + expect(processedEvent!.breadcrumbs).toEqual([{ message: 'test' }]); + }); }); - test('applyToEvent merge', async () => { + test('applyToEvent merge', () => { + expect.assertions(5); const scope = new Scope(); scope.setExtra('a', 2); scope.setTag('a', 'b'); @@ -118,31 +121,36 @@ describe('Scope', () => { tags: { b: 'c' }, user: { id: '3' }, }; - await scope.applyToEvent(event); - expect(event.extra).toEqual({ a: 2, b: 3 }); - expect(event.tags).toEqual({ a: 'b', b: 'c' }); - expect(event.user).toEqual({ id: '3' }); - expect(event.fingerprint).toEqual(['efgh', 'abcd']); - expect(event.breadcrumbs).toEqual([{ message: 'test2' }]); + return scope.applyToEvent(event).then(processedEvent => { + expect(processedEvent!.extra).toEqual({ a: 2, b: 3 }); + expect(processedEvent!.tags).toEqual({ a: 'b', b: 'c' }); + expect(processedEvent!.user).toEqual({ id: '3' }); + expect(processedEvent!.fingerprint).toEqual(['efgh', 'abcd']); + expect(processedEvent!.breadcrumbs).toEqual([{ message: 'test2' }]); + }); }); test('applyToEvent message fingerprint', async () => { + expect.assertions(1); const scope = new Scope(); const event: SentryEvent = { fingerprint: ['bar'], message: 'foo', }; - await scope.applyToEvent(event); - expect(event.fingerprint).toEqual(['bar', 'foo']); + return scope.applyToEvent(event).then(processedEvent => { + expect(processedEvent!.fingerprint).toEqual(['bar', 'foo']); + }); }); - test('applyToEvent scope level should be stronger', async () => { + test('applyToEvent scope level should be stronger', () => { + expect.assertions(1); const scope = new Scope(); scope.setLevel(Severity.Warning); const event: SentryEvent = {}; event.level = Severity.Critical; - await scope.applyToEvent(event); - expect(event.level).toEqual('warning'); + return scope.applyToEvent(event).then(processedEvent => { + expect(processedEvent!.level).toEqual('warning'); + }); }); test('clear', () => { @@ -157,30 +165,33 @@ describe('Scope', () => { expect((scope as any).extra).toEqual({}); }); - test('addEventProcessor', async done => { - expect.assertions(2); + test('addEventProcessor', () => { + expect.assertions(3); const event: SentryEvent = { extra: { b: 3 }, }; const localScope = new Scope(); localScope.setExtra('a', 'b'); - localScope.addEventProcessor(async (processedEvent: SentryEvent) => { + localScope.addEventProcessor((processedEvent: SentryEvent) => { expect(processedEvent.extra).toEqual({ a: 'b', b: 3 }); return processedEvent; }); - localScope.addEventProcessor(async (processedEvent: SentryEvent) => { + localScope.addEventProcessor((processedEvent: SentryEvent) => { processedEvent.dist = '1'; return processedEvent; }); - localScope.addEventProcessor(async (processedEvent: SentryEvent) => { + localScope.addEventProcessor((processedEvent: SentryEvent) => { expect(processedEvent.dist).toEqual('1'); - done(); return processedEvent; }); - await localScope.applyToEvent(event); + + return localScope.applyToEvent(event).then(final => { + expect(final!.dist).toEqual('1'); + }); }); - test('addEventProcessor async', async () => { + test('addEventProcessor async', () => { + jest.useFakeTimers(); expect.assertions(6); const event: SentryEvent = { extra: { b: 3 }, @@ -188,7 +199,7 @@ describe('Scope', () => { const localScope = new Scope(); localScope.setExtra('a', 'b'); const callCounter = jest.fn(); - localScope.addEventProcessor(async (processedEvent: SentryEvent) => { + localScope.addEventProcessor((processedEvent: SentryEvent) => { callCounter(1); expect(processedEvent.extra).toEqual({ a: 'b', b: 3 }); return processedEvent; @@ -202,21 +213,24 @@ describe('Scope', () => { processedEvent.dist = '1'; resolve(processedEvent); }, 1); + jest.runAllTimers(); }), ); - localScope.addEventProcessor(async (processedEvent: SentryEvent) => { + localScope.addEventProcessor((processedEvent: SentryEvent) => { callCounter(4); return processedEvent; }); - const final = await localScope.applyToEvent(event); - expect(callCounter.mock.calls[0][0]).toBe(1); - expect(callCounter.mock.calls[1][0]).toBe(2); - expect(callCounter.mock.calls[2][0]).toBe(3); - expect(callCounter.mock.calls[3][0]).toBe(4); - expect(final!.dist).toEqual('1'); + + return localScope.applyToEvent(event).then(processedEvent => { + expect(callCounter.mock.calls[0][0]).toBe(1); + expect(callCounter.mock.calls[1][0]).toBe(2); + expect(callCounter.mock.calls[2][0]).toBe(3); + expect(callCounter.mock.calls[3][0]).toBe(4); + expect(processedEvent!.dist).toEqual('1'); + }); }); - test('addEventProcessor return null', async () => { + test('addEventProcessor return null', () => { expect.assertions(1); const event: SentryEvent = { extra: { b: 3 }, @@ -224,11 +238,12 @@ describe('Scope', () => { const localScope = new Scope(); localScope.setExtra('a', 'b'); localScope.addEventProcessor(async (_: SentryEvent) => null); - const final = await localScope.applyToEvent(event); - expect(final).toBeNull(); + return localScope.applyToEvent(event).then(processedEvent => { + expect(processedEvent).toBeNull(); + }); }); - test('addEventProcessor pass along hint', async () => { + test('addEventProcessor pass along hint', () => { expect.assertions(3); const event: SentryEvent = { extra: { b: 3 }, @@ -240,7 +255,8 @@ describe('Scope', () => { expect(hint!.syntheticException).toBeTruthy(); return internalEvent; }); - const final = await localScope.applyToEvent(event, { syntheticException: new Error('what') }); - expect(final).toEqual(event); + return localScope.applyToEvent(event, { syntheticException: new Error('what') }).then(processedEvent => { + expect(processedEvent).toEqual(event); + }); }); }); diff --git a/packages/minimal/package.json b/packages/minimal/package.json index ce09250ce2ab..c542775f023e 100644 --- a/packages/minimal/package.json +++ b/packages/minimal/package.json @@ -44,7 +44,7 @@ "fix:prettier": "prettier --write \"{src,test}/**/*.ts\"", "fix:tslint": "tslint --fix -t stylish -p .", "test": "jest", - "test:watch": "jest --watch --notify" + "test:watch": "jest --watch" }, "jest": { "collectCoverage": true, diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index c400b40cd181..59fa603cfacb 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -2,7 +2,7 @@ import { BaseBackend, Dsn, getCurrentHub, Options } from '@sentry/core'; import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types'; import { isError, isPlainObject } from '@sentry/utils/is'; import { limitObjectDepthToSize, serializeKeysToEventMessage } from '@sentry/utils/object'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { createHash } from 'crypto'; import { extractStackFromError, parseError, parseStack, prepareFramesForEvent } from './parsers'; import { HTTPSTransport, HTTPTransport } from './transports'; @@ -70,7 +70,7 @@ export class NodeBackend extends BaseBackend { /** * @inheritDoc */ - public eventFromException(exception: any, hint?: SentryEventHint): QuickPromise { + public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise { let ex: any = exception; if (!isError(exception)) { @@ -98,7 +98,7 @@ export class NodeBackend extends BaseBackend { } } - return new QuickPromise(resolve => + return new SyncPromise(resolve => parseError(ex as Error, this.options).then(event => { resolve({ ...event, @@ -115,14 +115,14 @@ export class NodeBackend extends BaseBackend { message: string, level: Severity = Severity.Info, hint?: SentryEventHint, - ): QuickPromise { + ): SyncPromise { const event: SentryEvent = { event_id: hint && hint.event_id, level, message, }; - return new QuickPromise(resolve => { + return new SyncPromise(resolve => { if (this.options.attachStacktrace && hint && hint.syntheticException) { const stack = hint.syntheticException ? extractStackFromError(hint.syntheticException) : []; parseStack(stack, this.options).then(frames => { diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 090c7cf4248e..df295e677d80 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -1,6 +1,6 @@ import { BaseClient, Scope } from '@sentry/core'; import { SentryEvent, SentryEventHint } from '@sentry/types'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { NodeBackend, NodeOptions } from './backend'; import { SDK_NAME, SDK_VERSION } from './version'; @@ -22,7 +22,7 @@ export class NodeClient extends BaseClient { /** * @inheritDoc */ - protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): QuickPromise { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SyncPromise { event.platform = event.platform || 'node'; event.sdk = { ...event.sdk, diff --git a/packages/node/src/integrations/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index fec6c49d80d0..df7395aabb5a 100644 --- a/packages/node/src/integrations/linkederrors.ts +++ b/packages/node/src/integrations/linkederrors.ts @@ -1,6 +1,6 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/core'; import { Integration, SentryEvent, SentryEventHint, SentryException } from '@sentry/types'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { getExceptionFromError } from '../parsers'; const DEFAULT_KEY = 'cause'; @@ -58,12 +58,12 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - public handler(event: SentryEvent, hint?: SentryEventHint): QuickPromise { + public handler(event: SentryEvent, hint?: SentryEventHint): SyncPromise { if (!event.exception || !event.exception.values || !hint || !(hint.originalException instanceof Error)) { - return QuickPromise.resolve(event); + return SyncPromise.resolve(event); } - return new QuickPromise(resolve => { + return new SyncPromise(resolve => { this.walkErrorTree(hint.originalException as ExtendedError, this.key).then((linkedErrors: SentryException[]) => { if (event && event.exception) { event.exception.values = [...linkedErrors, ...event.exception.values]; @@ -80,11 +80,11 @@ export class LinkedErrors implements Integration { error: ExtendedError, key: string, stack: SentryException[] = [], - ): QuickPromise { + ): SyncPromise { if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) { - return QuickPromise.resolve(stack); + return SyncPromise.resolve(stack); } - return new QuickPromise(resolve => { + return new SyncPromise(resolve => { getExceptionFromError(error[key]).then((exception: SentryException) => { this.walkErrorTree(error[key], key, [exception, ...stack]).then(resolve); }); diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index f1142917f5af..22a9dd672f58 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,6 +1,6 @@ import { SentryEvent, SentryException, StackFrame } from '@sentry/types'; import { basename, dirname } from '@sentry/utils/path'; -import { QuickPromise } from '@sentry/utils/quickpromise'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { snipLine } from '@sentry/utils/string'; import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; @@ -74,13 +74,13 @@ function getModule(filename: string, base?: string): string { * * @param filenames Array of filepaths to read content from. */ -function readSourceFiles(filenames: string[]): QuickPromise<{ [key: string]: string | null }> { +function readSourceFiles(filenames: string[]): SyncPromise<{ [key: string]: string | null }> { // we're relying on filenames being de-duped already if (filenames.length === 0) { - return QuickPromise.resolve({}); + return SyncPromise.resolve({}); } - return new QuickPromise<{ + return new SyncPromise<{ [key: string]: string | null; }>(resolve => { const sourceFiles: { @@ -135,7 +135,7 @@ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { } /** JSDoc */ -export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): QuickPromise { +export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): SyncPromise { const filesToRead: string[] = []; const linesOfContext = @@ -176,7 +176,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions // We do an early return if we do not want to fetch context liens if (linesOfContext <= 0) { - return QuickPromise.resolve(frames); + return SyncPromise.resolve(frames); } try { @@ -184,7 +184,7 @@ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions } catch (_) { // This happens in electron for example where we are not able to read files from asar. // So it's fine, we recover be just returning all frames without pre/post context. - return QuickPromise.resolve(frames); + return SyncPromise.resolve(frames); } } @@ -198,8 +198,8 @@ function addPrePostContext( filesToRead: string[], frames: StackFrame[], linesOfContext: number, -): QuickPromise { - return new QuickPromise(resolve => +): SyncPromise { + return new SyncPromise(resolve => readSourceFiles(filesToRead).then(sourceFiles => { const result = frames.map(frame => { if (frame.filename && sourceFiles[frame.filename]) { @@ -229,10 +229,10 @@ function addPrePostContext( } /** JSDoc */ -export function getExceptionFromError(error: Error, options?: NodeOptions): QuickPromise { +export function getExceptionFromError(error: Error, options?: NodeOptions): SyncPromise { const name = error.name || error.constructor.name; const stack = extractStackFromError(error); - return new QuickPromise(resolve => + return new SyncPromise(resolve => parseStack(stack, options).then(frames => { const result = { stacktrace: { @@ -247,9 +247,9 @@ export function getExceptionFromError(error: Error, options?: NodeOptions): Quic } /** JSDoc */ -export function parseError(error: ExtendedError, options?: NodeOptions): QuickPromise { +export function parseError(error: ExtendedError, options?: NodeOptions): SyncPromise { const name = error.name || error.constructor.name; - return new QuickPromise(resolve => + return new SyncPromise(resolve => getExceptionFromError(error, options).then((exception: SentryException) => { const result = { exception: { diff --git a/packages/utils/package.json b/packages/utils/package.json index 38dfe1d72293..50cf15c56d41 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -42,7 +42,7 @@ "fix:prettier": "prettier --write \"{src,test}/**/*.ts\"", "fix:tslint": "tslint --fix -t stylish -p .", "test": "jest", - "test:watch": "jest --watch --notify" + "test:watch": "jest --watch" }, "jest": { "collectCoverage": true, diff --git a/packages/utils/src/is.ts b/packages/utils/src/is.ts index 5be6f6261225..283378d09c71 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -140,9 +140,11 @@ export function isNaN(wat: any): boolean { } /** - * Checks wheter given value is a Promise. + * Checks whether given value has a then function. * @param wat A value to be checked. */ -export function isPromise(wat: any): boolean { - return Boolean(wat && typeof wat.then === 'function'); +export function isThenable(wat: any): boolean { + // tslint:disable:no-unsafe-any + return Boolean(wat && wat.then && isFunction(wat.then)); + // tslint:enable:no-unsafe-any } diff --git a/packages/utils/src/quickpromise.ts b/packages/utils/src/quickpromise.ts deleted file mode 100644 index 844273874dc9..000000000000 --- a/packages/utils/src/quickpromise.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** JSDoc */ -export enum PromiseState { - /** JSDoc */ - INITIAL, - /** JSDoc */ - RESOLVED, -} - -/** JSDoc */ -export class QuickPromise { - /** JSDoc */ - private result?: T | QuickPromise; - /** JSDoc */ - private state: PromiseState = PromiseState.INITIAL; - /** JSDoc */ - private callbacks: any[] = []; - - /** JSDoc */ - public constructor(callback: (resolve: (result: T) => void | QuickPromise) => void) { - callback((result: T) => { - this.result = result; - this.state = PromiseState.RESOLVED; - this.triggerHandlers(); - }); - } - - /** JSDoc */ - public then(callback: (result: T) => void | QuickPromise): void { - if (this.state !== PromiseState.INITIAL) { - this.triggerHandler(callback); - } else { - this.callbacks.push([callback]); - } - } - - /** JSDoc */ - private triggerHandler(callback: (result: T) => void | QuickPromise): void { - if (this.state === PromiseState.RESOLVED) { - callback(this.result as T); - } - } - - /** JSDoc */ - private triggerHandlers(): void { - const callbacks = this.callbacks; - this.callbacks = []; - callbacks.forEach(call => { - this.triggerHandler(call[0]); - }); - } - - /** JSDoc */ - public static resolve(result: T): QuickPromise { - return new QuickPromise(resolve => { - resolve(result); - }); - } -} diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts new file mode 100644 index 000000000000..a9b3a04b8062 --- /dev/null +++ b/packages/utils/src/syncpromise.ts @@ -0,0 +1,413 @@ +import { isThenable } from './is'; + +/** JSDoc */ +enum States { + /** JSDoc */ + PENDING = 'PENDING', + /** JSDoc */ + RESOLVED = 'RESOLVED', + /** JSDoc */ + REJECTED = 'REJECTED', +} + +/** JSDoc */ +interface Handler { + onSuccess: HandlerOnSuccess; + onFail: HandlerOnFail; +} + +type HandlerOnSuccess = (value: T) => U | Thenable; +type HandlerOnFail = (reason: any) => U | Thenable; +type Finally = () => U | Thenable; + +/** JSDoc */ +interface Thenable { + then(onSuccess?: HandlerOnSuccess, onFail?: HandlerOnFail | ((reason: any) => void)): Thenable; +} + +type Resolve = (value?: R | Thenable | any) => void; +type Reject = (value?: any) => void; + +/** JSDoc */ +export class SyncPromise { + /** JSDoc */ + private state: States = States.PENDING; + /** JSDoc */ + private handlers: Array> = []; + /** JSDoc */ + private value: T | any; + + public constructor(callback: (resolve: Resolve, reject: Reject) => void) { + try { + callback(this.resolve, this.reject); + } catch (e) { + this.reject(e); + } + } + + /** JSDoc */ + private readonly resolve = (value: T) => { + this.setResult(value, States.RESOLVED); + }; + + /** JSDoc */ + private readonly reject = (reason: any) => { + this.setResult(reason, States.REJECTED); + }; + + /** JSDoc */ + private readonly setResult = (value: T | any, state: States) => { + const set = () => { + if (this.state !== States.PENDING) { + return null; + } + + if (isThenable(value)) { + return (value as Thenable).then(this.resolve, this.reject); + } + + this.value = value; + this.state = state; + + return this.executeHandlers(); + }; + + set(); + }; + + /** JSDoc */ + private readonly executeHandlers = () => { + if (this.state === States.PENDING) { + return null; + } + + this.handlers.forEach(handler => { + if (this.state === States.REJECTED) { + // tslint:disable-next-line:no-unsafe-any + return handler.onFail(this.value); + } + // tslint:disable-next-line:no-unsafe-any + return handler.onSuccess(this.value); + }); + + this.handlers = []; + return null; + }; + + /** JSDoc */ + private readonly attachHandler = (handler: Handler) => { + this.handlers = [...this.handlers, handler]; + + this.executeHandlers(); + }; + + /** JSDoc */ + public then(onSuccess?: HandlerOnSuccess, onFail?: HandlerOnFail): SyncPromise { + return new SyncPromise((resolve, reject) => { + this.attachHandler({ + onFail: reason => { + if (!onFail) { + reject(reason); + return; + } + + try { + resolve(onFail(reason)); + return; + } catch (e) { + reject(e); + return; + } + }, + onSuccess: result => { + if (!onSuccess) { + resolve(result); + return; + } + + try { + resolve(onSuccess(result)); + return; + } catch (e) { + reject(e); + return; + } + }, + }); + }); + } + + /** JSDoc */ + public catch(onFail: HandlerOnFail): SyncPromise { + // tslint:disable-next-line:no-unsafe-any + return this.then((val: any) => val, onFail); + } + + /** JSDoc */ + public toString(): string { + return `[object SyncPromise]`; + } + + /** JSDoc */ + public finally(cb: Finally): SyncPromise { + return new SyncPromise((resolve, reject) => { + let val: U | any; + let isRejected: boolean; + + return this.then( + value => { + isRejected = false; + val = value; + return cb(); + }, + reason => { + isRejected = true; + val = reason; + return cb(); + }, + ).then(() => { + if (isRejected) { + reject(val); + return; + } + + // tslint:disable-next-line:no-unsafe-any + resolve(val); + return; + }); + }); + } + + /** JSDoc */ + public static resolve(value?: U | Thenable): SyncPromise { + return new SyncPromise(resolve => { + resolve(value); + }); + } + + /** JSDoc */ + public static reject(reason?: any): SyncPromise { + return new SyncPromise((_, reject) => { + reject(reason); + }); + } +} + +// /** JSDoc */ +// export enum PromiseState { +// /** JSDoc */ +// PENDING, +// /** JSDoc */ +// RESOLVED, +// /** JSDoc */ +// REJECTED, +// } + +// /** JSDoc*/ +// interface Handle { +// onFulfilled?: ((value: T) => T | SyncPromise); +// onRejected?: ((reason: any) => void); +// } + +// /** JSDoc */ +// export class SyncPromise { +// /** JSDoc */ +// private value?: T | SyncPromise; + +// /** JSDoc */ +// private reason?: any; + +// /** JSDoc */ +// private state: PromiseState = PromiseState.PENDING; +// /** JSDoc */ +// private handlers: Array> = []; + +// /** JSDoc*/ +// private readonly id: number = Math.random(); + +// /** JSDoc */ +// public constructor( +// callback: ( +// resolve: ((value: T) => T | SyncPromise), +// reject: ((reason: any) => never | SyncPromise | void), +// ) => void, +// ) { +// callback( +// (value: T) => { +// console.log('CTOR callback RESOLVE'); +// // console.log('res', this.donee); +// this.value = value; +// this.state = PromiseState.RESOLVED; +// this.triggerHandlers(); +// return this.value; +// }, +// (reason: any) => { +// console.log('CTOR callback REJECT'); +// this.reason = reason; +// this.state = PromiseState.REJECTED; +// this.triggerHandlers(); +// }, +// ); +// } + +// /** JSDoc */ +// public then( +// onFulfilled?: ((value: T) => TResult1 | SyncPromise) | null, +// onRejected?: ((reason: any) => TResult2 | SyncPromise | never | void) | null, +// ): SyncPromise { +// /** JSDoc */ +// // public then( +// // onFulfilled?: ((value: T) => T) | undefined | null, +// // onRejected?: ((reason: any) => never) | undefined | null, +// // ): SyncPromise { +// return new SyncPromise((resolve, reject) => { +// this.done( +// result => { +// console.log('onFulfilled!!!'); +// console.log('result:', result); +// console.log('onFulfilled:', onFulfilled); +// console.log('onRejected:', onRejected); +// if (onFulfilled) { +// try { +// // @ts-ignore +// return resolve(onFulfilled(result)); +// } catch (ex) { +// return reject(ex); +// } +// } else { +// // @ts-ignore +// return resolve(result); +// } +// }, +// reason => { +// console.log('onRejected!!!'); +// console.log('reason:', reason); +// console.log('onFulfilled:', onFulfilled); +// console.log('onRejected:', onRejected); +// if (onRejected) { +// try { +// // @ts-ignore +// return resolve(onRejected(reason)); +// } catch (ex) { +// return reject(ex); +// } +// } else { +// return reject(reason); +// } +// }, +// ); +// }); +// } + +// /** JSDoc*/ +// public catch(onRejected?: ((reason: any) => never | SyncPromise | never | void)): SyncPromise { +// return this.then(null, onRejected); +// } + +// /** JSDoc */ +// private handle(handler: Handle): void { +// console.log('*******************'); +// console.log('handle called with:'); +// console.log('this:', this); +// console.log('handler.onFulfilled:', handler.onFulfilled); +// console.log('handler.onRejected:', handler.onRejected); +// console.log('*******************'); + +// if (this.state === PromiseState.PENDING) { +// this.handlers.push(handler); +// } else if (this.state === PromiseState.RESOLVED && handler.onFulfilled) { +// handler.onFulfilled(this.value as T); +// } else if (this.state === PromiseState.REJECTED && handler.onRejected) { +// handler.onRejected(this.reason); +// } +// } + +// /** JSDoc */ +// public done( +// onFulfilled?: ((value: T) => T | SyncPromise), +// onRejected?: ((reason: any) => void | SyncPromise), +// ): void { +// // console.log('done called'); +// this.handle({ onFulfilled, onRejected }); +// } + +// /** JSDoc */ +// private triggerHandlers(): void { +// const callbacks = this.handlers; +// this.handlers = []; +// callbacks.forEach(call => { +// this.handle(call); +// }); +// } + +// /** JSDoc */ +// public static resolve(value: T): SyncPromise { +// return new SyncPromise(resolve => resolve(value)); +// } + +// /** JSDoc */ +// public static reject(reason: any): SyncPromise { +// return new SyncPromise((_, reject) => reject(reason)); +// } +// } + +/* +enum PromiseState { + INITIAL, + RESOLVED, + REJECTED, +}; + +class SyncPromise { + private value?: T | any; + private state: PromiseState = PromiseState.INITIAL; + private _callbacks: Array = []; + + constructor(callback: (onResolve, onReject) => void) { + callback((value: T) => { + this.value = value; + this.state = PromiseState.RESOLVED; + this.triggerHandlers(); + }, (err: any) => { + this.value = err; + this.state = PromiseState.REJECTED; + this.triggerHandlers(); + }); + } + + public then(onResolve, onReject): void { + if (this.state != PromiseState.INITIAL) { + this.triggerHandler(onResolve, onReject); + } else { + this._callbacks.push([onResolve, onReject]); + } + } + + private triggerHandler(onResolve, onReject): void { + if (this.state == PromiseState.RESOLVED) { + onResolve(this.value); + } else { + onReject(this.value); + } + } + + private triggerHandlers(): void { + let callbacks = this._callbacks; + this._callbacks = []; + callbacks.forEach((onResolve, onReject) => { + this.triggerHandler(onResolve, onReject); + }); + } + + static resolve(value: T): SyncPromise { + return new SyncPromise((resolve) => { + resolve(value); + }); + } + + static reject(value: any): SyncPromise { + return new SyncPromise((_, reject) => { + reject(value); + }); + } +}*/ diff --git a/packages/utils/test/is.test.ts b/packages/utils/test/is.test.ts index 0fb0ccc9bb89..00e626a1f746 100644 --- a/packages/utils/test/is.test.ts +++ b/packages/utils/test/is.test.ts @@ -1,4 +1,4 @@ -import { isDOMError, isDOMException, isError, isErrorEvent, isPrimitive } from '../src/is'; +import { isDOMError, isDOMException, isError, isErrorEvent, isPrimitive, isThenable } from '../src/is'; import { supportsDOMError, supportsDOMException, supportsErrorEvent } from '../src/supports'; class SentryError extends Error { @@ -71,3 +71,20 @@ describe('isPrimitive()', () => { expect(isPrimitive(new Date())).toEqual(false); }); }); + +describe('isThenable()', () => { + test('should work as advertised', () => { + expect(isThenable(Promise.resolve(true))).toEqual(true); + // expect(isThenable(async () => false)).toEqual(true); + + expect(isThenable(undefined)).toEqual(false); + expect(isThenable(null)).toEqual(false); + expect(isThenable(true)).toEqual(false); + expect(isThenable('foo')).toEqual(false); + expect(isThenable(42)).toEqual(false); + expect(isThenable({})).toEqual(false); + expect(isThenable([])).toEqual(false); + expect(isThenable(new Error('foo'))).toEqual(false); + expect(isThenable(new Date())).toEqual(false); + }); +}); diff --git a/packages/utils/test/quickpromise.test.ts b/packages/utils/test/quickpromise.test.ts deleted file mode 100644 index f2a989529a87..000000000000 --- a/packages/utils/test/quickpromise.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { QuickPromise } from '../src/quickpromise'; - -describe('QuickPromise', () => { - test('with callback', () => { - expect.assertions(1); - - new QuickPromise(done => done(42)).done(val => { - expect(val).toEqual(42); - }); - }); - - test('with new Promise', finished => { - expect.assertions(2); - - new QuickPromise(done => { - new Promise(resolve => { - expect(true).toBe(true); - resolve(41); - }).then(done); - }).done(val => { - expect(val).toEqual(41); - finished(); - }); - }); - - test('with setTimeout', finished => { - jest.useFakeTimers(); - expect.assertions(1); - - new QuickPromise(done => { - setTimeout(() => done(41), 10); - jest.runAllTimers(); - }).done(val => { - expect(val).toEqual(41); - finished(); - }); - }); - - test('calling the callback immediatly', () => { - expect.assertions(1); - - let foo: number = 1; - - new QuickPromise(_ => { - foo = 2; - }); - - expect(foo).toEqual(2); - }); - - test('calling the callback not immediatly', () => { - jest.useFakeTimers(); - expect.assertions(5); - - const qp = new QuickPromise(done => setTimeout(() => done(2))); - qp.done(_ => { - // - }) - .done(_ => { - // - }) - .done(_ => { - // - }); - expect((qp as any).callbacks).toHaveLength(3); - expect(qp).not.toHaveProperty('result'); - qp.done(result => { - expect(result).toEqual(2); - }); - expect((qp as any).callbacks).toHaveLength(4); - jest.runAllTimers(); - expect(qp).toHaveProperty('result'); - }); - - test('multi then', () => { - expect.assertions(3); - - new QuickPromise(done => done(2)) - .done(result => { - expect(result).toEqual(2); - }) - .done(result => { - expect(result).toEqual(2); - }) - .done(result => { - expect(result).toEqual(2); - }); - }); -}); diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts new file mode 100644 index 000000000000..4b6545602160 --- /dev/null +++ b/packages/utils/test/syncpromise.test.ts @@ -0,0 +1,142 @@ +import { SyncPromise } from '../src/syncpromise'; + +describe('SyncPromise', () => { + test('simple', () => { + expect.assertions(1); + + return new SyncPromise(resolve => resolve(42)).then(val => { + expect(val).toBe(42); + }); + }); + + test('using new Promise internally', () => { + expect.assertions(2); + + return new SyncPromise(done => { + new Promise(resolve => { + expect(true).toBe(true); + resolve(41); + }).then(done); + }).then(val => { + expect(val).toEqual(41); + }); + }); + + test('with setTimeout', () => { + jest.useFakeTimers(); + expect.assertions(1); + + return new SyncPromise(resolve => { + setTimeout(() => resolve(12), 10); + jest.runAllTimers(); + }).then(val => { + expect(val).toEqual(12); + }); + }); + + test('calling the callback immediatly', () => { + expect.assertions(1); + + let foo: number = 1; + + new SyncPromise(_ => { + foo = 2; + }); + + expect(foo).toEqual(2); + }); + + test('calling the callback not immediatly', () => { + jest.useFakeTimers(); + expect.assertions(4); + + const qp = new SyncPromise(resolve => setTimeout(() => resolve(2))); + qp.then(value => { + expect(value).toEqual(2); + }); + expect(qp).not.toHaveProperty('value'); + qp.then(value => { + expect(value).toEqual(2); + }); + jest.runAllTimers(); + expect(qp).toHaveProperty('value'); + }); + + test('multiple then returning undefined', () => { + expect.assertions(3); + + return new SyncPromise(resolve => resolve(2)) + .then(result => { + expect(result).toEqual(2); + }) + .then(result => { + expect(result).toBeUndefined(); + }) + .then(result => { + expect(result).toBeUndefined(); + }); + }); + + test('multiple then returning different values', () => { + expect.assertions(3); + + return new SyncPromise(resolve => resolve(2)) + .then(result => { + expect(result).toEqual(2); + return 3; + }) + .then(result => { + expect(result).toEqual(3); + return 4; + }) + .then(result => { + expect(result).toEqual(4); + }); + }); + + test('multiple then returning different SyncPromise', () => { + expect.assertions(2); + + return new SyncPromise(resolve => resolve(2)) + .then(result => { + expect(result).toEqual(2); + return new SyncPromise(resolve2 => resolve2('yo')); + }) + .then(result => { + expect(result).toEqual('yo'); + }); + }); + + test('reject immediatly and do not call then', () => { + expect.assertions(1); + + return new SyncPromise((_, reject) => reject('test')) + .then(_ => { + expect(true).toBeFalsy(); + }) + .catch(reason => { + expect(reason).toBe('test'); + }); + }); + + test('reject', () => { + expect.assertions(1); + + return new SyncPromise((_, reject) => reject('test')).catch(reason => { + expect(reason).toBe('test'); + }); + }); + + test('rejecting after first then', () => { + expect.assertions(2); + + return new SyncPromise(resolve => resolve(2)) + .then(value => { + expect(value).toEqual(2); + return SyncPromise.reject('wat'); + }) + .catch(reason => { + expect(reason).toBe('wat'); + }); + }); +}); From 68ff201b249080a4f80869f31eba138b4200550d Mon Sep 17 00:00:00 2001 From: HazA Date: Mon, 28 Jan 2019 20:02:29 +0100 Subject: [PATCH 06/16] fix: Browser SDK --- packages/browser/src/backend.ts | 105 ++++++++++++++---------- packages/browser/src/client.ts | 3 +- packages/browser/src/index.ts | 2 +- packages/browser/test/backend.test.ts | 8 -- packages/core/src/baseclient.ts | 4 +- packages/node/src/parsers.ts | 2 +- packages/utils/src/syncpromise.ts | 25 +++--- packages/utils/test/syncpromise.test.ts | 9 ++ 8 files changed, 85 insertions(+), 73 deletions(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index ec4605410443..d97a9e2019a5 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -3,6 +3,7 @@ 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'; @@ -70,48 +71,60 @@ export class BrowserBackend extends BaseBackend { /** * @inheritDoc */ - public eventFromException(exception: any, hint?: SentryEventHint): SentryEvent { - let event; - - 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 - event = eventFromStacktrace(computeStackTrace(exception as Error)); - } 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 = this.eventFromMessage(message, undefined, hint); - addExceptionTypeValue(event, message); - } else if (isError(exception as Error)) { - // we have a real Error object, do nothing - event = eventFromStacktrace(computeStackTrace(exception as Error)); - } 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); - 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 = this.eventFromMessage(ex, undefined, hint); - addExceptionTypeValue(event, `${ex}`); - } + public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise { + return new SyncPromise(resolve => { + 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 + event = eventFromStacktrace(computeStackTrace(exception as Error)); + 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; + + this.eventFromMessage(message, undefined, hint).then(messageEvent => { + addExceptionTypeValue(messageEvent, message); + resolve(this.buildEvent(messageEvent)); + }); + } else if (isError(exception as Error)) { + // we have a real Error object, do nothing + event = eventFromStacktrace(computeStackTrace(exception as Error)); + 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); + addExceptionTypeValue(event, 'Custom Object'); + resolve(this.buildEvent(event)); + } 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; + this.eventFromMessage(ex, undefined, hint).then(messageEvent => { + addExceptionTypeValue(messageEvent, `${ex}`); + resolve(this.buildEvent(messageEvent)); + }); + } + }); + } - event = { + /** JSDOC */ + private buildEvent(event: SentryEvent, hint?: SentryEventHint): SentryEvent { + return { ...event, event_id: hint && hint.event_id, exception: { @@ -122,14 +135,16 @@ export class BrowserBackend extends BaseBackend { }, }, }; - - return event; } /** * @inheritDoc */ - public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: SentryEventHint): SentryEvent { + public eventFromMessage( + message: string, + level: Severity = Severity.Info, + hint?: SentryEventHint, + ): SyncPromise { const event: SentryEvent = { event_id: hint && hint.event_id, level, @@ -144,6 +159,6 @@ export class BrowserBackend extends BaseBackend { }; } - return event; + return SyncPromise.resolve(event); } } diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index a74e8d9a826b..fcdc0e33307b 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -2,6 +2,7 @@ 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'; @@ -49,7 +50,7 @@ export class BrowserClient extends BaseClient { /** * @inheritDoc */ - protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SentryEvent | null { + protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SyncPromise { event.platform = event.platform || 'javascript'; event.sdk = { ...event.sdk, diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 3d956aa57027..e32b11a4937e 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -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'; diff --git a/packages/browser/test/backend.test.ts b/packages/browser/test/backend.test.ts index 413cd91fe2e6..996ffb562ade 100644 --- a/packages/browser/test/backend.test.ts +++ b/packages/browser/test/backend.test.ts @@ -1,7 +1,5 @@ import { expect } from 'chai'; -import { Status } from '../src'; import { BrowserBackend } from '../src/backend'; -import { SimpleTransport } from './mocks/simpletransport'; const dsn = 'https://123@sentry.io/42'; const testEvent = { @@ -25,11 +23,5 @@ describe('BrowserBackend', () => { expect(e.message).equal('Cannot sendEvent without a valid Dsn'); } }); - - it('should call sendEvent() on provided transport', async () => { - backend = new BrowserBackend({ dsn, transport: SimpleTransport }); - const status = await backend.sendEvent(testEvent); - expect(status.status).equal(Status.Success); - }); }); }); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 511d89e6171f..9f166d8e3aaf 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -11,8 +11,8 @@ import { import { isPrimitive } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; -import { SyncPromise } from '@sentry/utils/syncpromise'; import { truncate } from '@sentry/utils/string'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { BackendClass } from './basebackend'; import { Dsn } from './dsn'; import { IntegrationIndex, setupIntegrations } from './integration'; @@ -263,7 +263,7 @@ export abstract class BaseClient implement return scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS)); } - return SyncPromise.resolve(prepared); + return SyncPromise.resolve(prepared as SentryEvent | null); } /** diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index 22a9dd672f58..26b7aac1d8c6 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -1,7 +1,7 @@ import { SentryEvent, SentryException, StackFrame } from '@sentry/types'; import { basename, dirname } from '@sentry/utils/path'; -import { SyncPromise } from '@sentry/utils/syncpromise'; import { snipLine } from '@sentry/utils/string'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { readFile } from 'fs'; import { LRUMap } from 'lru_map'; import * as stacktrace from 'stack-trace'; diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index a9b3a04b8062..2e95ce764f95 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -57,22 +57,18 @@ export class SyncPromise { /** JSDoc */ private readonly setResult = (value: T | any, state: States) => { - const set = () => { - if (this.state !== States.PENDING) { - return null; - } - - if (isThenable(value)) { - return (value as Thenable).then(this.resolve, this.reject); - } + if (this.state !== States.PENDING) { + return null; + } - this.value = value; - this.state = state; + if (isThenable(value)) { + return (value as Thenable).then(this.resolve, this.reject); + } - return this.executeHandlers(); - }; + this.value = value; + this.state = state; - set(); + return this.executeHandlers(); }; /** JSDoc */ @@ -96,8 +92,7 @@ export class SyncPromise { /** JSDoc */ private readonly attachHandler = (handler: Handler) => { - this.handlers = [...this.handlers, handler]; - + this.handlers = this.handlers.concat(handler); this.executeHandlers(); }; diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts index 4b6545602160..b898f134eee9 100644 --- a/packages/utils/test/syncpromise.test.ts +++ b/packages/utils/test/syncpromise.test.ts @@ -9,6 +9,15 @@ describe('SyncPromise', () => { }); }); + test('simple static', () => { + expect.assertions(1); + + const p = SyncPromise.resolve(10); + return p.then(val => { + expect(val).toBe(10); + }); + }); + test('using new Promise internally', () => { expect.assertions(2); From cfcf3663c0f45a8d2445ee1375eadb98af535da5 Mon Sep 17 00:00:00 2001 From: HazA Date: Mon, 28 Jan 2019 21:49:35 +0100 Subject: [PATCH 07/16] fix: beforeSend callback --- packages/core/src/baseclient.ts | 138 +++++++++----- packages/core/src/interfaces.ts | 8 +- packages/core/test/lib/base.test.ts | 173 ++++++++++-------- packages/core/test/mocks/backend.ts | 16 +- .../lib => utils/test}/promisebuffer.test.ts | 2 +- 5 files changed, 199 insertions(+), 138 deletions(-) rename packages/{core/test/lib => utils/test}/promisebuffer.test.ts (98%) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 9f166d8e3aaf..45032afc5e11 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -8,7 +8,7 @@ import { SentryEventHint, Severity, } from '@sentry/types'; -import { isPrimitive } from '@sentry/utils/is'; +import { isPrimitive, isThenable } from '@sentry/utils/is'; import { logger } from '@sentry/utils/logger'; import { consoleSandbox, uuid4 } from '@sentry/utils/misc'; import { truncate } from '@sentry/utils/string'; @@ -130,35 +130,48 @@ export abstract class BaseClient implement /** * @inheritDoc */ - public captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string { - const promisedEvent = this.getBackend().eventFromException(exception, hint); - promisedEvent.then(event => { - this.captureEvent(event, hint, scope); - }); - return (hint && hint.event_id) || ''; + public captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string | undefined { + let eventId: string | undefined = hint && hint.event_id; + + this.getBackend() + .eventFromException(exception, hint) + .then(event => { + this.processEvent(event, hint, scope).then(finalEvent => { + eventId = (finalEvent && finalEvent.event_id) || undefined; + }); + }); + + return eventId; } /** * @inheritDoc */ - public captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string { + public captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string | undefined { + let eventId: string | undefined = hint && hint.event_id; + const promisedEvent = isPrimitive(message) ? this.getBackend().eventFromMessage(`${message}`, level, hint) : this.getBackend().eventFromException(message, hint); promisedEvent.then(event => { - this.captureEvent(event, hint, scope); + this.processEvent(event, hint, scope).then(finalEvent => { + eventId = (finalEvent && finalEvent.event_id) || undefined; + }); }); - return (hint && hint.event_id) || ''; + return eventId; } /** * @inheritDoc */ - public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string { - this.processEvent(event, hint, scope); - return (hint && hint.event_id) || ''; + public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string | undefined { + let eventId: string | undefined = hint && hint.event_id; + this.processEvent(event, hint, scope).then(finalEvent => { + eventId = (finalEvent && finalEvent.event_id) || undefined; + }); + return eventId; } /** @@ -282,57 +295,82 @@ export abstract class BaseClient implement * @param send A function to actually send the event. * @param scope A scope containing event metadata. * @param hint May contain additional informartion about the original exception. - * @returns A Promise that resolves with the event status. + * @returns A SyncPromise that resolves with the event. */ - protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): void { + protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): SyncPromise { + const { beforeSend, sampleRate } = this.getOptions(); + if (!this.isEnabled()) { logger.log('SDK not enabled, will not send event.'); - return; + return SyncPromise.resolve(); } - const { beforeSend, sampleRate } = this.getOptions(); - // 1.0 === 100% events are sent // 0.0 === 0% events are sent if (typeof sampleRate === 'number' && Math.random() > sampleRate) { logger.log('This event has been sampled, will not send event.'); - return; + return SyncPromise.resolve(); } - this.prepareEvent(event, scope, hint).then(prepared => { - if (prepared === null) { - logger.log('An event processor returned null, will not send event.'); - return; - } - - let finalEvent: SentryEvent | null = prepared; + return new SyncPromise(resolve => { + this.prepareEvent(event, scope, hint).then(prepared => { + if (prepared === null) { + logger.log('An event processor returned null, will not send event.'); + resolve(null); + return; + } - try { - const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; - if (!isInternalException && beforeSend) { - finalEvent = beforeSend(prepared, hint); - if ((typeof finalEvent as any) === 'undefined') { - logger.error('`beforeSend` method has to return `null` or a valid event.'); + let finalEvent: SentryEvent | null = prepared; + + try { + const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; + if (!isInternalException && beforeSend) { + const beforeSendResult = beforeSend(prepared, hint); + if ((typeof beforeSendResult as any) === 'undefined') { + logger.error('`beforeSend` method has to return `null` or a valid event.'); + } else if (isThenable(beforeSendResult)) { + (beforeSendResult as Promise).then(processedEvent => { + finalEvent = processedEvent; + + if (finalEvent === null) { + logger.log('`beforeSend` returned `null`, will not send event.'); + resolve(null); + return; + } + + // From here on we are really async + this.getBackend().sendEvent(finalEvent); + resolve(finalEvent); + }); + } else { + finalEvent = beforeSendResult as SentryEvent | null; + + if (finalEvent === null) { + logger.log('`beforeSend` returned `null`, will not send event.'); + resolve(null); + return; + } + + // From here on we are really async + this.getBackend().sendEvent(finalEvent); + resolve(finalEvent); + } + } else { + this.getBackend().sendEvent(finalEvent); + resolve(finalEvent); } + } catch (exception) { + this.captureException(exception, { + data: { + __sentry__: true, + }, + originalException: exception as Error, + }); + logger.error('`beforeSend` throw an error, will not send event.'); + resolve(null); + return; } - } catch (exception) { - this.captureException(exception, { - data: { - __sentry__: true, - }, - originalException: exception as Error, - }); - logger.error('`beforeSend` throw an error, will not send event.'); - return; - } - - if (finalEvent === null) { - logger.log('`beforeSend` returned `null`, will not send event.'); - return; - } - - // From here on we are really async - this.getBackend().sendEvent(finalEvent); + }); }); } diff --git a/packages/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 7ca1dd31f754..b28677e6f356 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -112,7 +112,7 @@ export interface Options { * @param hint May contain additional information about the original exception. * @returns A new event that will be sent | null. */ - beforeSend?(event: SentryEvent, hint?: SentryEventHint): SentryEvent | null; + beforeSend?(event: SentryEvent, hint?: SentryEventHint): Promise | SentryEvent | null; /** * A callback invoked when adding a breadcrumb, allowing to optionally modify @@ -164,7 +164,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns The event id */ - captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string; + captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Captures a message event and sends it to Sentry. @@ -175,7 +175,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns The event id */ - captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string; + captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Captures a manually created event and sends it to Sentry. @@ -185,7 +185,7 @@ export interface Client { * @param scope An optional scope containing event metadata. * @returns The event id */ - captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string; + captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Records a new breadcrumb which will be attached to future events. diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index b5c3ea6c88a2..70293b89516e 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,5 +1,5 @@ import { Scope } from '@sentry/hub'; -import { SentryEvent, Status } from '@sentry/types'; +import { SentryEvent } from '@sentry/types'; import { SentryError } from '@sentry/utils/error'; import { TestBackend } from '../mocks/backend'; import { TestClient } from '../mocks/client'; @@ -40,47 +40,55 @@ jest.mock('@sentry/utils/string', () => ({ describe('BaseClient', () => { describe('constructor() / getDsn()', () => { test('returns the Dsn', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); expect(client.getDsn()!.toString()).toBe(PUBLIC_DSN); }); test('allows missing Dsn', () => { + expect.assertions(1); const client = new TestClient({}); expect(client.getDsn()).toBeUndefined(); }); test('throws with invalid Dsn', () => { + expect.assertions(1); expect(() => new TestClient({ dsn: 'abc' })).toThrow(SentryError); }); }); describe('install()', () => { - test('calls install() on Backend', async () => { + test('calls install() on Backend', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); client.install(); expect(TestBackend.instance!.installed).toBe(1); }); - test('calls install() only once', async () => { + test('calls install() only once', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); client.install(); client.install(); expect(TestBackend.instance!.installed).toBe(1); }); - test('resolves the result of install()', async () => { + test('resolves the result of install()', () => { + expect.assertions(1); const client = new TestClient({ mockInstallFailure: true }); const installed = client.install(); expect(installed).toBeFalsy(); }); - test('does not install() when disabled', async () => { + test('does not install() when disabled', () => { + expect.assertions(1); const client = new TestClient({ enabled: false, dsn: PUBLIC_DSN }); client.install(); expect(TestBackend.instance!.installed).toBe(0); }); - test('does not install() without Dsn', async () => { + test('does not install() without Dsn', () => { + expect.assertions(1); const client = new TestClient({}); client.install(); expect(TestBackend.instance!.installed).toBe(0); @@ -89,6 +97,7 @@ describe('BaseClient', () => { describe('getOptions()', () => { test('returns the options', () => { + expect.assertions(1); const options = { dsn: PUBLIC_DSN, test: true }; const client = new TestClient(options); expect(client.getOptions()).toEqual(options); @@ -97,6 +106,7 @@ describe('BaseClient', () => { describe('getBreadcrumbs() / addBreadcrumb()', () => { test('adds a breadcrumb', () => { + expect.assertions(1); const client = new TestClient({}); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); @@ -105,6 +115,7 @@ describe('BaseClient', () => { }); test('adds a timestamp to new breadcrumbs', () => { + expect.assertions(1); const client = new TestClient({}); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); @@ -113,6 +124,7 @@ describe('BaseClient', () => { }); test('discards breadcrumbs beyond maxBreadcrumbs', () => { + expect.assertions(2); const client = new TestClient({ maxBreadcrumbs: 1 }); const scope = new Scope(); scope.addBreadcrumb({ message: 'hello' }, 100); @@ -122,6 +134,7 @@ describe('BaseClient', () => { }); test('allows concurrent updates', () => { + expect.assertions(1); const client = new TestClient({}); const scope = new Scope(); client.addBreadcrumb({ message: 'hello' }, undefined, scope); @@ -130,6 +143,7 @@ describe('BaseClient', () => { }); test('calls beforeBreadcrumb and adds the breadcrumb without any changes', () => { + expect.assertions(1); const beforeBreadcrumb = jest.fn(breadcrumb => breadcrumb); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); @@ -138,6 +152,7 @@ describe('BaseClient', () => { }); test('calls beforeBreadcrumb and uses the new one', () => { + expect.assertions(1); const beforeBreadcrumb = jest.fn(() => ({ message: 'changed' })); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); @@ -146,6 +161,7 @@ describe('BaseClient', () => { }); test('calls beforeBreadcrumb and discards the breadcrumb when returned null', () => { + expect.assertions(1); const beforeBreadcrumb = jest.fn(() => null); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); @@ -154,6 +170,7 @@ describe('BaseClient', () => { }); test('calls beforeBreadcrumb gets an access to a hint as a second argument', () => { + expect.assertions(2); const beforeBreadcrumb = jest.fn((breadcrumb, hint) => ({ ...breadcrumb, data: hint.data })); const client = new TestClient({ beforeBreadcrumb }); const scope = new Scope(); @@ -164,10 +181,10 @@ describe('BaseClient', () => { }); describe('captures', () => { - test('captures and sends exceptions', async () => { + test('captures and sends exceptions', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); - const scope = new Scope(); - await client.captureException(new Error('test exception'), undefined, scope); + client.captureException(new Error('test exception')); expect(TestBackend.instance!.event).toEqual({ event_id: '42', exception: { @@ -181,54 +198,56 @@ describe('BaseClient', () => { }); }); - test('captures and sends messages', async () => { + test('captures and sends messages', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); - const scope = new Scope(); - await client.captureMessage('test message', undefined, undefined, scope); + client.captureMessage('test message'); expect(TestBackend.instance!.event).toEqual({ event_id: '42', - fingerprint: ['test message'], message: 'test message', }); }); - test.only('should call eventFromException if input to captureMessage is not a primitive', async () => { + test('should call eventFromException if input to captureMessage is not a primitive', () => { + expect.assertions(2); const client = new TestClient({ dsn: PUBLIC_DSN }); - const scope = new Scope(); const spy = jest.spyOn(TestBackend.instance!, 'eventFromException'); - await client.captureMessage('foo', undefined, undefined, scope); - await client.captureMessage(null, undefined, undefined, scope); - await client.captureMessage(undefined, undefined, undefined, scope); - await client.captureMessage(1, undefined, undefined, scope); - await client.captureMessage(false, undefined, undefined, scope); + client.captureMessage('foo'); + client.captureMessage(null as any); + client.captureMessage(undefined as any); + client.captureMessage(1 as any); + client.captureMessage(false as any); expect(spy.mock.calls.length).toEqual(0); - await client.captureMessage({}, undefined, undefined, scope); - await client.captureMessage([], undefined, undefined, scope); + client.captureMessage({} as any); + client.captureMessage([] as any); expect(spy.mock.calls.length).toEqual(2); }); }); describe('captureEvent() / prepareEvent()', () => { - test('skips when disabled', async () => { + test('skips when disabled', () => { + expect.assertions(1); const client = new TestClient({ enabled: false, dsn: PUBLIC_DSN }); const scope = new Scope(); - await client.captureEvent({}, undefined, scope); + client.captureEvent({}, undefined, scope); expect(TestBackend.instance!.event).toBeUndefined(); }); - test('skips without a Dsn', async () => { + test('skips without a Dsn', () => { + expect.assertions(1); const client = new TestClient({}); const scope = new Scope(); - await client.captureEvent({}, undefined, scope); + client.captureEvent({}, undefined, scope); expect(TestBackend.instance!.event).toBeUndefined(); }); - test('sends an event', async () => { + test('sends an event', () => { + expect.assertions(2); const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!.message).toBe('message'); expect(TestBackend.instance!.event).toEqual({ event_id: '42', @@ -237,13 +256,14 @@ describe('BaseClient', () => { }); }); - test('adds the configured environment', async () => { + test('adds the configured environment', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN, environment: 'env', }); const scope = new Scope(); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ environment: 'env', event_id: '42', @@ -252,13 +272,14 @@ describe('BaseClient', () => { }); }); - test('adds the configured release', async () => { + test('adds the configured release', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN, release: 'v1.0.0', }); const scope = new Scope(); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ event_id: '42', fingerprint: ['message'], @@ -267,11 +288,12 @@ describe('BaseClient', () => { }); }); - test('adds breadcrumbs', async () => { + test('adds breadcrumbs', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.addBreadcrumb({ message: 'breadcrumb' }, 100); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ breadcrumbs: [{ message: 'breadcrumb' }], event_id: '42', @@ -280,12 +302,13 @@ describe('BaseClient', () => { }); }); - test('limits previously saved breadcrumbs', async () => { + test('limits previously saved breadcrumbs', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN, maxBreadcrumbs: 1 }); const scope = new Scope(); scope.addBreadcrumb({ message: '1' }, 100); scope.addBreadcrumb({ message: '2' }, 200); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ breadcrumbs: [{ message: '2' }], event_id: '42', @@ -294,13 +317,14 @@ describe('BaseClient', () => { }); }); - test('adds context data', async () => { + test('adds context data', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setExtra('b', 'b'); scope.setTag('a', 'a'); scope.setUser({ id: 'user' }); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ event_id: '42', extra: { b: 'b' }, @@ -311,11 +335,12 @@ describe('BaseClient', () => { }); }); - test('adds fingerprint', async () => { + test('adds fingerprint', () => { + expect.assertions(1); const client = new TestClient({ dsn: PUBLIC_DSN }); const scope = new Scope(); scope.setFingerprint(['abcd']); - await client.captureEvent({ message: 'message' }, undefined, scope); + client.captureEvent({ message: 'message' }, undefined, scope); expect(TestBackend.instance!.event!).toEqual({ event_id: '42', fingerprint: ['abcd'], @@ -323,31 +348,32 @@ describe('BaseClient', () => { }); }); - test('calls beforeSend and uses original event without any changes', async () => { + test('calls beforeSend and uses original event without any changes', () => { + expect.assertions(1); const beforeSend = jest.fn(event => event); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); + client.captureEvent({ message: 'hello' }); expect(TestBackend.instance!.event!.message).toBe('hello'); }); - test('calls beforeSend and uses the new one', async () => { - const beforeSend = jest.fn(() => ({ message: 'changed' })); + test('calls beforeSend and uses the new one', () => { + expect.assertions(1); + const beforeSend = jest.fn(() => ({ message: 'changed1' })); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); - expect(TestBackend.instance!.event!.message).toBe('changed'); + client.captureEvent({ message: 'hello' }); + expect(TestBackend.instance!.event!.message).toBe('changed1'); }); - test('calls beforeSend and discards the event', async () => { + test('calls beforeSend and discards the event', () => { + expect.assertions(1); const beforeSend = jest.fn(() => null); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); + client.captureEvent({ message: 'hello' }); expect(TestBackend.instance!.event).toBeUndefined(); }); - test('calls async beforeSend and uses original event without any changes', async () => { + test('calls async beforeSend and uses original event without any changes', () => { + expect.assertions(1); const beforeSend = jest.fn( async event => new Promise(resolve => { @@ -355,57 +381,54 @@ describe('BaseClient', () => { }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); + client.captureEvent({ message: 'hello' }); expect(TestBackend.instance!.event!.message).toBe('hello'); }); - test('calls async beforeSend and uses the new one', async () => { + test('calls async beforeSend and uses the new one', () => { + jest.useFakeTimers(); + expect.assertions(1); const beforeSend = jest.fn( - async () => + () => new Promise(resolve => { - resolve({ message: 'changed' }); + setTimeout(() => resolve({ message: 'changed2' })); }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); - expect(TestBackend.instance!.event!.message).toBe('changed'); + client.captureEvent({ message: 'hello' }); + jest.runAllTimers(); + expect(TestBackend.instance!.event!.message).toBe('changed2'); }); - test('calls async beforeSend and discards the event', async () => { + test('calls async beforeSend and discards the event', () => { + jest.useFakeTimers(); + expect.assertions(1); const beforeSend = jest.fn( - async () => + () => new Promise(resolve => { - resolve(null); + setTimeout(() => resolve(null)); }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); + client.captureEvent({ message: 'hello' }); + jest.runAllTimers(); expect(TestBackend.instance!.event).toBeUndefined(); }); - test('calls beforeSend gets an access to a hint as a second argument', async () => { + test('calls beforeSend gets an access to a hint as a second argument', () => { + expect.assertions(2); const beforeSend = jest.fn((event, hint) => ({ ...event, data: hint.data })); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }, scope); + client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }, scope); expect(TestBackend.instance!.event!.message).toBe('hello'); expect(TestBackend.instance!.event!.data).toBe('someRandomThing'); }); - - test("doesn't do anything with rate limits yet", async () => { - const client = new TestClient({ dsn: PUBLIC_DSN }); - TestBackend.instance!.sendEvent = async () => ({ status: Status.RateLimit }); - const scope = new Scope(); - await client.captureEvent({}, undefined, scope); - // TODO: Test rate limiting queues here - }); }); describe('integrations', () => { test('setup each one of them on ctor', () => { + expect.assertions(2); const client = new TestClient({ dsn: PUBLIC_DSN, integrations: [new TestIntegration()], diff --git a/packages/core/test/mocks/backend.ts b/packages/core/test/mocks/backend.ts index 44dbaa47bdb3..857ab219ba69 100644 --- a/packages/core/test/mocks/backend.ts +++ b/packages/core/test/mocks/backend.ts @@ -1,4 +1,5 @@ -import { SentryEvent, SentryResponse, Status } from '@sentry/types'; +import { SentryEvent } from '@sentry/types'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { BaseBackend } from '../../src/basebackend'; import { Options } from '../../src/interfaces'; @@ -24,8 +25,8 @@ export class TestBackend extends BaseBackend { return !this.options.mockInstallFailure; } - public async eventFromException(exception: any): Promise { - return { + public eventFromException(exception: any): SyncPromise { + return SyncPromise.resolve({ exception: { values: [ { @@ -34,15 +35,14 @@ export class TestBackend extends BaseBackend { }, ], }, - }; + }); } - public async eventFromMessage(message: string): Promise { - return { message }; + public eventFromMessage(message: string): SyncPromise { + return SyncPromise.resolve({ message }); } - public async sendEvent(event: SentryEvent): Promise { + public sendEvent(event: SentryEvent): void { this.event = event; - return { status: Status.Success }; } } diff --git a/packages/core/test/lib/promisebuffer.test.ts b/packages/utils/test/promisebuffer.test.ts similarity index 98% rename from packages/core/test/lib/promisebuffer.test.ts rename to packages/utils/test/promisebuffer.test.ts index da2b699fb119..6dff403b02bd 100644 --- a/packages/core/test/lib/promisebuffer.test.ts +++ b/packages/utils/test/promisebuffer.test.ts @@ -1,4 +1,4 @@ -import { PromiseBuffer } from '../../src/promisebuffer'; +import { PromiseBuffer } from '../src/promisebuffer'; // tslint:disable:no-floating-promises From 53b4744481380ba988168200871048052144bdb1 Mon Sep 17 00:00:00 2001 From: HazA Date: Mon, 28 Jan 2019 22:23:05 +0100 Subject: [PATCH 08/16] fix: Core tests --- packages/core/test/lib/base.test.ts | 37 ++++++++++++++++++++++------- packages/core/test/mocks/backend.ts | 2 ++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 70293b89516e..c75fbd05de32 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -38,6 +38,11 @@ jest.mock('@sentry/utils/string', () => ({ })); describe('BaseClient', () => { + beforeEach(() => { + TestBackend.sendEventCalled = undefined; + TestBackend.instance = undefined; + }); + describe('constructor() / getDsn()', () => { test('returns the Dsn', () => { expect.assertions(1); @@ -372,32 +377,47 @@ describe('BaseClient', () => { expect(TestBackend.instance!.event).toBeUndefined(); }); - test('calls async beforeSend and uses original event without any changes', () => { + test('calls async beforeSend and uses original event without any changes', done => { + jest.useFakeTimers(); expect.assertions(1); const beforeSend = jest.fn( async event => new Promise(resolve => { - resolve(event); + setTimeout(() => resolve(event), 1); }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); client.captureEvent({ message: 'hello' }); - expect(TestBackend.instance!.event!.message).toBe('hello'); + jest.runOnlyPendingTimers(); + TestBackend.sendEventCalled = (event: SentryEvent) => { + expect(event.message).toBe('hello'); + }; + setTimeout(() => { + done(); + }, 5); + jest.runOnlyPendingTimers(); }); - test('calls async beforeSend and uses the new one', () => { + test('calls async beforeSend and uses the new one', done => { jest.useFakeTimers(); expect.assertions(1); const beforeSend = jest.fn( () => new Promise(resolve => { - setTimeout(() => resolve({ message: 'changed2' })); + setTimeout(() => resolve({ message: 'changed2' }), 1); }), ); + const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); client.captureEvent({ message: 'hello' }); - jest.runAllTimers(); - expect(TestBackend.instance!.event!.message).toBe('changed2'); + jest.runOnlyPendingTimers(); + TestBackend.sendEventCalled = (event: SentryEvent) => { + expect(event.message).toBe('changed2'); + }; + setTimeout(() => { + done(); + }, 5); + jest.runOnlyPendingTimers(); }); test('calls async beforeSend and discards the event', () => { @@ -419,8 +439,7 @@ describe('BaseClient', () => { expect.assertions(2); const beforeSend = jest.fn((event, hint) => ({ ...event, data: hint.data })); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); - const scope = new Scope(); - client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }, scope); + client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' }); expect(TestBackend.instance!.event!.message).toBe('hello'); expect(TestBackend.instance!.event!.data).toBe('someRandomThing'); }); diff --git a/packages/core/test/mocks/backend.ts b/packages/core/test/mocks/backend.ts index 857ab219ba69..d91cb411cb4e 100644 --- a/packages/core/test/mocks/backend.ts +++ b/packages/core/test/mocks/backend.ts @@ -10,6 +10,7 @@ export interface TestOptions extends Options { export class TestBackend extends BaseBackend { public static instance?: TestBackend; + public static sendEventCalled?: (event: SentryEvent) => void; public installed: number; public event?: SentryEvent; @@ -44,5 +45,6 @@ export class TestBackend extends BaseBackend { public sendEvent(event: SentryEvent): void { this.event = event; + TestBackend.sendEventCalled && TestBackend.sendEventCalled(event); } } From 70dafe3e0ae26229ed150abbbacaf0b61361f087 Mon Sep 17 00:00:00 2001 From: HazA Date: Mon, 28 Jan 2019 23:13:59 +0100 Subject: [PATCH 09/16] fix: linting and tests --- packages/browser/test/backend.test.ts | 2 +- .../test/integrations/linkederrors.test.ts | 83 ++++++----- packages/core/src/baseclient.ts | 30 ++-- packages/core/test/lib/base.test.ts | 16 +- packages/core/test/mocks/backend.ts | 1 + .../test/integrations/linkederrors.test.ts | 137 +++++++++++------- .../test/integrations/transaction.test.ts | 20 +-- packages/node/test/parsers.test.ts | 46 ++++-- packages/utils/test/syncpromise.test.ts | 48 ++++-- 9 files changed, 232 insertions(+), 151 deletions(-) diff --git a/packages/browser/test/backend.test.ts b/packages/browser/test/backend.test.ts index 996ffb562ade..73b9b22fd376 100644 --- a/packages/browser/test/backend.test.ts +++ b/packages/browser/test/backend.test.ts @@ -18,7 +18,7 @@ describe('BrowserBackend', () => { backend = new BrowserBackend({ dsn }); try { - await backend.sendEvent(testEvent); + backend.sendEvent(testEvent); } catch (e) { expect(e.message).equal('Cannot sendEvent without a valid Dsn'); } diff --git a/packages/browser/test/integrations/linkederrors.test.ts b/packages/browser/test/integrations/linkederrors.test.ts index 26e9e6a8420b..27a6dfdc995b 100644 --- a/packages/browser/test/integrations/linkederrors.test.ts +++ b/packages/browser/test/integrations/linkederrors.test.ts @@ -64,22 +64,23 @@ describe('LinkedErrors', () => { const originalException = one; const backend = new BrowserBackend({}); - const event = await backend.eventFromException(originalException); - const result = linkedErrors.handler(event, { - originalException, + return backend.eventFromException(originalException).then(event => { + const result = linkedErrors.handler(event, { + originalException, + }); + + // It shouldn't include root exception, as it's already processed in the event by the main error handler + expect(result!.exception!.values!.length).equal(3); + expect(result!.exception!.values![0].type).equal('SyntaxError'); + expect(result!.exception!.values![0].value).equal('three'); + expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); + expect(result!.exception!.values![1].type).equal('TypeError'); + expect(result!.exception!.values![1].value).equal('two'); + expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); + expect(result!.exception!.values![2].type).equal('Error'); + expect(result!.exception!.values![2].value).equal('one'); + expect(result!.exception!.values![2].stacktrace).to.have.property('frames'); }); - - // It shouldn't include root exception, as it's already processed in the event by the main error handler - expect(result!.exception!.values!.length).equal(3); - expect(result!.exception!.values![0].type).equal('SyntaxError'); - expect(result!.exception!.values![0].value).equal('three'); - expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); - expect(result!.exception!.values![1].type).equal('TypeError'); - expect(result!.exception!.values![1].value).equal('two'); - expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); - expect(result!.exception!.values![2].type).equal('Error'); - expect(result!.exception!.values![2].value).equal('one'); - expect(result!.exception!.values![2].stacktrace).to.have.property('frames'); }); it('should allow to change walk key', async () => { @@ -97,21 +98,22 @@ describe('LinkedErrors', () => { const originalException = one; const backend = new BrowserBackend({}); - const event = await backend.eventFromException(originalException); - const result = linkedErrors.handler(event, { - originalException, + return backend.eventFromException(originalException).then(event => { + const result = linkedErrors.handler(event, { + originalException, + }); + + expect(result!.exception!.values!.length).equal(3); + expect(result!.exception!.values![0].type).equal('SyntaxError'); + expect(result!.exception!.values![0].value).equal('three'); + expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); + expect(result!.exception!.values![1].type).equal('TypeError'); + expect(result!.exception!.values![1].value).equal('two'); + expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); + expect(result!.exception!.values![2].type).equal('Error'); + expect(result!.exception!.values![2].value).equal('one'); + expect(result!.exception!.values![2].stacktrace).to.have.property('frames'); }); - - expect(result!.exception!.values!.length).equal(3); - expect(result!.exception!.values![0].type).equal('SyntaxError'); - expect(result!.exception!.values![0].value).equal('three'); - expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); - expect(result!.exception!.values![1].type).equal('TypeError'); - expect(result!.exception!.values![1].value).equal('two'); - expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); - expect(result!.exception!.values![2].type).equal('Error'); - expect(result!.exception!.values![2].value).equal('one'); - expect(result!.exception!.values![2].stacktrace).to.have.property('frames'); }); it('should allow to change stack size limit', async () => { @@ -126,18 +128,19 @@ describe('LinkedErrors', () => { two.cause = three; const backend = new BrowserBackend({}); - const event = await backend.eventFromException(one); - const result = linkedErrors.handler(event, { - originalException: one, + return backend.eventFromException(one).then(event => { + const result = linkedErrors.handler(event, { + originalException: one, + }); + + expect(result!.exception!.values!.length).equal(2); + expect(result!.exception!.values![0].type).equal('TypeError'); + expect(result!.exception!.values![0].value).equal('two'); + expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); + expect(result!.exception!.values![1].type).equal('Error'); + expect(result!.exception!.values![1].value).equal('one'); + expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); }); - - expect(result!.exception!.values!.length).equal(2); - expect(result!.exception!.values![0].type).equal('TypeError'); - expect(result!.exception!.values![0].value).equal('two'); - expect(result!.exception!.values![0].stacktrace).to.have.property('frames'); - expect(result!.exception!.values![1].type).equal('Error'); - expect(result!.exception!.values![1].value).equal('one'); - expect(result!.exception!.values![1].stacktrace).to.have.property('frames'); }); }); }); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 45032afc5e11..2e7e877be3a3 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -329,19 +329,23 @@ export abstract class BaseClient implement if ((typeof beforeSendResult as any) === 'undefined') { logger.error('`beforeSend` method has to return `null` or a valid event.'); } else if (isThenable(beforeSendResult)) { - (beforeSendResult as Promise).then(processedEvent => { - finalEvent = processedEvent; - - if (finalEvent === null) { - logger.log('`beforeSend` returned `null`, will not send event.'); - resolve(null); - return; - } - - // From here on we are really async - this.getBackend().sendEvent(finalEvent); - resolve(finalEvent); - }); + (beforeSendResult as Promise) + .then(processedEvent => { + finalEvent = processedEvent; + + if (finalEvent === null) { + logger.log('`beforeSend` returned `null`, will not send event.'); + resolve(null); + return; + } + + // From here on we are really async + this.getBackend().sendEvent(finalEvent); + resolve(finalEvent); + }) + .catch(e => { + logger.error(`beforeSend rejected with ${e}`); + }); } else { finalEvent = beforeSendResult as SentryEvent | null; diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index c75fbd05de32..120fd04b555c 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -383,7 +383,9 @@ describe('BaseClient', () => { const beforeSend = jest.fn( async event => new Promise(resolve => { - setTimeout(() => resolve(event), 1); + setTimeout(() => { + resolve(event); + }, 1); }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); @@ -402,9 +404,11 @@ describe('BaseClient', () => { jest.useFakeTimers(); expect.assertions(1); const beforeSend = jest.fn( - () => + async () => new Promise(resolve => { - setTimeout(() => resolve({ message: 'changed2' }), 1); + setTimeout(() => { + resolve({ message: 'changed2' }); + }, 1); }), ); @@ -424,9 +428,11 @@ describe('BaseClient', () => { jest.useFakeTimers(); expect.assertions(1); const beforeSend = jest.fn( - () => + async () => new Promise(resolve => { - setTimeout(() => resolve(null)); + setTimeout(() => { + resolve(null); + }); }), ); const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); diff --git a/packages/core/test/mocks/backend.ts b/packages/core/test/mocks/backend.ts index d91cb411cb4e..b4a741ff8c77 100644 --- a/packages/core/test/mocks/backend.ts +++ b/packages/core/test/mocks/backend.ts @@ -45,6 +45,7 @@ export class TestBackend extends BaseBackend { public sendEvent(event: SentryEvent): void { this.event = event; + // tslint:disable-next-line TestBackend.sendEventCalled && TestBackend.sendEventCalled(event); } } diff --git a/packages/node/test/integrations/linkederrors.test.ts b/packages/node/test/integrations/linkederrors.test.ts index 1f41ea9265d0..525948bc0906 100644 --- a/packages/node/test/integrations/linkederrors.test.ts +++ b/packages/node/test/integrations/linkederrors.test.ts @@ -1,4 +1,4 @@ -import { NodeBackend } from '../../src'; +import { NodeBackend, SentryEvent } from '../../src'; import { LinkedErrors } from '../../src/integrations/linkederrors'; let linkedErrors: LinkedErrors; @@ -14,26 +14,37 @@ describe('LinkedErrors', () => { describe('handler', () => { it('should bail out if event doesnt contain exception', async () => { + expect.assertions(2); const spy = jest.spyOn(linkedErrors, 'walkErrorTree'); const event = { message: 'foo', }; - const result = await linkedErrors.handler(event); - expect(spy.mock.calls.length).toEqual(0); - expect(result).toEqual(event); + return linkedErrors.handler(event).then(result => { + expect(spy.mock.calls.length).toEqual(0); + expect(result).toEqual(event); + }); }); it('should bail out if event contains exception, but no hint', async () => { + expect.assertions(2); const spy = jest.spyOn(linkedErrors, 'walkErrorTree'); const one = new Error('originalException'); const backend = new NodeBackend({}); - const event = await backend.eventFromException(one); - const result = await linkedErrors.handler(event); - expect(spy.mock.calls.length).toEqual(0); - expect(result).toEqual(event); + let event: SentryEvent | undefined; + return backend + .eventFromException(one) + .then(eventFromException => { + event = eventFromException; + return linkedErrors.handler(eventFromException); + }) + .then(result => { + expect(spy.mock.calls.length).toEqual(0); + expect(result).toEqual(event); + }); }); it('should call walkErrorTree if event contains exception and hint with originalException', async () => { + expect.assertions(1); const spy = jest.spyOn(linkedErrors, 'walkErrorTree').mockImplementation( async () => new Promise<[]>(resolve => { @@ -42,14 +53,19 @@ describe('LinkedErrors', () => { ); const one = new Error('originalException'); const backend = new NodeBackend({}); - const event = await backend.eventFromException(one); - await linkedErrors.handler(event, { - originalException: one, - }); - expect(spy.mock.calls.length).toEqual(1); + return backend.eventFromException(one).then(event => + linkedErrors + .handler(event, { + originalException: one, + }) + .then(_ => { + expect(spy.mock.calls.length).toEqual(1); + }), + ); }); it('should recursively walk error to find linked exceptions and assign them to the event', async () => { + expect.assertions(10); const one: ExtendedError = new Error('one'); const two: ExtendedError = new TypeError('two'); const three: ExtendedError = new SyntaxError('three'); @@ -57,24 +73,28 @@ describe('LinkedErrors', () => { two.cause = three; const backend = new NodeBackend({}); - const event = await backend.eventFromException(one); - const result = await linkedErrors.handler(event, { - originalException: one, - }); - - expect(result!.exception!.values!.length).toEqual(3); - expect(result!.exception!.values![0].type).toEqual('SyntaxError'); - expect(result!.exception!.values![0].value).toEqual('three'); - expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); - expect(result!.exception!.values![1].type).toEqual('TypeError'); - expect(result!.exception!.values![1].value).toEqual('two'); - expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); - expect(result!.exception!.values![2].type).toEqual('Error'); - expect(result!.exception!.values![2].value).toEqual('one'); - expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames'); + return backend.eventFromException(one).then(event => + linkedErrors + .handler(event, { + originalException: one, + }) + .then(result => { + expect(result!.exception!.values!.length).toEqual(3); + expect(result!.exception!.values![0].type).toEqual('SyntaxError'); + expect(result!.exception!.values![0].value).toEqual('three'); + expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); + expect(result!.exception!.values![1].type).toEqual('TypeError'); + expect(result!.exception!.values![1].value).toEqual('two'); + expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); + expect(result!.exception!.values![2].type).toEqual('Error'); + expect(result!.exception!.values![2].value).toEqual('one'); + expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames'); + }), + ); }); it('should allow to change walk key', async () => { + expect.assertions(10); linkedErrors = new LinkedErrors({ key: 'reason', }); @@ -86,24 +106,28 @@ describe('LinkedErrors', () => { two.reason = three; const backend = new NodeBackend({}); - const event = await backend.eventFromException(one); - const result = await linkedErrors.handler(event, { - originalException: one, - }); - - expect(result!.exception!.values!.length).toEqual(3); - expect(result!.exception!.values![0].type).toEqual('SyntaxError'); - expect(result!.exception!.values![0].value).toEqual('three'); - expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); - expect(result!.exception!.values![1].type).toEqual('TypeError'); - expect(result!.exception!.values![1].value).toEqual('two'); - expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); - expect(result!.exception!.values![2].type).toEqual('Error'); - expect(result!.exception!.values![2].value).toEqual('one'); - expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames'); + return backend.eventFromException(one).then(event => + linkedErrors + .handler(event, { + originalException: one, + }) + .then(result => { + expect(result!.exception!.values!.length).toEqual(3); + expect(result!.exception!.values![0].type).toEqual('SyntaxError'); + expect(result!.exception!.values![0].value).toEqual('three'); + expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); + expect(result!.exception!.values![1].type).toEqual('TypeError'); + expect(result!.exception!.values![1].value).toEqual('two'); + expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); + expect(result!.exception!.values![2].type).toEqual('Error'); + expect(result!.exception!.values![2].value).toEqual('one'); + expect(result!.exception!.values![2].stacktrace).toHaveProperty('frames'); + }), + ); }); it('should allow to change stack size limit', async () => { + expect.assertions(7); linkedErrors = new LinkedErrors({ limit: 2, }); @@ -115,18 +139,21 @@ describe('LinkedErrors', () => { two.cause = three; const backend = new NodeBackend({}); - const event = await backend.eventFromException(one); - const result = await linkedErrors.handler(event, { - originalException: one, - }); - - expect(result!.exception!.values!.length).toEqual(2); - expect(result!.exception!.values![0].type).toEqual('TypeError'); - expect(result!.exception!.values![0].value).toEqual('two'); - expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); - expect(result!.exception!.values![1].type).toEqual('Error'); - expect(result!.exception!.values![1].value).toEqual('one'); - expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); + return backend.eventFromException(one).then(event => + linkedErrors + .handler(event, { + originalException: one, + }) + .then(result => { + expect(result!.exception!.values.length).toEqual(2); + expect(result!.exception!.values![0].type).toEqual('TypeError'); + expect(result!.exception!.values![0].value).toEqual('two'); + expect(result!.exception!.values![0].stacktrace).toHaveProperty('frames'); + expect(result!.exception!.values![1].type).toEqual('Error'); + expect(result!.exception!.values![1].value).toEqual('one'); + expect(result!.exception!.values![1].stacktrace).toHaveProperty('frames'); + }), + ); }); }); }); diff --git a/packages/node/test/integrations/transaction.test.ts b/packages/node/test/integrations/transaction.test.ts index 8341fd6e496a..e59f724bdd51 100644 --- a/packages/node/test/integrations/transaction.test.ts +++ b/packages/node/test/integrations/transaction.test.ts @@ -4,8 +4,8 @@ const transaction: Transaction = new Transaction(); describe('Transaction', () => { describe('extracts info from module/function of the first `in_app` frame', () => { - it('using module only', async () => { - const event = await transaction.process({ + it('using module only', () => { + const event = transaction.process({ exception: { values: [ { @@ -30,8 +30,8 @@ describe('Transaction', () => { expect(event.transaction).toEqual('Qux/?'); }); - it('using function only', async () => { - const event = await transaction.process({ + it('using function only', () => { + const event = transaction.process({ exception: { values: [ { @@ -56,8 +56,8 @@ describe('Transaction', () => { expect(event.transaction).toEqual('?/Baz'); }); - it('using module and function', async () => { - const event = await transaction.process({ + it('using module and function', () => { + const event = transaction.process({ exception: { values: [ { @@ -84,8 +84,8 @@ describe('Transaction', () => { expect(event.transaction).toEqual('Foo/Bar'); }); - it('using default', async () => { - const event = await transaction.process({ + it('using default', () => { + const event = transaction.process({ exception: { values: [ { @@ -108,8 +108,8 @@ describe('Transaction', () => { expect(event.transaction).toEqual(''); }); - it('no value with no `in_app` frame', async () => { - const event = await transaction.process({ + it('no value with no `in_app` frame', () => { + const event = transaction.process({ exception: { values: [ { diff --git a/packages/node/test/parsers.test.ts b/packages/node/test/parsers.test.ts index 1337413cc53f..8df4d9b4a70c 100644 --- a/packages/node/test/parsers.test.ts +++ b/packages/node/test/parsers.test.ts @@ -19,27 +19,41 @@ describe('parsers.ts', () => { }); describe('lru file cache', () => { - test('parseStack with same file', async () => { - await Parsers.parseStack(frames); - const mockCalls = spy.mock.calls.length; - await Parsers.parseStack(frames); - // Calls to readFile shouldn't increase if there isn't a new error - expect(spy).toHaveBeenCalledTimes(mockCalls); + test('parseStack with same file', done => { + expect.assertions(1); + let mockCalls = 0; + Parsers.parseStack(frames).then(_ => { + mockCalls = spy.mock.calls.length; + Parsers.parseStack(frames).then(_1 => { + // Calls to readFile shouldn't increase if there isn't a new error + expect(spy).toHaveBeenCalledTimes(mockCalls); + done(); + }); + }); }); - test('parseStack with adding different file', async () => { - await Parsers.parseStack(frames); - const mockCalls = spy.mock.calls.length; - await Parsers.parseStack(stacktrace.parse(getError())); - const newErrorCalls = spy.mock.calls.length; - expect(newErrorCalls).toBeGreaterThan(mockCalls); - await Parsers.parseStack(stacktrace.parse(getError())); - expect(spy).toHaveBeenCalledTimes(newErrorCalls); + test('parseStack with adding different file', done => { + expect.assertions(2); + let mockCalls = 0; + let newErrorCalls = 0; + Parsers.parseStack(frames).then(_ => { + mockCalls = spy.mock.calls.length; + Parsers.parseStack(stacktrace.parse(getError())).then(_1 => { + newErrorCalls = spy.mock.calls.length; + expect(newErrorCalls).toBeGreaterThan(mockCalls); + Parsers.parseStack(stacktrace.parse(getError())).then(_2 => { + expect(spy).toHaveBeenCalledTimes(newErrorCalls); + done(); + }); + }); + }); }); }); test('parseStack with no context', async () => { - await Parsers.parseStack(frames, { frameContextLines: 0 }); - expect(spy).toHaveBeenCalledTimes(0); + expect.assertions(1); + return Parsers.parseStack(frames, { frameContextLines: 0 }).then(_ => { + expect(spy).toHaveBeenCalledTimes(0); + }); }); }); diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts index b898f134eee9..f6d2ae7c8af0 100644 --- a/packages/utils/test/syncpromise.test.ts +++ b/packages/utils/test/syncpromise.test.ts @@ -4,7 +4,9 @@ describe('SyncPromise', () => { test('simple', () => { expect.assertions(1); - return new SyncPromise(resolve => resolve(42)).then(val => { + return new SyncPromise(resolve => { + resolve(42); + }).then(val => { expect(val).toBe(42); }); }); @@ -25,7 +27,11 @@ describe('SyncPromise', () => { new Promise(resolve => { expect(true).toBe(true); resolve(41); - }).then(done); + }) + .then(done) + .catch(_ => { + // + }); }).then(val => { expect(val).toEqual(41); }); @@ -36,7 +42,9 @@ describe('SyncPromise', () => { expect.assertions(1); return new SyncPromise(resolve => { - setTimeout(() => resolve(12), 10); + setTimeout(() => { + resolve(12); + }, 10); jest.runAllTimers(); }).then(val => { expect(val).toEqual(12); @@ -59,7 +67,11 @@ describe('SyncPromise', () => { jest.useFakeTimers(); expect.assertions(4); - const qp = new SyncPromise(resolve => setTimeout(() => resolve(2))); + const qp = new SyncPromise(resolve => + setTimeout(() => { + resolve(2); + }), + ); qp.then(value => { expect(value).toEqual(2); }); @@ -74,7 +86,9 @@ describe('SyncPromise', () => { test('multiple then returning undefined', () => { expect.assertions(3); - return new SyncPromise(resolve => resolve(2)) + return new SyncPromise(resolve => { + resolve(2); + }) .then(result => { expect(result).toEqual(2); }) @@ -89,7 +103,9 @@ describe('SyncPromise', () => { test('multiple then returning different values', () => { expect.assertions(3); - return new SyncPromise(resolve => resolve(2)) + return new SyncPromise(resolve => { + resolve(2); + }) .then(result => { expect(result).toEqual(2); return 3; @@ -106,10 +122,14 @@ describe('SyncPromise', () => { test('multiple then returning different SyncPromise', () => { expect.assertions(2); - return new SyncPromise(resolve => resolve(2)) + return new SyncPromise(resolve => { + resolve(2); + }) .then(result => { expect(result).toEqual(2); - return new SyncPromise(resolve2 => resolve2('yo')); + return new SyncPromise(resolve2 => { + resolve2('yo'); + }); }) .then(result => { expect(result).toEqual('yo'); @@ -119,7 +139,9 @@ describe('SyncPromise', () => { test('reject immediatly and do not call then', () => { expect.assertions(1); - return new SyncPromise((_, reject) => reject('test')) + return new SyncPromise((_, reject) => { + reject('test'); + }) .then(_ => { expect(true).toBeFalsy(); }) @@ -131,7 +153,9 @@ describe('SyncPromise', () => { test('reject', () => { expect.assertions(1); - return new SyncPromise((_, reject) => reject('test')).catch(reason => { + return new SyncPromise((_, reject) => { + reject('test'); + }).catch(reason => { expect(reason).toBe('test'); }); }); @@ -139,7 +163,9 @@ describe('SyncPromise', () => { test('rejecting after first then', () => { expect.assertions(2); - return new SyncPromise(resolve => resolve(2)) + return new SyncPromise(resolve => { + resolve(2); + }) .then(value => { expect(value).toEqual(2); return SyncPromise.reject('wat'); From cb00da2cb13bf258be6cc30d03d9c07db4706b25 Mon Sep 17 00:00:00 2001 From: HazA Date: Tue, 29 Jan 2019 00:16:36 +0100 Subject: [PATCH 10/16] fix: browser npm package + add error for manual tests --- packages/browser/rollup.config.js | 2 +- packages/browser/test/manual/npm-build.js | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/browser/rollup.config.js b/packages/browser/rollup.config.js index 57d039b440dd..41227d9efdae 100644 --- a/packages/browser/rollup.config.js +++ b/packages/browser/rollup.config.js @@ -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', diff --git a/packages/browser/test/manual/npm-build.js b/packages/browser/test/manual/npm-build.js index 7b2c1f46da7c..c06f00c4f254 100644 --- a/packages/browser/test/manual/npm-build.js +++ b/packages/browser/test/manual/npm-build.js @@ -39,7 +39,10 @@ function runTests() { const bundlePath = path.join(__dirname, 'tmp.js'); const { window } = new JSDOM(``, { runScripts: 'dangerously' }); - window.onerror = function() { + window.onerror = function(error) { + console.error('ERROR thrown in manual test:'); + console.error(error); + console.error('------------------'); process.exit(1); }; From 2b84aedd975e25fbaeafbee007b65ec0257b3ff6 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Thu, 31 Jan 2019 12:02:23 +0100 Subject: [PATCH 11/16] meta: Remove deprecations --- packages/core/src/integration.ts | 20 +++------------- .../core/src/integrations/sdkinformation.ts | 24 ------------------- packages/node/src/index.ts | 2 +- packages/types/src/index.ts | 9 +------ packages/utils/test/syncpromise.test.ts | 14 +++++++++++ 5 files changed, 19 insertions(+), 50 deletions(-) delete mode 100644 packages/core/src/integrations/sdkinformation.ts diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 37317986e162..8f38d425c52a 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -47,25 +47,11 @@ export function getIntegrationsToSetup(options: Options): Integration[] { } /** Setup given integration */ -export function setupIntegration(integration: Integration, options: Options): void { +export function setupIntegration(integration: Integration): void { if (installedIntegrations.indexOf(getIntegrationName(integration)) !== -1) { return; } - - try { - integration.setupOnce(); - } catch (_Oo) { - /** @deprecated */ - // TODO: Remove in v5 - - // tslint:disable:deprecation - if (integration.install) { - logger.warn(`Integration ${getIntegrationName(integration)}: The install method is deprecated. Use "setupOnce".`); - integration.install(options); - } - // tslint:enable:deprecation - } - + integration.setupOnce(); installedIntegrations.push(getIntegrationName(integration)); logger.log(`Integration installed: ${getIntegrationName(integration)}`); } @@ -80,7 +66,7 @@ export function setupIntegrations(options: O): IntegrationInd const integrations: IntegrationIndex = {}; getIntegrationsToSetup(options).forEach(integration => { integrations[getIntegrationName(integration)] = integration; - setupIntegration(integration, options); + setupIntegration(integration); }); return integrations; } diff --git a/packages/core/src/integrations/sdkinformation.ts b/packages/core/src/integrations/sdkinformation.ts deleted file mode 100644 index 58ce510a5651..000000000000 --- a/packages/core/src/integrations/sdkinformation.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { Integration } from '@sentry/types'; -import { logger } from '@sentry/utils/logger'; - -/** - * @deprecated - * This file can be safely removed in the next major bump - */ - -/** Adds SDK info to an event. */ -export class SDKInformation implements Integration { - /** - * @inheritDoc - */ - public name: string = 'SDKInformation'; - - /** - * @inheritDoc - */ - public setupOnce(): void { - logger.warn( - "SDKInformation Integration is deprecated and can be safely removed. It's functionality has been merged into the SDK's core.", - ); - } -} diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 458f405e13de..b7ad4725be62 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -27,7 +27,7 @@ export { withScope, } from '@sentry/core'; -export { NodeBackend, NodeOptions } from './backend'; +export { NodeOptions } from './backend'; export { NodeClient } from './client'; export { defaultIntegrations, init } from './sdk'; export { SDK_NAME, SDK_VERSION } from './version'; diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 8f7d84095dd9..33655e528980 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -199,12 +199,9 @@ export interface Integration { * Returns {@link IntegrationClass.id} */ name: string; - // TODO: Remove with v5 - /** @deprecated */ - install?(options?: object): void; // This takes no options on purpose, options should be passed in the constructor - setupOnce(): void; // TODO: make not optional + setupOnce(): void; } /** Integration Class Interface */ @@ -253,10 +250,6 @@ export interface Transport { * @param timeout Number time in ms to wait until the buffer is drained. */ close(timeout?: number): Promise; - - // TODO: Remove with v5 - /** @deprecated Implement sendEvent instead */ - captureEvent?(event: SentryEvent): Promise; } /** JSDoc */ diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts index f6d2ae7c8af0..c110f93ccfde 100644 --- a/packages/utils/test/syncpromise.test.ts +++ b/packages/utils/test/syncpromise.test.ts @@ -11,6 +11,20 @@ describe('SyncPromise', () => { }); }); + test('simple chaining', () => { + expect.assertions(1); + + return new SyncPromise(resolve => { + resolve(42); + }) + .then(_ => SyncPromise.resolve('a')) + .then(_ => SyncPromise.resolve(0.1)) + .then(_ => SyncPromise.resolve(false)) + .then(val => { + expect(val).toBe(false); + }); + }); + test('simple static', () => { expect.assertions(1); From 8ea2ec75383dfef715c73d9756c110fce3160bd8 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 4 Feb 2019 13:57:40 +0100 Subject: [PATCH 12/16] fix: Typedocs and public exposed functions --- packages/browser/src/backend.ts | 5 +++- .../browser/src/integrations/breadcrumbs.ts | 4 ++- packages/browser/src/integrations/helpers.ts | 12 ++++++-- packages/browser/src/parsers.ts | 14 +++++++-- packages/browser/src/sdk.ts | 26 ++++++++++++----- packages/browser/src/tracekit.ts | 6 ++++ packages/core/README.md | 2 ++ packages/core/src/integrations/index.ts | 1 - packages/hub/src/hub.ts | 7 ++++- packages/hub/src/index.ts | 2 +- packages/hub/src/interfaces.ts | 10 +++++-- packages/hub/src/scope.ts | 7 +++++ packages/node/src/backend.ts | 5 +++- packages/node/src/handlers.ts | 15 ++++++++-- packages/node/src/index.ts | 3 +- .../src/integrations/onuncaughtexception.ts | 4 ++- packages/node/src/parsers.ts | 29 +++++++++++++------ packages/node/src/sdk.ts | 15 ++++++++-- packages/node/src/transports/base.ts | 5 +++- packages/node/test/index.test.ts | 2 +- .../test/integrations/linkederrors.test.ts | 3 +- packages/node/test/parsers.test.ts | 2 +- packages/typescript/tslint.json | 14 ++++----- 23 files changed, 145 insertions(+), 48 deletions(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index d97a9e2019a5..b56362278fc1 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -28,7 +28,10 @@ export interface BrowserOptions extends Options { whitelistUrls?: Array; } -/** The Sentry Browser SDK Backend. */ +/** + * The Sentry Browser SDK Backend. + * @hidden + */ export class BrowserBackend extends BaseBackend { /** * @inheritDoc diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts index 12d53c1f2ac9..e6e1c56ed7cc 100644 --- a/packages/browser/src/integrations/breadcrumbs.ts +++ b/packages/browser/src/integrations/breadcrumbs.ts @@ -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__?: { diff --git a/packages/browser/src/integrations/helpers.ts b/packages/browser/src/integrations/helpers.ts index adbede9786be..913fc6100990 100644 --- a/packages/browser/src/integrations/helpers.ts +++ b/packages/browser/src/integrations/helpers.ts @@ -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; @@ -28,6 +33,7 @@ export function ignoreNextOnError(): void { * * @param fn A function to wrap. * @returns The wrapped function. + * @hidden */ export function wrap( fn: SentryWrappedFunction, @@ -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) => { @@ -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 diff --git a/packages/browser/src/parsers.ts b/packages/browser/src/parsers.ts index 04c6ae133265..3be0460277a0 100644 --- a/packages/browser/src/parsers.ts +++ b/packages/browser/src/parsers.ts @@ -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); @@ -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 = { @@ -52,7 +55,9 @@ export function eventFromPlainObject(exception: {}, syntheticException: Error | return event; } -/** JSDoc */ +/** + * @hidden + */ export function eventFromStacktrace(stacktrace: TraceKitStackTrace): SentryEvent { const exception = exceptionFromStacktrace(stacktrace); @@ -63,7 +68,9 @@ export function eventFromStacktrace(stacktrace: TraceKitStackTrace): SentryEvent }; } -/** JSDoc */ +/** + * @hidden + */ export function prepareFramesForEvent(stack: TraceKitStackFrame[]): StackFrame[] { if (!stack || !stack.length) { return []; @@ -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 || {}; diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 4705d469ac88..bb4fcf4f3b6e 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -4,17 +4,13 @@ import { BrowserClient, ReportDialogOptions } from './client'; import { Breadcrumbs, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations'; export const defaultIntegrations = [ - // Common new CoreIntegrations.Dedupe(), new CoreIntegrations.InboundFilters(), new CoreIntegrations.FunctionToString(), new CoreIntegrations.ExtraErrorData(), - // Native Wrappers new TryCatch(), new Breadcrumbs(), - // Global Handlers new GlobalHandlers(), - // Misc new LinkedErrors(), new UserAgent(), ]; @@ -27,29 +23,42 @@ export const defaultIntegrations = [ * the provided methods. * * @example + * + * ``` + * * import { init } from '@sentry/browser'; * * init({ * dsn: '__DSN__', * // ... * }); + * ``` * * @example + * ``` + * * import { configureScope } from '@sentry/browser'; * configureScope((scope: Scope) => { * scope.setExtra({ battery: 0.7 }); * scope.setTag({ user_mode: 'admin' }); * scope.setUser({ id: '4711' }); * }); + * ``` * * @example + * ``` + * * import { addBreadcrumb } from '@sentry/browser'; * addBreadcrumb({ * message: 'My Breadcrumb', * // ... * }); + * ``` * * @example + * + * ``` + * * import * as Sentry from '@sentry/browser'; * Sentry.captureMessage('Hello, world!'); * Sentry.captureException(new Error('Good bye')); @@ -59,8 +68,9 @@ export const defaultIntegrations = [ * // ... * ], * }); + * ``` * - * @see BrowserOptions for documentation on configuration options. + * @see {@link BrowserOptions} for documentation on configuration options. */ export function init(options: BrowserOptions = {}): void { if (options.defaultIntegrations === undefined) { @@ -91,14 +101,16 @@ export function lastEventId(): string | undefined { } /** - * This function is here to be API compatible with the loader + * This function is here to be API compatible with the loader. + * @hidden */ export function forceLoad(): void { // Noop } /** - * This function is here to be API compatible with the loader + * This function is here to be API compatible with the loader. + * @hidden */ export function onLoad(callback: () => void): void { callback(); diff --git a/packages/browser/src/tracekit.ts b/packages/browser/src/tracekit.ts index 02641369a286..4176df2c8d9b 100644 --- a/packages/browser/src/tracekit.ts +++ b/packages/browser/src/tracekit.ts @@ -3,6 +3,9 @@ import { isUndefined, isError, isErrorEvent } from '@sentry/utils/is'; import { getGlobalObject } from '@sentry/utils/misc'; +/** + * @hidden + */ export interface StackFrame { url: string; func: string; @@ -12,6 +15,9 @@ export interface StackFrame { context: string[]; } +/** + * @hidden + */ export interface StackTrace { /** * Known modes: callers, failed, multiline, onerror, stack, stacktrace diff --git a/packages/core/README.md b/packages/core/README.md index 84ca4586b865..b228fc669a80 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -21,3 +21,5 @@ This package contains interface definitions, base classes and utilities for building Sentry JavaScript SDKs, like `@sentry/node` or `@sentry/browser`. + +Please consider all classes and exported functions and interfaces `internal`. diff --git a/packages/core/src/integrations/index.ts b/packages/core/src/integrations/index.ts index 64f1fc91de67..24708d2c64f9 100644 --- a/packages/core/src/integrations/index.ts +++ b/packages/core/src/integrations/index.ts @@ -1,6 +1,5 @@ export { Dedupe } from './dedupe'; export { FunctionToString } from './functiontostring'; -export { SDKInformation } from './sdkinformation'; export { InboundFilters } from './inboundfilters'; export { ExtraErrorData } from './extraerrordata'; diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index f68238221f38..ae49978cc283 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -27,6 +27,8 @@ declare module 'domain' { * * WARNING: This number should only be incresed when the global interface * changes a and new methods are introduced. + * + * @hidden */ export const API_VERSION = 3; @@ -71,6 +73,8 @@ export class Hub { * * @param version A version number to compare to. * @return True if the given version is newer; otherwise false. + * + * @hidden */ public isOlderThan(version: number): boolean { return this.version < version; @@ -347,7 +351,7 @@ export function getCurrentHub(): Hub { * This will tell whether a carrier has a hub on it or not * @param carrier object */ -export function hasHubOnCarrier(carrier: any): boolean { +function hasHubOnCarrier(carrier: any): boolean { if (carrier && carrier.__SENTRY__ && carrier.__SENTRY__.hub) { return true; } else { @@ -359,6 +363,7 @@ export function hasHubOnCarrier(carrier: any): boolean { * This will create a new {@link Hub} and add to the passed object on * __SENTRY__.hub. * @param carrier object + * @hidden */ export function getHubFromCarrier(carrier: any): Hub { if (carrier && carrier.__SENTRY__ && carrier.__SENTRY__.hub) { diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index 743beb0302ef..6656b1b24df9 100644 --- a/packages/hub/src/index.ts +++ b/packages/hub/src/index.ts @@ -1,3 +1,3 @@ export { Carrier, Layer } from './interfaces'; export { addGlobalEventProcessor, Scope } from './scope'; -export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, setHubOnCarrier } from './hub'; +export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, makeMain, setHubOnCarrier } from './hub'; diff --git a/packages/hub/src/interfaces.ts b/packages/hub/src/interfaces.ts index e1c25ba4969b..963cb08240ab 100644 --- a/packages/hub/src/interfaces.ts +++ b/packages/hub/src/interfaces.ts @@ -1,13 +1,19 @@ import { Hub } from './hub'; import { Scope } from './scope'; -/** A layer in the process stack. */ +/** + * A layer in the process stack. + * @hidden + */ export interface Layer { client?: any; scope?: Scope; } -/** An object that contains a hub and maintains a scope stack. */ +/** + * An object that contains a hub and maintains a scope stack. + * @hidden + */ export interface Carrier { __SENTRY__?: { hub?: Hub; diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 4d94b9a0cdeb..c3ac6b819cd2 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -4,6 +4,12 @@ import { getGlobalObject } from '@sentry/utils/misc'; import { assign, safeNormalize } from '@sentry/utils/object'; import { SyncPromise } from '@sentry/utils/syncpromise'; +/** + * Event processors are used to change the event before it will be send. + * We strongly advise to make this function sync. + * Returning a Promise works just fine, just be sure what you are doing. + * Eventprosccing will be stopped until your Promise is resolved. + */ export type EventProcessor = ( event: SentryEvent, hint?: SentryEventHint, @@ -228,6 +234,7 @@ export class Scope { * @param event SentryEvent * @param hint May contain additional informartion about the original exception. * @param maxBreadcrumbs number of max breadcrumbs to merged into event. + * @hidden */ public applyToEvent( event: SentryEvent, diff --git a/packages/node/src/backend.ts b/packages/node/src/backend.ts index 59fa603cfacb..e42af177d50c 100644 --- a/packages/node/src/backend.ts +++ b/packages/node/src/backend.ts @@ -36,7 +36,10 @@ export interface NodeOptions extends Options { frameContextLines?: number; } -/** The Sentry Node SDK Backend. */ +/** + * The Sentry Node SDK Backend. + * @hidden + */ export class NodeBackend extends BaseBackend { /** * @inheritdoc diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 9519eb211ef5..a61f337ae45b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -152,6 +152,7 @@ function extractUserData(req: { [key: string]: any }, keys: boolean | string[]): * @param event Will be mutated and enriched with req data * @param req Request object * @param options object containing flags to enable functionality + * @hidden */ export function parseRequest( event: SentryEvent, @@ -211,7 +212,10 @@ export function parseRequest( return event; } -/** JSDoc */ +/** + * Express compatible request handler. + * @see Exposed as `Handlers.requestHandler` + */ export function requestHandler(options?: { request?: boolean; serverName?: boolean; @@ -253,7 +257,10 @@ function getStatusCodeFromResponse(error: MiddlewareError): number { return statusCode ? parseInt(statusCode as string, 10) : 500; } -/** JSDoc */ +/** + * Express compatible error handler. + * @see Exposed as `Handlers.errorHandler` + */ export function errorHandler(): ( error: MiddlewareError, req: http.IncomingMessage, @@ -276,7 +283,9 @@ export function errorHandler(): ( }; } -/** JSDoc */ +/** + * @hidden + */ export function defaultOnFatalError(error: Error): void { console.error(error && error.stack ? error.stack : error); const options = (getCurrentHub().getClient() as NodeClient).getOptions(); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index b7ad4725be62..afcc20cb18c3 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -35,7 +35,6 @@ export { SDK_NAME, SDK_VERSION } from './version'; import { Integrations as CoreIntegrations } from '@sentry/core'; import * as Handlers from './handlers'; import * as NodeIntegrations from './integrations'; -import * as Parsers from './parsers'; import * as Transports from './transports'; const INTEGRATIONS = { @@ -43,4 +42,4 @@ const INTEGRATIONS = { ...NodeIntegrations, }; -export { INTEGRATIONS as Integrations, Transports, Parsers, Handlers }; +export { INTEGRATIONS as Integrations, Transports, Handlers }; diff --git a/packages/node/src/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index c28fb36470f8..928bc3d40d1d 100644 --- a/packages/node/src/integrations/onuncaughtexception.ts +++ b/packages/node/src/integrations/onuncaughtexception.ts @@ -37,7 +37,9 @@ export class OnUncaughtException implements Integration { } } -/** JSDoc */ +/** + * @hidden + */ export function makeErrorHandler( onFatalError: (firstError: Error, secondError?: Error) => void = defaultOnFatalError, ): (error: Error) => void { diff --git a/packages/node/src/parsers.ts b/packages/node/src/parsers.ts index a5f124ef17c0..6c715321c1e6 100644 --- a/packages/node/src/parsers.ts +++ b/packages/node/src/parsers.ts @@ -13,6 +13,7 @@ const FILE_CONTENT_CACHE = new LRUMap(100); /** * Resets the file cache. Exists for testing purposes. + * @hidden */ export function resetFileContentCache(): void { FILE_CONTENT_CACHE.clear(); @@ -125,7 +126,9 @@ function readSourceFiles(filenames: string[]): SyncPromise<{ [key: string]: stri }); } -/** JSDoc */ +/** + * @hidden + */ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { const stack = stacktrace.parse(error); if (!stack) { @@ -134,7 +137,9 @@ export function extractStackFromError(error: Error): stacktrace.StackFrame[] { return stack; } -/** JSDoc */ +/** + * @hidden + */ export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): SyncPromise { const filesToRead: string[] = []; @@ -228,7 +233,9 @@ function addPrePostContext( ); } -/** JSDoc */ +/** + * @hidden + */ export function getExceptionFromError(error: Error, options?: NodeOptions): SyncPromise { const name = error.name || error.constructor.name; const stack = extractStackFromError(error); @@ -246,20 +253,24 @@ export function getExceptionFromError(error: Error, options?: NodeOptions): Sync ); } -/** JSDoc */ +/** + * @hidden + */ export function parseError(error: ExtendedError, options?: NodeOptions): SyncPromise { - const name = error.name || error.constructor.name; return new SyncPromise(resolve => - getExceptionFromError(error, options).then((exception: SentryException) => resolve({ + getExceptionFromError(error, options).then((exception: SentryException) => { + resolve({ exception: { values: [exception], }, - }), - ), + }); + }), ); } -/** JSDoc */ +/** + * @hidden + */ export function prepareFramesForEvent(stack: StackFrame[]): StackFrame[] { if (!stack || !stack.length) { return []; diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 8d0e457bb362..fa5a802704b7 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -29,30 +29,40 @@ export const defaultIntegrations = [ * provided methods. * * @example + * ``` + * * const { init } = require('@sentry/node'); * * init({ * dsn: '__DSN__', * // ... * }); - * + * ``` * * @example + * ``` + * * const { configureScope } = require('@sentry/node'); * configureScope((scope: Scope) => { * scope.setExtra({ battery: 0.7 }); * scope.setTag({ user_mode: 'admin' }); * scope.setUser({ id: '4711' }); * }); + * ``` * * @example + * ``` + * * const { addBreadcrumb } = require('@sentry/node'); * addBreadcrumb({ * message: 'My Breadcrumb', * // ... * }); + * ``` * * @example + * ``` + * * const Sentry = require('@sentry/node'); * Sentry.captureMessage('Hello, world!'); * Sentry.captureException(new Error('Good bye')); @@ -62,8 +72,9 @@ export const defaultIntegrations = [ * // ... * ], * }); + * ``` * - * @see NodeOptions for documentation on configuration options. + * @see {@link NodeOptions} for documentation on configuration options. */ export function init(options: NodeOptions = {}): void { if (options.defaultIntegrations === undefined) { diff --git a/packages/node/src/transports/base.ts b/packages/node/src/transports/base.ts index 1359903b4cad..4bd74729b473 100644 --- a/packages/node/src/transports/base.ts +++ b/packages/node/src/transports/base.ts @@ -8,7 +8,10 @@ import * as https from 'https'; import * as url from 'url'; import { SDK_NAME, SDK_VERSION } from '../version'; -/** Internal used interface for typescript */ +/** + * Internal used interface for typescript. + * @hidden + */ export interface HTTPRequest { request( options: http.RequestOptions | https.RequestOptions | string | url.URL, diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 75cfb49a2a90..ebb194f5c48e 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -8,12 +8,12 @@ import { configureScope, getCurrentHub, init, - NodeBackend, NodeClient, Scope, SentryEvent, SentryResponse, } from '../src'; +import { NodeBackend } from '../src/backend'; import { SetTimeoutTransport } from './helper/settimeouttransport'; const dsn = 'https://53039209a22b4ec1bcc296a3c9fdecd6@sentry.io/4291'; diff --git a/packages/node/test/integrations/linkederrors.test.ts b/packages/node/test/integrations/linkederrors.test.ts index 525948bc0906..2ab688deeae0 100644 --- a/packages/node/test/integrations/linkederrors.test.ts +++ b/packages/node/test/integrations/linkederrors.test.ts @@ -1,4 +1,5 @@ -import { NodeBackend, SentryEvent } from '../../src'; +import { SentryEvent } from '../../src'; +import { NodeBackend } from '../../src/backend'; import { LinkedErrors } from '../../src/integrations/linkederrors'; let linkedErrors: LinkedErrors; diff --git a/packages/node/test/parsers.test.ts b/packages/node/test/parsers.test.ts index 8df4d9b4a70c..01be51e7ea88 100644 --- a/packages/node/test/parsers.test.ts +++ b/packages/node/test/parsers.test.ts @@ -1,7 +1,7 @@ // import { StackFrame } from '@sentry/types'; import * as fs from 'fs'; import * as stacktrace from 'stack-trace'; -import { Parsers } from '../src'; +import * as Parsers from '../src/parsers'; import { getError } from './helper/error'; describe('parsers.ts', () => { diff --git a/packages/typescript/tslint.json b/packages/typescript/tslint.json index 04a6cb6e3c3e..ceec3be0b288 100644 --- a/packages/typescript/tslint.json +++ b/packages/typescript/tslint.json @@ -20,13 +20,13 @@ "completed-docs": [ true, { - "classes": true, - "enums": true, - "enum-members": true, - "functions": true, - "interfaces": true, - "methods": { "tags": { "content": {}, "existence": ["inheritDoc"] } }, - "properties": { "tags": { "content": {}, "existence": ["inheritDoc"] } } + "classes": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "enums": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "enum-members": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "functions": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "interfaces": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "methods": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } }, + "properties": { "tags": { "content": {}, "existence": ["inheritDoc", "hidden"] } } } ], "interface-name": [true, "never-prefix"], From 0b95f34b7b96222eed594edcdeb6ceb9fb685144 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Mon, 4 Feb 2019 14:05:21 +0100 Subject: [PATCH 13/16] meta: Changelog --- CHANGELOG.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 129cdd3152e7..75d3799f1b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 described it in the docs, you should be fine by just updating it. This is a **breaking** release +since we removed some public API surface 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 From 5ca41a836dedaf8179ae8a99111f1e7999e6c4bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 6 Feb 2019 09:06:58 +0100 Subject: [PATCH 14/16] Apply suggestions from code review Co-Authored-By: HazAT --- CHANGELOG.md | 4 ++-- packages/hub/src/scope.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d3799f1b9b..f14e0ced99b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,8 @@ ### 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 described it in the docs, you should be fine by just updating it. This is a **breaking** release -since we removed some public API surface and removed some classes from the default export. +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 diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index c3ac6b819cd2..f75cb08f4523 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -7,8 +7,8 @@ import { SyncPromise } from '@sentry/utils/syncpromise'; /** * Event processors are used to change the event before it will be send. * We strongly advise to make this function sync. - * Returning a Promise works just fine, just be sure what you are doing. - * Eventprosccing will be stopped until your Promise is resolved. + * Returning a Promise will work just fine, but better be sure that you know what you are doing. + * Event processing will be deferred until your Promise is resolved. */ export type EventProcessor = ( event: SentryEvent, From 7875c0bdcefdd5c0feabbd9940656c6f9a58389b Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Wed, 6 Feb 2019 12:51:45 +0100 Subject: [PATCH 15/16] fix: CodeReview --- packages/browser/src/backend.ts | 96 ++++---- packages/browser/test/backend.test.ts | 20 +- packages/core/src/baseclient.ts | 143 +++++------ packages/hub/src/scope.ts | 26 +- packages/hub/test/scope.test.ts | 35 ++- packages/utils/src/syncpromise.ts | 301 +++--------------------- packages/utils/test/is.test.ts | 3 +- packages/utils/test/syncpromise.test.ts | 59 +++++ 8 files changed, 259 insertions(+), 424 deletions(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index b56362278fc1..22e59c56b53d 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -75,57 +75,57 @@ export class BrowserBackend extends BaseBackend { * @inheritDoc */ public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise { - return new SyncPromise(resolve => { - 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 - event = eventFromStacktrace(computeStackTrace(exception as Error)); - 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; - - this.eventFromMessage(message, undefined, hint).then(messageEvent => { - addExceptionTypeValue(messageEvent, message); - resolve(this.buildEvent(messageEvent)); - }); - } else if (isError(exception as Error)) { - // we have a real Error object, do nothing - event = eventFromStacktrace(computeStackTrace(exception as Error)); - 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); - addExceptionTypeValue(event, 'Custom Object'); - resolve(this.buildEvent(event)); - } 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; - this.eventFromMessage(ex, undefined, hint).then(messageEvent => { - addExceptionTypeValue(messageEvent, `${ex}`); - resolve(this.buildEvent(messageEvent)); - }); - } + 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 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 domException = exception as DOMException; + const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException'); + const message = domException.message ? `${name}: ${domException.message}` : name; + + return this.eventFromMessage(message, undefined, 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 objectException = exception as {}; + event = eventFromPlainObject(objectException, hint.syntheticException); + addExceptionTypeValue(event, 'Custom Object'); + return SyncPromise.resolve(this.buildEvent(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)); }); } - /** JSDOC */ + /** + * This is an internal helper function that creates an event. + */ private buildEvent(event: SentryEvent, hint?: SentryEventHint): SentryEvent { return { ...event, diff --git a/packages/browser/test/backend.test.ts b/packages/browser/test/backend.test.ts index 73b9b22fd376..fe107bf60342 100644 --- a/packages/browser/test/backend.test.ts +++ b/packages/browser/test/backend.test.ts @@ -1,27 +1,13 @@ import { expect } from 'chai'; import { BrowserBackend } from '../src/backend'; -const dsn = 'https://123@sentry.io/42'; -const testEvent = { - event_id: '1337', - message: 'Pickle Rick', - user: { - username: 'Morty', - }, -}; - let backend: BrowserBackend; describe('BrowserBackend', () => { describe('sendEvent()', () => { - it('should throw when no Dsn is provided', async () => { - backend = new BrowserBackend({ dsn }); - - try { - backend.sendEvent(testEvent); - } catch (e) { - expect(e.message).equal('Cannot sendEvent without a valid Dsn'); - } + it('should use NoopTransport', async () => { + backend = new BrowserBackend({}); + expect(backend.getTransport().constructor.name).to.equal('NoopTransport'); }); }); }); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 2e7e877be3a3..bc6173defc6b 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -135,10 +135,12 @@ export abstract class BaseClient implement this.getBackend() .eventFromException(exception, hint) - .then(event => { - this.processEvent(event, hint, scope).then(finalEvent => { - eventId = (finalEvent && finalEvent.event_id) || undefined; - }); + .then(event => this.processEvent(event, hint, scope)) + .then(finalEvent => { + eventId = finalEvent.event_id; + }) + .catch(reason => { + logger.log(reason); }); return eventId; @@ -154,11 +156,14 @@ export abstract class BaseClient implement ? this.getBackend().eventFromMessage(`${message}`, level, hint) : this.getBackend().eventFromException(message, hint); - promisedEvent.then(event => { - this.processEvent(event, hint, scope).then(finalEvent => { - eventId = (finalEvent && finalEvent.event_id) || undefined; + promisedEvent + .then(event => this.processEvent(event, hint, scope)) + .then(finalEvent => { + eventId = finalEvent.event_id; + }) + .catch(reason => { + logger.log(reason); }); - }); return eventId; } @@ -168,9 +173,13 @@ export abstract class BaseClient implement */ public captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string | undefined { let eventId: string | undefined = hint && hint.event_id; - this.processEvent(event, hint, scope).then(finalEvent => { - eventId = (finalEvent && finalEvent.event_id) || undefined; - }); + this.processEvent(event, hint, scope) + .then(finalEvent => { + eventId = finalEvent.event_id; + }) + .catch(reason => { + logger.log(reason); + }); return eventId; } @@ -270,13 +279,17 @@ export abstract class BaseClient implement prepared.event_id = uuid4(); } + // We prepare the result here with a resolved SentryEvent. + let result = SyncPromise.resolve(prepared); + // This should be the last thing called, since we want that // {@link Hub.addEventProcessor} gets the finished prepared event. if (scope) { - return scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS)); + // In case we have a hub we reassign it. + result = scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS)); } - return SyncPromise.resolve(prepared as SentryEvent | null); + return result; } /** @@ -286,37 +299,29 @@ export abstract class BaseClient implement * platform specific meta data (such as the User's IP address) must be added * by the SDK implementor. * - * The returned event status offers clues to whether the event was sent to - * Sentry and accepted there. If the {@link Options.shouldSend} hook returns - * `false`, the status will be {@link SendStatus.Skipped}. If the rate limit - * was exceeded, the status will be {@link SendStatus.RateLimit}. * * @param event The event to send to Sentry. - * @param send A function to actually send the event. - * @param scope A scope containing event metadata. * @param hint May contain additional informartion about the original exception. - * @returns A SyncPromise that resolves with the event. + * @param scope A scope containing event metadata. + * @returns A SyncPromise that resolves with the event or rejects in case event was/will not be send. */ - protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): SyncPromise { + protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): SyncPromise { const { beforeSend, sampleRate } = this.getOptions(); if (!this.isEnabled()) { - logger.log('SDK not enabled, will not send event.'); - return SyncPromise.resolve(); + return SyncPromise.reject('SDK not enabled, will not send event.'); } // 1.0 === 100% events are sent // 0.0 === 0% events are sent if (typeof sampleRate === 'number' && Math.random() > sampleRate) { - logger.log('This event has been sampled, will not send event.'); - return SyncPromise.resolve(); + return SyncPromise.reject('This event has been sampled, will not send event.'); } - return new SyncPromise(resolve => { + return new SyncPromise((resolve, reject) => { this.prepareEvent(event, scope, hint).then(prepared => { if (prepared === null) { - logger.log('An event processor returned null, will not send event.'); - resolve(null); + reject('An event processor returned null, will not send event.'); return; } @@ -324,42 +329,27 @@ export abstract class BaseClient implement try { const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; - if (!isInternalException && beforeSend) { - const beforeSendResult = beforeSend(prepared, hint); - if ((typeof beforeSendResult as any) === 'undefined') { - logger.error('`beforeSend` method has to return `null` or a valid event.'); - } else if (isThenable(beforeSendResult)) { - (beforeSendResult as Promise) - .then(processedEvent => { - finalEvent = processedEvent; - - if (finalEvent === null) { - logger.log('`beforeSend` returned `null`, will not send event.'); - resolve(null); - return; - } - - // From here on we are really async - this.getBackend().sendEvent(finalEvent); - resolve(finalEvent); - }) - .catch(e => { - logger.error(`beforeSend rejected with ${e}`); - }); - } else { - finalEvent = beforeSendResult as SentryEvent | null; - - if (finalEvent === null) { - logger.log('`beforeSend` returned `null`, will not send event.'); - resolve(null); - return; - } - - // From here on we are really async - this.getBackend().sendEvent(finalEvent); - resolve(finalEvent); - } + if (isInternalException || !beforeSend) { + this.getBackend().sendEvent(finalEvent); + resolve(finalEvent); + return; + } + + const beforeSendResult = beforeSend(prepared, hint); + if ((typeof beforeSendResult as any) === 'undefined') { + logger.error('`beforeSend` method has to return `null` or a valid event.'); + } else if (isThenable(beforeSendResult)) { + this.handleAsyncBeforeSend(beforeSendResult as Promise, resolve, reject); } else { + finalEvent = beforeSendResult as SentryEvent | null; + + if (finalEvent === null) { + logger.log('`beforeSend` returned `null`, will not send event.'); + resolve(null); + return; + } + + // From here on we are really async this.getBackend().sendEvent(finalEvent); resolve(finalEvent); } @@ -370,14 +360,35 @@ export abstract class BaseClient implement }, originalException: exception as Error, }); - logger.error('`beforeSend` throw an error, will not send event.'); - resolve(null); - return; + reject('`beforeSend` throw an error, will not send event.'); } }); }); } + /** + * Resolves before send Promise and calls resolve/reject on parent SyncPromise. + */ + private handleAsyncBeforeSend( + beforeSend: Promise, + resolve: (event: SentryEvent) => void, + reject: (reason: string) => void, + ): void { + beforeSend + .then(processedEvent => { + if (processedEvent === null) { + reject('`beforeSend` returned `null`, will not send event.'); + return; + } + // From here on we are really async + this.getBackend().sendEvent(processedEvent); + resolve(processedEvent); + }) + .catch(e => { + reject(`beforeSend rejected with ${e}`); + }); + } + /** * @inheritDoc */ diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index f75cb08f4523..3a02f153b1f3 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -87,23 +87,15 @@ export class Scope { if (event === null || !isFunction(processor)) { resolve(event); } else { - try { - const result = processor({ ...event }, hint) as SentryEvent | null; - if (isThenable(result)) { - (result as Promise) - .then((final: SentryEvent | null) => { - this.notifyEventProcessors(processors, final, hint, index + 1) - .then(resolve) - .catch(reject); - }) - .catch(reject); - } else { - this.notifyEventProcessors(processors, result, hint, index + 1) - .then(resolve) - .catch(reject); - } - } catch (e) { - reject(e); + const result = processor({ ...event }, hint) as SentryEvent | null; + if (isThenable(result)) { + (result as Promise) + .then(final => this.notifyEventProcessors(processors, final, hint, index + 1).then(resolve)) + .catch(reject); + } else { + this.notifyEventProcessors(processors, result, hint, index + 1) + .then(resolve) + .catch(reject); } } }); diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts index 9707983b5b8c..7b4c5fc72e1a 100644 --- a/packages/hub/test/scope.test.ts +++ b/packages/hub/test/scope.test.ts @@ -190,7 +190,7 @@ describe('Scope', () => { }); }); - test('addEventProcessor async', () => { + test('addEventProcessor async', async () => { jest.useFakeTimers(); expect.assertions(6); const event: SentryEvent = { @@ -230,6 +230,39 @@ describe('Scope', () => { }); }); + test('addEventProcessor async with reject', async () => { + jest.useFakeTimers(); + expect.assertions(2); + const event: SentryEvent = { + extra: { b: 3 }, + }; + const localScope = new Scope(); + localScope.setExtra('a', 'b'); + const callCounter = jest.fn(); + localScope.addEventProcessor((processedEvent: SentryEvent) => { + callCounter(1); + expect(processedEvent.extra).toEqual({ a: 'b', b: 3 }); + return processedEvent; + }); + localScope.addEventProcessor( + async (_processedEvent: SentryEvent) => + new Promise((_, reject) => { + setTimeout(() => { + reject('bla'); + }, 1); + jest.runAllTimers(); + }), + ); + localScope.addEventProcessor((processedEvent: SentryEvent) => { + callCounter(4); + return processedEvent; + }); + + return localScope.applyToEvent(event).catch(reason => { + expect(reason).toEqual('bla'); + }); + }); + test('addEventProcessor return null', () => { expect.assertions(1); const event: SentryEvent = { diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts index 2e95ce764f95..e60018c2c99a 100644 --- a/packages/utils/src/syncpromise.ts +++ b/packages/utils/src/syncpromise.ts @@ -12,13 +12,12 @@ enum States { /** JSDoc */ interface Handler { - onSuccess: HandlerOnSuccess; onFail: HandlerOnFail; + onSuccess: HandlerOnSuccess; } type HandlerOnSuccess = (value: T) => U | Thenable; type HandlerOnFail = (reason: any) => U | Thenable; -type Finally = () => U | Thenable; /** JSDoc */ interface Thenable { @@ -29,7 +28,7 @@ type Resolve = (value?: R | Thenable | any) => void; type Reject = (value?: any) => void; /** JSDoc */ -export class SyncPromise { +export class SyncPromise implements PromiseLike { /** JSDoc */ private state: States = States.PENDING; /** JSDoc */ @@ -58,36 +57,36 @@ export class SyncPromise { /** JSDoc */ private readonly setResult = (value: T | any, state: States) => { if (this.state !== States.PENDING) { - return null; + return; } if (isThenable(value)) { - return (value as Thenable).then(this.resolve, this.reject); + (value as Thenable).then(this.resolve, this.reject); + return; } this.value = value; this.state = state; - return this.executeHandlers(); + this.executeHandlers(); }; /** JSDoc */ private readonly executeHandlers = () => { if (this.state === States.PENDING) { - return null; + return; } - this.handlers.forEach(handler => { - if (this.state === States.REJECTED) { - // tslint:disable-next-line:no-unsafe-any - return handler.onFail(this.value); - } + if (this.state === States.REJECTED) { // tslint:disable-next-line:no-unsafe-any - return handler.onSuccess(this.value); - }); + this.handlers.forEach(h => h.onFail && h.onFail(this.value)); + } else { + // tslint:disable-next-line:no-unsafe-any + this.handlers.forEach(h => h.onSuccess && h.onSuccess(this.value)); + } this.handlers = []; - return null; + return; }; /** JSDoc */ @@ -97,17 +96,21 @@ export class SyncPromise { }; /** JSDoc */ - public then(onSuccess?: HandlerOnSuccess, onFail?: HandlerOnFail): SyncPromise { - return new SyncPromise((resolve, reject) => { + public then( + onfulfilled?: ((value: T) => TResult1 | PromiseLike) | undefined | null, + onrejected?: ((reason: any) => TResult2 | PromiseLike) | undefined | null, + ): SyncPromise { + // public then(onSuccess?: HandlerOnSuccess, onFail?: HandlerOnFail): SyncPromise { + return new SyncPromise((resolve, reject) => { this.attachHandler({ onFail: reason => { - if (!onFail) { + if (!onrejected) { reject(reason); return; } try { - resolve(onFail(reason)); + resolve(onrejected(reason)); return; } catch (e) { reject(e); @@ -115,13 +118,12 @@ export class SyncPromise { } }, onSuccess: result => { - if (!onSuccess) { + if (!onfulfilled) { resolve(result); return; } - try { - resolve(onSuccess(result)); + resolve(onfulfilled(result)); return; } catch (e) { reject(e); @@ -133,9 +135,9 @@ export class SyncPromise { } /** JSDoc */ - public catch(onFail: HandlerOnFail): SyncPromise { + public catch(onFail: HandlerOnFail): SyncPromise { // tslint:disable-next-line:no-unsafe-any - return this.then((val: any) => val, onFail); + return this.then((val: any) => val, onFail as any); } /** JSDoc */ @@ -144,37 +146,7 @@ export class SyncPromise { } /** JSDoc */ - public finally(cb: Finally): SyncPromise { - return new SyncPromise((resolve, reject) => { - let val: U | any; - let isRejected: boolean; - - return this.then( - value => { - isRejected = false; - val = value; - return cb(); - }, - reason => { - isRejected = true; - val = reason; - return cb(); - }, - ).then(() => { - if (isRejected) { - reject(val); - return; - } - - // tslint:disable-next-line:no-unsafe-any - resolve(val); - return; - }); - }); - } - - /** JSDoc */ - public static resolve(value?: U | Thenable): SyncPromise { + public static resolve(value?: U | Thenable): SyncPromise { return new SyncPromise(resolve => { resolve(value); }); @@ -187,222 +159,3 @@ export class SyncPromise { }); } } - -// /** JSDoc */ -// export enum PromiseState { -// /** JSDoc */ -// PENDING, -// /** JSDoc */ -// RESOLVED, -// /** JSDoc */ -// REJECTED, -// } - -// /** JSDoc*/ -// interface Handle { -// onFulfilled?: ((value: T) => T | SyncPromise); -// onRejected?: ((reason: any) => void); -// } - -// /** JSDoc */ -// export class SyncPromise { -// /** JSDoc */ -// private value?: T | SyncPromise; - -// /** JSDoc */ -// private reason?: any; - -// /** JSDoc */ -// private state: PromiseState = PromiseState.PENDING; -// /** JSDoc */ -// private handlers: Array> = []; - -// /** JSDoc*/ -// private readonly id: number = Math.random(); - -// /** JSDoc */ -// public constructor( -// callback: ( -// resolve: ((value: T) => T | SyncPromise), -// reject: ((reason: any) => never | SyncPromise | void), -// ) => void, -// ) { -// callback( -// (value: T) => { -// console.log('CTOR callback RESOLVE'); -// // console.log('res', this.donee); -// this.value = value; -// this.state = PromiseState.RESOLVED; -// this.triggerHandlers(); -// return this.value; -// }, -// (reason: any) => { -// console.log('CTOR callback REJECT'); -// this.reason = reason; -// this.state = PromiseState.REJECTED; -// this.triggerHandlers(); -// }, -// ); -// } - -// /** JSDoc */ -// public then( -// onFulfilled?: ((value: T) => TResult1 | SyncPromise) | null, -// onRejected?: ((reason: any) => TResult2 | SyncPromise | never | void) | null, -// ): SyncPromise { -// /** JSDoc */ -// // public then( -// // onFulfilled?: ((value: T) => T) | undefined | null, -// // onRejected?: ((reason: any) => never) | undefined | null, -// // ): SyncPromise { -// return new SyncPromise((resolve, reject) => { -// this.done( -// result => { -// console.log('onFulfilled!!!'); -// console.log('result:', result); -// console.log('onFulfilled:', onFulfilled); -// console.log('onRejected:', onRejected); -// if (onFulfilled) { -// try { -// // @ts-ignore -// return resolve(onFulfilled(result)); -// } catch (ex) { -// return reject(ex); -// } -// } else { -// // @ts-ignore -// return resolve(result); -// } -// }, -// reason => { -// console.log('onRejected!!!'); -// console.log('reason:', reason); -// console.log('onFulfilled:', onFulfilled); -// console.log('onRejected:', onRejected); -// if (onRejected) { -// try { -// // @ts-ignore -// return resolve(onRejected(reason)); -// } catch (ex) { -// return reject(ex); -// } -// } else { -// return reject(reason); -// } -// }, -// ); -// }); -// } - -// /** JSDoc*/ -// public catch(onRejected?: ((reason: any) => never | SyncPromise | never | void)): SyncPromise { -// return this.then(null, onRejected); -// } - -// /** JSDoc */ -// private handle(handler: Handle): void { -// console.log('*******************'); -// console.log('handle called with:'); -// console.log('this:', this); -// console.log('handler.onFulfilled:', handler.onFulfilled); -// console.log('handler.onRejected:', handler.onRejected); -// console.log('*******************'); - -// if (this.state === PromiseState.PENDING) { -// this.handlers.push(handler); -// } else if (this.state === PromiseState.RESOLVED && handler.onFulfilled) { -// handler.onFulfilled(this.value as T); -// } else if (this.state === PromiseState.REJECTED && handler.onRejected) { -// handler.onRejected(this.reason); -// } -// } - -// /** JSDoc */ -// public done( -// onFulfilled?: ((value: T) => T | SyncPromise), -// onRejected?: ((reason: any) => void | SyncPromise), -// ): void { -// // console.log('done called'); -// this.handle({ onFulfilled, onRejected }); -// } - -// /** JSDoc */ -// private triggerHandlers(): void { -// const callbacks = this.handlers; -// this.handlers = []; -// callbacks.forEach(call => { -// this.handle(call); -// }); -// } - -// /** JSDoc */ -// public static resolve(value: T): SyncPromise { -// return new SyncPromise(resolve => resolve(value)); -// } - -// /** JSDoc */ -// public static reject(reason: any): SyncPromise { -// return new SyncPromise((_, reject) => reject(reason)); -// } -// } - -/* -enum PromiseState { - INITIAL, - RESOLVED, - REJECTED, -}; - -class SyncPromise { - private value?: T | any; - private state: PromiseState = PromiseState.INITIAL; - private _callbacks: Array = []; - - constructor(callback: (onResolve, onReject) => void) { - callback((value: T) => { - this.value = value; - this.state = PromiseState.RESOLVED; - this.triggerHandlers(); - }, (err: any) => { - this.value = err; - this.state = PromiseState.REJECTED; - this.triggerHandlers(); - }); - } - - public then(onResolve, onReject): void { - if (this.state != PromiseState.INITIAL) { - this.triggerHandler(onResolve, onReject); - } else { - this._callbacks.push([onResolve, onReject]); - } - } - - private triggerHandler(onResolve, onReject): void { - if (this.state == PromiseState.RESOLVED) { - onResolve(this.value); - } else { - onReject(this.value); - } - } - - private triggerHandlers(): void { - let callbacks = this._callbacks; - this._callbacks = []; - callbacks.forEach((onResolve, onReject) => { - this.triggerHandler(onResolve, onReject); - }); - } - - static resolve(value: T): SyncPromise { - return new SyncPromise((resolve) => { - resolve(value); - }); - } - - static reject(value: any): SyncPromise { - return new SyncPromise((_, reject) => { - reject(value); - }); - } -}*/ diff --git a/packages/utils/test/is.test.ts b/packages/utils/test/is.test.ts index 00e626a1f746..efbacfaa1b5e 100644 --- a/packages/utils/test/is.test.ts +++ b/packages/utils/test/is.test.ts @@ -1,5 +1,6 @@ import { isDOMError, isDOMException, isError, isErrorEvent, isPrimitive, isThenable } from '../src/is'; import { supportsDOMError, supportsDOMException, supportsErrorEvent } from '../src/supports'; +import { SyncPromise } from '../src/syncpromise'; class SentryError extends Error { public name: string; @@ -75,7 +76,7 @@ describe('isPrimitive()', () => { describe('isThenable()', () => { test('should work as advertised', () => { expect(isThenable(Promise.resolve(true))).toEqual(true); - // expect(isThenable(async () => false)).toEqual(true); + expect(isThenable(SyncPromise.resolve(true))).toEqual(true); expect(isThenable(undefined)).toEqual(false); expect(isThenable(null)).toEqual(false); diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts index c110f93ccfde..e11ff58c5d30 100644 --- a/packages/utils/test/syncpromise.test.ts +++ b/packages/utils/test/syncpromise.test.ts @@ -25,6 +25,65 @@ describe('SyncPromise', () => { }); }); + test('compare to regular promise', async () => { + expect.assertions(2); + + const ap = new Promise(resolve => { + resolve('1'); + }); + + const bp = new Promise(resolve => { + resolve('2'); + }); + + const cp = new Promise(resolve => { + resolve('3'); + }); + + const fp = async (s: Promise, prepend: string) => + new Promise(resolve => { + s.then(val => { + resolve(prepend + val); + }).catch(_ => { + // bla + }); + }); + + const res = await cp + .then(async val => fp(Promise.resolve('x'), val)) + .then(async val => fp(bp, val)) + .then(async val => fp(ap, val)); + + expect(res).toBe('3x21'); + + const a = new SyncPromise(resolve => { + resolve('1'); + }); + + const b = new SyncPromise(resolve => { + resolve('2'); + }); + + const c = new SyncPromise(resolve => { + resolve('3'); + }); + + const f = (s: SyncPromise, prepend: string) => + new SyncPromise(resolve => { + s.then(val => { + resolve(prepend + val); + }); + }); + + return c + .then(val => f(SyncPromise.resolve('x'), val)) + .then(val => f(b, val)) + .then(val => f(a, val)) + .then(val => { + expect(val).toBe(res); + }); + }); + test('simple static', () => { expect.assertions(1); From 14ffee1cd0825e3c62144bdfb6261a1c2dcdc416 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Wed, 6 Feb 2019 15:38:34 +0100 Subject: [PATCH 16/16] feat: https://github.com/getsentry/sentry-javascript/pull/1871 --- packages/browser/src/backend.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index 22e59c56b53d..9720099da19b 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -92,7 +92,7 @@ export class BrowserBackend extends BaseBackend { const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException'); const message = domException.message ? `${name}: ${domException.message}` : name; - return this.eventFromMessage(message, undefined, hint).then(messageEvent => { + return this.eventFromMessage(message, Severity.Error, hint).then(messageEvent => { addExceptionTypeValue(messageEvent, message); return SyncPromise.resolve(this.buildEvent(messageEvent)); });