diff --git a/CHANGELOG.md b/CHANGELOG.md index 129cdd3152e7..f14e0ced99b3 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 describe it in the docs, you should be fine by just updating it. This is a **breaking** release +since we removed some methods from the public API and removed some classes from the default export. + +- **breaking** [node] fix: Events created from exception shouldn't have top-level message attribute - [utils] ref: Update wrap method to hide internal sentry flags - [utils] fix: Make internal Sentry flags non-enumerable in fill util +- [utils] ref: Move `SentryError` + `PromiseBuffer` to utils +- **breaking** [core] ref: Use `SyncPromise` internally, this reduces memory pressure by a lot. +- **breaking** [browser] ref: Removed `BrowserBackend` from default export. +- **breaking** [node] ref: Removed `BrowserBackend` from default export. ## 4.5.3 - [browser]: fix: Fix UnhandledPromise: [object Object] -- [core]: fix: Error in extraErrorData integration where event would not be send in case of non assignable object property. +- [core]: fix: Error in extraErrorData integration where event would not be send in case of non assignable object + property. - [hub]: feat: Support non async event processors - ## 4.5.2 - [utils] fix: Decycling for objects to no produce an endless loop 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/src/backend.ts b/packages/browser/src/backend.ts index 83fb1835ad11..9720099da19b 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -1,7 +1,9 @@ -import { BaseBackend, Options, SentryError } from '@sentry/core'; +import { BaseBackend, Options } from '@sentry/core'; import { SentryEvent, SentryEventHint, Severity, Transport } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import { isDOMError, isDOMException, isError, isErrorEvent, isPlainObject } from '@sentry/utils/is'; import { supportsBeacon, supportsFetch } from '@sentry/utils/supports'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { addExceptionTypeValue, eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers'; import { computeStackTrace } from './tracekit'; import { BeaconTransport, FetchTransport, XHRTransport } from './transports'; @@ -26,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 @@ -69,48 +74,60 @@ export class BrowserBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromException(exception: any, hint?: SentryEventHint): Promise { - let event; + public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise { + let event: SentryEvent; if (isErrorEvent(exception as ErrorEvent) && (exception as ErrorEvent).error) { // If it is an ErrorEvent with `error` property, extract it to get actual Error - const ex = exception as ErrorEvent; - exception = ex.error; // tslint:disable-line:no-parameter-reassignment + const errorEvent = exception as ErrorEvent; + exception = errorEvent.error; // tslint:disable-line:no-parameter-reassignment event = eventFromStacktrace(computeStackTrace(exception as Error)); + return SyncPromise.resolve(this.buildEvent(event)); } else if (isDOMError(exception as DOMError) || isDOMException(exception as DOMException)) { // If it is a DOMError or DOMException (which are legacy APIs, but still supported in some browsers) // then we just extract the name and message, as they don't provide anything else // https://developer.mozilla.org/en-US/docs/Web/API/DOMError // https://developer.mozilla.org/en-US/docs/Web/API/DOMException - const ex = exception as DOMException; - const name = ex.name || (isDOMError(ex) ? 'DOMError' : 'DOMException'); - const message = ex.message ? `${name}: ${ex.message}` : name; - - event = await this.eventFromMessage(message, undefined, hint); - addExceptionTypeValue(event, message); + const domException = exception as DOMException; + const name = domException.name || (isDOMError(domException) ? 'DOMError' : 'DOMException'); + const message = domException.message ? `${name}: ${domException.message}` : name; + + return this.eventFromMessage(message, Severity.Error, hint).then(messageEvent => { + addExceptionTypeValue(messageEvent, message); + return SyncPromise.resolve(this.buildEvent(messageEvent)); + }); } else if (isError(exception as Error)) { // we have a real Error object, do nothing event = eventFromStacktrace(computeStackTrace(exception as Error)); + return SyncPromise.resolve(this.buildEvent(event)); } else if (isPlainObject(exception as {}) && hint && hint.syntheticException) { // If it is plain Object, serialize it manually and extract options // This will allow us to group events based on top-level keys // which is much better than creating new group when any key/value change - const ex = exception as {}; - event = eventFromPlainObject(ex, hint.syntheticException); + const objectException = exception as {}; + event = eventFromPlainObject(objectException, hint.syntheticException); addExceptionTypeValue(event, 'Custom Object'); - } else { - // If none of previous checks were valid, then it means that - // it's not a DOMError/DOMException - // it's not a plain Object - // it's not a valid ErrorEvent (one with an error property) - // it's not an Error - // So bail out and capture it as a simple message: - const ex = exception as string; - event = await this.eventFromMessage(ex, undefined, hint); - addExceptionTypeValue(event, `${ex}`); + return SyncPromise.resolve(this.buildEvent(event)); } - event = { + // If none of previous checks were valid, then it means that + // it's not a DOMError/DOMException + // it's not a plain Object + // it's not a valid ErrorEvent (one with an error property) + // it's not an Error + // So bail out and capture it as a simple message: + const stringException = exception as string; + return this.eventFromMessage(stringException, undefined, hint).then(messageEvent => { + addExceptionTypeValue(messageEvent, `${stringException}`); + return SyncPromise.resolve(this.buildEvent(messageEvent)); + }); + } + + /** + * This is an internal helper function that creates an event. + */ + private buildEvent(event: SentryEvent, hint?: SentryEventHint): SentryEvent { + return { ...event, event_id: hint && hint.event_id, exception: { @@ -121,18 +138,16 @@ export class BrowserBackend extends BaseBackend { }, }, }; - - return event; } /** * @inheritDoc */ - public async eventFromMessage( + public eventFromMessage( message: string, level: Severity = Severity.Info, hint?: SentryEventHint, - ): Promise { + ): SyncPromise { const event: SentryEvent = { event_id: hint && hint.event_id, level, @@ -147,6 +162,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 bca2bd077f5b..fcdc0e33307b 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -1,6 +1,8 @@ -import { API, BaseClient, Scope, SentryError } from '@sentry/core'; +import { API, BaseClient, Scope } from '@sentry/core'; import { DsnLike, SentryEvent, SentryEventHint } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import { getGlobalObject } from '@sentry/utils/misc'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { BrowserBackend, BrowserOptions } from './backend'; import { SDK_NAME, SDK_VERSION } from './version'; @@ -48,7 +50,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): 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/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 1c39f2429f81..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, @@ -79,8 +85,8 @@ export function wrap( } catch (ex) { ignoreNextOnError(); - withScope(async scope => { - scope.addEventProcessor(async (event: SentryEvent) => { + withScope(scope => { + scope.addEventProcessor((event: SentryEvent) => { const processedEvent = { ...event }; if (options.mechanism) { @@ -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/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/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/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/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/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/browser/test/backend.test.ts b/packages/browser/test/backend.test.ts index 413cd91fe2e6..fe107bf60342 100644 --- a/packages/browser/test/backend.test.ts +++ b/packages/browser/test/backend.test.ts @@ -1,35 +1,13 @@ 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 = { - 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 { - await backend.sendEvent(testEvent); - } catch (e) { - 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); + it('should use NoopTransport', async () => { + backend = new BrowserBackend({}); + expect(backend.getTransport().constructor.name).to.equal('NoopTransport'); }); }); }); 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/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); }; 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/basebackend.ts b/packages/core/src/basebackend.ts index b42439622175..fe214cab84be 100644 --- a/packages/core/src/basebackend.ts +++ b/packages/core/src/basebackend.ts @@ -1,8 +1,9 @@ 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'; -import { SentryError } from './error'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { Backend, Options } from './interfaces'; import { NoopTransport } from './transports/noop'; @@ -40,29 +41,24 @@ export abstract class BaseBackend implements Backend { /** * @inheritDoc */ - public async eventFromException(_exception: any, _hint?: SentryEventHint): Promise { + public eventFromException(_exception: any, _hint?: SentryEventHint): SyncPromise { 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): SyncPromise { throw new SentryError('Backend has to implement `eventFromMessage` method'); } /** * @inheritDoc */ - public async sendEvent(event: SentryEvent): Promise { - // TODO: Remove with v5 - // tslint:disable-next-line - if (this.transport.captureEvent) { - // tslint:disable-next-line - return this.transport.captureEvent(event); - } - // -------------------- - return this.transport.sendEvent(serialize(event)); + public sendEvent(event: SentryEvent): void { + 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 0daccbae7b97..bc6173defc6b 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -6,20 +6,17 @@ import { SentryBreadcrumbHint, SentryEvent, SentryEventHint, - SentryResponse, Severity, - Status, } from '@sentry/types'; -import { forget } from '@sentry/utils/async'; -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'; +import { SyncPromise } from '@sentry/utils/syncpromise'; 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 @@ -96,9 +93,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 +130,57 @@ 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): 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.event_id; + }) + .catch(reason => { + logger.log(reason); + }); + + return eventId; } /** * @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): 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.processEvent(event, hint, scope)) + .then(finalEvent => { + eventId = finalEvent.event_id; + }) + .catch(reason => { + logger.log(reason); + }); + + return eventId; } /** * @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): string | undefined { + let eventId: string | undefined = hint && hint.event_id; + this.processEvent(event, hint, scope) + .then(finalEvent => { + eventId = finalEvent.event_id; + }) + .catch(reason => { + logger.log(reason); + }); + return eventId; } /** @@ -241,7 +246,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): SyncPromise { const { environment, maxBreadcrumbs = DEFAULT_BREADCRUMBS, release, dist } = this.getOptions(); const prepared = { ...event }; @@ -274,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 prepared; + return result; } /** @@ -290,109 +299,103 @@ 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 Promise that resolves with the event status. + * @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 async processEvent( - event: SentryEvent, - send: (finalEvent: SentryEvent) => Promise, - hint?: SentryEventHint, - scope?: Scope, - ): Promise { + protected processEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): SyncPromise { + const { beforeSend, sampleRate } = this.getOptions(); + if (!this.isEnabled()) { - return { - status: Status.Skipped, - }; + return SyncPromise.reject('SDK not enabled, will not send event.'); } - const { beforeSend, sampleRate } = this.getOptions(); - // 1.0 === 100% events are sent // 0.0 === 0% events are sent if (typeof sampleRate === 'number' && Math.random() > sampleRate) { - return { - status: Status.Skipped, - }; + return SyncPromise.reject('This event has been sampled, will not send event.'); } - const prepared = await this.prepareEvent(event, scope, hint); - if (prepared === null) { - return { - status: Status.Skipped, - }; - } - - let finalEvent: SentryEvent | null = prepared; - - try { - const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; - if (!isInternalException && beforeSend) { - finalEvent = await beforeSend(prepared, hint); - if ((typeof finalEvent as any) === 'undefined') { - logger.error('`beforeSend` method has to return `null` or a valid event'); + return new SyncPromise((resolve, reject) => { + this.prepareEvent(event, scope, hint).then(prepared => { + if (prepared === null) { + reject('An event processor returned null, will not send event.'); + return; } - } - } 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, - }; - } - if (finalEvent === null) { - return { - reason: 'Event dropped due to being discarded by beforeSend method', - status: Status.Skipped, - }; - } + let finalEvent: SentryEvent | null = prepared; + + try { + const isInternalException = hint && hint.data && (hint.data as { [key: string]: any }).__sentry__ === true; + 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); + } + } catch (exception) { + this.captureException(exception, { + data: { + __sentry__: true, + }, + originalException: exception as Error, + }); + reject('`beforeSend` throw an error, will not send event.'); + } + }); + }); + } - 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; - } 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, - }; - } + /** + * 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 */ 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/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/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/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/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/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/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/core/src/interfaces.ts b/packages/core/src/interfaces.ts index 45f1540dfa6f..b28677e6f356 100644 --- a/packages/core/src/interfaces.ts +++ b/packages/core/src/interfaces.ts @@ -6,12 +6,12 @@ import { SentryBreadcrumbHint, SentryEvent, SentryEventHint, - SentryResponse, Severity, Transport, TransportClass, TransportOptions, } from '@sentry/types'; +import { SyncPromise } from '@sentry/utils/syncpromise'; import { Dsn } from './dsn'; /** Console logging verbosity for the SDK. */ @@ -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 | Promise; + beforeSend?(event: SentryEvent, hint?: SentryEventHint): Promise | SentryEvent | null; /** * A callback invoked when adding a breadcrumb, allowing to optionally modify @@ -162,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): Promise; + captureException(exception: any, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Captures a message event and sends it to Sentry. @@ -173,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): Promise; + captureMessage(message: string, level?: Severity, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Captures a manually created event and sends it to Sentry. @@ -183,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): Promise; + captureEvent(event: SentryEvent, hint?: SentryEventHint, scope?: Scope): string | undefined; /** * Records a new breadcrumb which will be attached to future events. @@ -244,13 +244,13 @@ export interface Backend { install?(): boolean; /** Creates a {@link SentryEvent} from an exception. */ - eventFromException(exception: any, hint?: SentryEventHint): Promise; + eventFromException(exception: any, hint?: SentryEventHint): SyncPromise; /** Creates a {@link SentryEvent} from a plain message. */ - eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): Promise; + eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): SyncPromise; /** 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/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 5698c353bd94..120fd04b555c 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 { SentryEvent } from '@sentry/types'; +import { SentryError } from '@sentry/utils/error'; import { TestBackend } from '../mocks/backend'; import { TestClient } from '../mocks/client'; import { TestIntegration } from '../mocks/integration'; @@ -38,49 +38,62 @@ jest.mock('@sentry/utils/string', () => ({ })); describe('BaseClient', () => { + beforeEach(() => { + TestBackend.sendEventCalled = undefined; + TestBackend.instance = undefined; + }); + 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 +102,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 +111,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 +120,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 +129,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 +139,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 +148,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 +157,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 +166,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 +175,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 +186,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 +203,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 +261,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 +277,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 +293,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 +307,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 +322,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 +340,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,89 +353,107 @@ 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', 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 }); - const scope = new Scope(); - await client.captureEvent({ message: 'hello' }, undefined, scope); - expect(TestBackend.instance!.event!.message).toBe('hello'); - }); - - test('calls async beforeSend and uses the new one', async () => { + client.captureEvent({ message: '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', done => { + jest.useFakeTimers(); + expect.assertions(1); const beforeSend = jest.fn( async () => new Promise(resolve => { - resolve({ message: 'changed' }); + setTimeout(() => { + resolve({ message: 'changed2' }); + }, 1); }), ); - 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'); - }); - test('calls async beforeSend and discards the event', async () => { + const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend }); + client.captureEvent({ message: 'hello' }); + jest.runOnlyPendingTimers(); + TestBackend.sendEventCalled = (event: SentryEvent) => { + expect(event.message).toBe('changed2'); + }; + setTimeout(() => { + done(); + }, 5); + jest.runOnlyPendingTimers(); + }); + + 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' }); 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/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/core/test/mocks/backend.ts b/packages/core/test/mocks/backend.ts index 44dbaa47bdb3..b4a741ff8c77 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'; @@ -9,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; @@ -24,8 +26,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 +36,16 @@ 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 }; + // tslint:disable-next-line + TestBackend.sendEventCalled && TestBackend.sendEventCalled(event); } } 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/package.json b/packages/hub/package.json index 0cd4a6d39ecf..3f498ef3edd8 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/hub.ts b/packages/hub/src/hub.ts index eaafd54d9657..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; @@ -66,27 +68,13 @@ 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. * * @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; @@ -196,7 +184,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 +201,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 +216,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, }); @@ -363,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 { @@ -375,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 2f30f16a182d..3a02f153b1f3 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -1,7 +1,15 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, User } from '@sentry/types'; +import { isFunction, isThenable } from '@sentry/utils/is'; 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 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, hint?: SentryEventHint, @@ -68,19 +76,29 @@ export class Scope { /** * This will be called after {@link applyToEvent} is finished. */ - protected async notifyEventProcessors(event: SentryEvent, hint?: SentryEventHint): Promise { - let processedEvent: SentryEvent | null = event; - for (const processor of [...getGlobalEventProcessors(), ...this.eventProcessors]) { - try { - processedEvent = await processor({ ...processedEvent }, hint); - if (processedEvent === null) { - return null; + protected notifyEventProcessors( + processors: EventProcessor[], + event: SentryEvent | null, + hint?: SentryEventHint, + index: number = 0, + ): SyncPromise { + return new SyncPromise((resolve, reject) => { + const processor = processors[index]; + if (event === null || !isFunction(processor)) { + resolve(event); + } else { + 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); } - } catch (e) { - continue; } - } - return processedEvent; + }); } /** @@ -208,12 +226,13 @@ 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 async applyToEvent( + public applyToEvent( event: SentryEvent, hint?: SentryEventHint, maxBreadcrumbs?: number, - ): Promise { + ): SyncPromise { if (this.extra && Object.keys(this.extra).length) { event.extra = { ...this.extra, ...event.extra }; } @@ -237,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/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..7b4c5fc72e1a 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 () => { + 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,57 @@ describe('Scope', () => { processedEvent.dist = '1'; resolve(processedEvent); }, 1); + jest.runAllTimers(); + }), + ); + localScope.addEventProcessor((processedEvent: SentryEvent) => { + callCounter(4); + return processedEvent; + }); + + 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 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(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).catch(reason => { + expect(reason).toEqual('bla'); + }); }); - test('addEventProcessor return null', async () => { + test('addEventProcessor return null', () => { expect.assertions(1); const event: SentryEvent = { extra: { b: 3 }, @@ -224,11 +271,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 +288,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 23ee66e1f713..f633f1985641 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 5423686e1aa5..e42af177d50c 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 { SyncPromise } from '@sentry/utils/syncpromise'; import { createHash } from 'crypto'; import { extractStackFromError, parseError, parseStack, prepareFramesForEvent } from './parsers'; import { HTTPSTransport, HTTPTransport } from './transports'; @@ -35,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 @@ -69,7 +73,7 @@ export class NodeBackend extends BaseBackend { /** * @inheritDoc */ - public async eventFromException(exception: any, hint?: SentryEventHint): Promise { + public eventFromException(exception: any, hint?: SentryEventHint): SyncPromise { let ex: any = exception; if (!isError(exception)) { @@ -97,36 +101,42 @@ export class NodeBackend extends BaseBackend { } } - const event: SentryEvent = await parseError(ex as Error, this.options); - - return { - ...event, - event_id: hint && hint.event_id, - }; + return new SyncPromise(resolve => + parseError(ex as Error, this.options).then(event => { + resolve({ + ...event, + event_id: hint && hint.event_id, + }); + }), + ); } /** * @inheritDoc */ - public async eventFromMessage( + public eventFromMessage( message: string, level: Severity = Severity.Info, hint?: SentryEventHint, - ): Promise { + ): SyncPromise { const event: SentryEvent = { event_id: hint && hint.event_id, level, message, }; - if (this.options.attachStacktrace && hint && hint.syntheticException) { - const stack = hint.syntheticException ? await extractStackFromError(hint.syntheticException) : []; - const frames = await parseStack(stack, this.options); - event.stacktrace = { - frames: prepareFramesForEvent(frames), - }; - } - - return event; + return new SyncPromise(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 e76ea420fd25..df295e677d80 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 { SyncPromise } from '@sentry/utils/syncpromise'; import { NodeBackend, NodeOptions } from './backend'; import { SDK_NAME, SDK_VERSION } from './version'; @@ -21,7 +22,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): SyncPromise { event.platform = event.platform || 'node'; event.sdk = { ...event.sdk, diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index dd2abcd64f82..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; @@ -230,7 +234,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(); }); @@ -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 458f405e13de..afcc20cb18c3 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'; @@ -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/linkederrors.ts b/packages/node/src/integrations/linkederrors.ts index 727bd0f29454..df7395aabb5a 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 { SyncPromise } from '@sentry/utils/syncpromise'; import { getExceptionFromError } from '../parsers'; const DEFAULT_KEY = 'cause'; @@ -45,10 +46,10 @@ 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); + return (self.handler(event, hint) as unknown) as Promise; } return event; }); @@ -57,27 +58,36 @@ export class LinkedErrors implements Integration { /** * @inheritDoc */ - public async handler(event: SentryEvent, hint?: SentryEventHint): Promise { + public handler(event: SentryEvent, hint?: SentryEventHint): SyncPromise { if (!event.exception || !event.exception.values || !hint || !(hint.originalException instanceof Error)) { - return event; + return SyncPromise.resolve(event); } - const linkedErrors = await this.walkErrorTree(hint.originalException, this.key); - event.exception.values = [...linkedErrors, ...event.exception.values]; - return event; + + 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]; + } + resolve(event); + }); + }); } /** * @inheritDoc */ - public async walkErrorTree( + public walkErrorTree( error: ExtendedError, key: string, stack: SentryException[] = [], - ): Promise { + ): SyncPromise { if (!(error[key] instanceof Error) || stack.length + 1 >= this.limit) { - return stack; + return SyncPromise.resolve(stack); } - const exception = await getExceptionFromError(error[key]); - return this.walkErrorTree(error[key], key, [exception, ...stack]); + 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/integrations/onuncaughtexception.ts b/packages/node/src/integrations/onuncaughtexception.ts index f099e5cf6d14..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 { @@ -58,7 +60,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/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 da112a6025ce..6c715321c1e6 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 { readFileAsync } from '@sentry/utils/fs'; import { basename, dirname } from '@sentry/utils/path'; 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'; import { NodeOptions } from './backend'; @@ -12,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(); @@ -73,55 +75,61 @@ function getModule(filename: string, base?: string): string { * * @param filenames Array of filepaths to read content from. */ -async function readSourceFiles( - filenames: string[], -): Promise<{ - [key: string]: string; -}> { +function readSourceFiles(filenames: string[]): SyncPromise<{ [key: string]: string | null }> { // we're relying on filenames being de-duped already if (filenames.length === 0) { - return {}; + return SyncPromise.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 SyncPromise<{ + [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 = await readFileAsync(filename); - 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 */ -export async function extractStackFromError(error: Error): Promise { +/** + * @hidden + */ +export function extractStackFromError(error: Error): stacktrace.StackFrame[] { const stack = stacktrace.parse(error); if (!stack) { return []; @@ -129,8 +137,10 @@ export async function extractStackFromError(error: Error): Promise { +/** + * @hidden + */ +export function parseStack(stack: stacktrace.StackFrame[], options?: NodeOptions): SyncPromise { const filesToRead: string[] = []; const linesOfContext = @@ -171,15 +181,15 @@ export async function parseStack(stack: stacktrace.StackFrame[], options?: NodeO // We do an early return if we do not want to fetch context liens if (linesOfContext <= 0) { - return frames; + return SyncPromise.resolve(frames); } 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. - return frames; + return SyncPromise.resolve(frames); } } @@ -189,61 +199,78 @@ export async function parseStack(stack: stacktrace.StackFrame[], options?: NodeO * @param filesToRead string[] of filepaths * @param frames StackFrame[] containg all frames */ -async function addPrePostContext( +function addPrePostContext( filesToRead: string[], frames: StackFrame[], linesOfContext: number, -): Promise { - const sourceFiles = await 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; - }); +): SyncPromise { + return new SyncPromise(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 async function getExceptionFromError(error: Error, options?: NodeOptions): Promise { +/** + * @hidden + */ +export function getExceptionFromError(error: Error, options?: NodeOptions): SyncPromise { const name = error.name || error.constructor.name; - const stack = await extractStackFromError(error); - const frames = await parseStack(stack, options); - - return { - stacktrace: { - frames: prepareFramesForEvent(frames), - }, - type: name, - value: error.message, - }; + const stack = extractStackFromError(error); + return new SyncPromise(resolve => + parseStack(stack, options).then(frames => { + const result = { + stacktrace: { + frames: prepareFramesForEvent(frames), + }, + type: name, + value: error.message, + }; + resolve(result); + }), + ); } -/** JSDoc */ -export async function parseError(error: ExtendedError, options?: NodeOptions): Promise { - const exception = await getExceptionFromError(error, options); - return { - exception: { - values: [exception], - }, - }; +/** + * @hidden + */ +export function parseError(error: ExtendedError, options?: NodeOptions): SyncPromise { + return new SyncPromise(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 c1b828757922..4bd74729b473 100644 --- a/packages/node/src/transports/base.ts +++ b/packages/node/src/transports/base.ts @@ -1,12 +1,17 @@ -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'; 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/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/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 1f41ea9265d0..2ab688deeae0 100644 --- a/packages/node/test/integrations/linkederrors.test.ts +++ b/packages/node/test/integrations/linkederrors.test.ts @@ -1,4 +1,5 @@ -import { NodeBackend } from '../../src'; +import { SentryEvent } from '../../src'; +import { NodeBackend } from '../../src/backend'; import { LinkedErrors } from '../../src/integrations/linkederrors'; let linkedErrors: LinkedErrors; @@ -14,26 +15,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 +54,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 +74,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 +107,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 +140,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..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', () => { @@ -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/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'; 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/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"], diff --git a/packages/utils/package.json b/packages/utils/package.json index 9fd602f80eb6..8e1f38c67deb 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/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/utils/src/is.ts b/packages/utils/src/is.ts index 2228c56eccec..283378d09c71 100644 --- a/packages/utils/src/is.ts +++ b/packages/utils/src/is.ts @@ -138,3 +138,13 @@ export function isRegExp(wat: any): boolean { export function isNaN(wat: any): boolean { return wat !== wat; } + +/** + * Checks whether given value has a then function. + * @param wat A value to be checked. + */ +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/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 diff --git a/packages/utils/src/syncpromise.ts b/packages/utils/src/syncpromise.ts new file mode 100644 index 000000000000..e60018c2c99a --- /dev/null +++ b/packages/utils/src/syncpromise.ts @@ -0,0 +1,161 @@ +import { isThenable } from './is'; + +/** JSDoc */ +enum States { + /** JSDoc */ + PENDING = 'PENDING', + /** JSDoc */ + RESOLVED = 'RESOLVED', + /** JSDoc */ + REJECTED = 'REJECTED', +} + +/** JSDoc */ +interface Handler { + onFail: HandlerOnFail; + onSuccess: HandlerOnSuccess; +} + +type HandlerOnSuccess = (value: T) => U | Thenable; +type HandlerOnFail = (reason: any) => 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 implements PromiseLike { + /** 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) => { + if (this.state !== States.PENDING) { + return; + } + + if (isThenable(value)) { + (value as Thenable).then(this.resolve, this.reject); + return; + } + + this.value = value; + this.state = state; + + this.executeHandlers(); + }; + + /** JSDoc */ + private readonly executeHandlers = () => { + if (this.state === States.PENDING) { + return; + } + + if (this.state === States.REJECTED) { + // tslint:disable-next-line:no-unsafe-any + 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; + }; + + /** JSDoc */ + private readonly attachHandler = (handler: Handler) => { + this.handlers = this.handlers.concat(handler); + this.executeHandlers(); + }; + + /** JSDoc */ + 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 (!onrejected) { + reject(reason); + return; + } + + try { + resolve(onrejected(reason)); + return; + } catch (e) { + reject(e); + return; + } + }, + onSuccess: result => { + if (!onfulfilled) { + resolve(result); + return; + } + try { + resolve(onfulfilled(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 as any); + } + + /** JSDoc */ + public toString(): string { + return `[object SyncPromise]`; + } + + /** 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); + }); + } +} diff --git a/packages/utils/test/is.test.ts b/packages/utils/test/is.test.ts index 0fb0ccc9bb89..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 } from '../src/is'; +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; @@ -71,3 +72,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(SyncPromise.resolve(true))).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/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 diff --git a/packages/utils/test/syncpromise.test.ts b/packages/utils/test/syncpromise.test.ts new file mode 100644 index 000000000000..e11ff58c5d30 --- /dev/null +++ b/packages/utils/test/syncpromise.test.ts @@ -0,0 +1,250 @@ +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('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('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); + + const p = SyncPromise.resolve(10); + return p.then(val => { + expect(val).toBe(10); + }); + }); + + test('using new Promise internally', () => { + expect.assertions(2); + + return new SyncPromise(done => { + new Promise(resolve => { + expect(true).toBe(true); + resolve(41); + }) + .then(done) + .catch(_ => { + // + }); + }).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'); + }); + }); +});