Skip to content

Commit afcdb4c

Browse files
HazATHazAT
committed
feat: SyncPromise implementation
1 parent 3ada698 commit afcdb4c

22 files changed

+729
-305
lines changed

‎packages/core/src/basebackend.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { Breadcrumb, SentryEvent, SentryEventHint, Severity, Transport } from '@
33
import { SentryError } from '@sentry/utils/error';
44
import { logger } from '@sentry/utils/logger';
55
import { serialize } from '@sentry/utils/object';
6-
import { QuickPromise } from '@sentry/utils/quickpromise';
6+
import { SyncPromise } from '@sentry/utils/syncpromise';
77
import { Backend, Options } from './interfaces';
88
import { NoopTransport } from './transports/noop';
99

@@ -41,22 +41,24 @@ export abstract class BaseBackend<O extends Options> implements Backend {
4141
/**
4242
* @inheritDoc
4343
*/
44-
public eventFromException(_exception: any, _hint?: SentryEventHint): QuickPromise<SentryEvent> {
44+
public eventFromException(_exception: any, _hint?: SentryEventHint): SyncPromise<SentryEvent> {
4545
throw new SentryError('Backend has to implement `eventFromException` method');
4646
}
4747

4848
/**
4949
* @inheritDoc
5050
*/
51-
public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): QuickPromise<SentryEvent> {
51+
public eventFromMessage(_message: string, _level?: Severity, _hint?: SentryEventHint): SyncPromise<SentryEvent> {
5252
throw new SentryError('Backend has to implement `eventFromMessage` method');
5353
}
5454

5555
/**
5656
* @inheritDoc
5757
*/
5858
public sendEvent(event: SentryEvent): void {
59-
this.transport.sendEvent(serialize(event));
59+
this.transport.sendEvent(serialize(event)).catch(reason => {
60+
logger.error(`Error while sending event: ${reason}`);
61+
});
6062
}
6163

6264
/**

‎packages/core/src/baseclient.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { isPrimitive } from '@sentry/utils/is';
1212
import { logger } from '@sentry/utils/logger';
1313
import { consoleSandbox, uuid4 } from '@sentry/utils/misc';
14-
import { QuickPromise } from '@sentry/utils/quickpromise';
14+
import { SyncPromise } from '@sentry/utils/syncpromise';
1515
import { truncate } from '@sentry/utils/string';
1616
import { BackendClass } from './basebackend';
1717
import { Dsn } from './dsn';
@@ -224,7 +224,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
224224
* @param scope A scope containing event metadata.
225225
* @returns A new event with more information.
226226
*/
227-
protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): QuickPromise<SentryEvent | null> {
227+
protected prepareEvent(event: SentryEvent, scope?: Scope, hint?: SentryEventHint): SyncPromise<SentryEvent | null> {
228228
const { environment, maxBreadcrumbs = DEFAULT_BREADCRUMBS, release, dist } = this.getOptions();
229229

230230
const prepared = { ...event };
@@ -263,7 +263,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
263263
return scope.applyToEvent(prepared, hint, Math.min(maxBreadcrumbs, MAX_BREADCRUMBS));
264264
}
265265

266-
return QuickPromise.resolve(prepared);
266+
return SyncPromise.resolve(prepared);
267267
}
268268

269269
/**
@@ -331,13 +331,8 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
331331
return;
332332
}
333333

334-
try {
335-
this.getBackend().sendEvent(finalEvent);
336-
} catch (error) {
337-
// We have a catch here since the transport can reject the request internally.
338-
// If we do not catch this here, we will run into an endless loop.
339-
logger.error(`${error}`);
340-
}
334+
// From here on we are really async
335+
this.getBackend().sendEvent(finalEvent);
341336
});
342337
}
343338

‎packages/core/src/interfaces.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
TransportClass,
1212
TransportOptions,
1313
} from '@sentry/types';
14-
import { QuickPromise } from '@sentry/utils/quickpromise';
14+
import { SyncPromise } from '@sentry/utils/syncpromise';
1515
import { Dsn } from './dsn';
1616

1717
/** Console logging verbosity for the SDK. */
@@ -244,10 +244,10 @@ export interface Backend {
244244
install?(): boolean;
245245

246246
/** Creates a {@link SentryEvent} from an exception. */
247-
eventFromException(exception: any, hint?: SentryEventHint): QuickPromise<SentryEvent>;
247+
eventFromException(exception: any, hint?: SentryEventHint): SyncPromise<SentryEvent>;
248248

249249
/** Creates a {@link SentryEvent} from a plain message. */
250-
eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): QuickPromise<SentryEvent>;
250+
eventFromMessage(message: string, level?: Severity, hint?: SentryEventHint): SyncPromise<SentryEvent>;
251251

252252
/** Submits the event to Sentry */
253253
sendEvent(event: SentryEvent): void;

‎packages/core/test/lib/base.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Scope } from '@sentry/hub';
22
import { SentryEvent, Status } from '@sentry/types';
3-
import { SentryError } from '../../src/error';
3+
import { SentryError } from '@sentry/utils/error';
44
import { TestBackend } from '../mocks/backend';
55
import { TestClient } from '../mocks/client';
66
import { TestIntegration } from '../mocks/integration';

‎packages/core/test/lib/dsn.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
import { SentryError } from '@sentry/utils/error';
12
import { Dsn } from '../../src/dsn';
2-
import { SentryError } from '../../src/error';
33

44
describe('Dsn', () => {
55
describe('fromComponents', () => {

‎packages/core/test/lib/integrations/rewriteframes.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ describe('RewriteFrames', () => {
4444
rewriteFrames = new RewriteFrames();
4545
});
4646

47-
it('transforms messageEvent frames', async () => {
48-
const event = await rewriteFrames.process(messageEvent);
47+
it('transforms messageEvent frames', () => {
48+
const event = rewriteFrames.process(messageEvent);
4949
expect(event.stacktrace!.frames![0].filename).toEqual('app:///file1.js');
5050
expect(event.stacktrace!.frames![1].filename).toEqual('app:///file2.js');
5151
});
5252

53-
it('transforms exceptionEvent frames', async () => {
54-
const event = await rewriteFrames.process(exceptionEvent);
53+
it('transforms exceptionEvent frames', () => {
54+
const event = rewriteFrames.process(exceptionEvent);
5555
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///file1.js');
5656
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///file2.js');
5757
});
@@ -64,14 +64,14 @@ describe('RewriteFrames', () => {
6464
});
6565
});
6666

67-
it('transforms messageEvent frames', async () => {
68-
const event = await rewriteFrames.process(messageEvent);
67+
it('transforms messageEvent frames', () => {
68+
const event = rewriteFrames.process(messageEvent);
6969
expect(event.stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
7070
expect(event.stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
7171
});
7272

73-
it('transforms exceptionEvent frames', async () => {
74-
const event = await rewriteFrames.process(exceptionEvent);
73+
it('transforms exceptionEvent frames', () => {
74+
const event = rewriteFrames.process(exceptionEvent);
7575
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('app:///src/app/file1.js');
7676
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('app:///src/app/file2.js');
7777
});
@@ -80,23 +80,23 @@ describe('RewriteFrames', () => {
8080
describe('can use custom iteratee', () => {
8181
beforeEach(() => {
8282
rewriteFrames = new RewriteFrames({
83-
iteratee: async (frame: StackFrame) => ({
83+
iteratee: (frame: StackFrame) => ({
8484
...frame,
8585
function: 'whoops',
8686
}),
8787
});
8888
});
8989

9090
it('transforms messageEvent frames', async () => {
91-
const event = await rewriteFrames.process(messageEvent);
91+
const event = rewriteFrames.process(messageEvent);
9292
expect(event.stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
9393
expect(event.stacktrace!.frames![0].function).toEqual('whoops');
9494
expect(event.stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');
9595
expect(event.stacktrace!.frames![1].function).toEqual('whoops');
9696
});
9797

9898
it('transforms exceptionEvent frames', async () => {
99-
const event = await rewriteFrames.process(exceptionEvent);
99+
const event = rewriteFrames.process(exceptionEvent);
100100
expect(event.exception!.values![0].stacktrace!.frames![0].filename).toEqual('/www/src/app/file1.js');
101101
expect(event.exception!.values![0].stacktrace!.frames![0].function).toEqual('whoops');
102102
expect(event.exception!.values![0].stacktrace!.frames![1].filename).toEqual('/www/src/app/file2.js');

‎packages/hub/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"fix:prettier": "prettier --write \"{src,test}/**/*.ts\"",
4545
"fix:tslint": "tslint --fix -t stylish -p .",
4646
"test": "jest",
47-
"test:watch": "jest --watch --notify"
47+
"test:watch": "jest --watch"
4848
},
4949
"jest": {
5050
"collectCoverage": true,

‎packages/hub/src/scope.ts

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { Breadcrumb, SentryEvent, SentryEventHint, Severity, User } from '@sentry/types';
2-
import { isPromise } from '@sentry/utils/is';
2+
import { isFunction, isThenable } from '@sentry/utils/is';
33
import { getGlobalObject } from '@sentry/utils/misc';
44
import { assign, safeNormalize } from '@sentry/utils/object';
5-
import { QuickPromise } from '@sentry/utils/quickpromise';
5+
import { SyncPromise } from '@sentry/utils/syncpromise';
66

77
export type EventProcessor = (
88
event: SentryEvent,
@@ -75,28 +75,29 @@ export class Scope {
7575
event: SentryEvent | null,
7676
hint?: SentryEventHint,
7777
index: number = 0,
78-
callback?: (result: SentryEvent | null) => void,
79-
): QuickPromise<SentryEvent | null> {
80-
return new QuickPromise<SentryEvent | null>(resolve => {
81-
if (event === null) {
82-
resolve(null);
78+
): SyncPromise<SentryEvent | null> {
79+
return new SyncPromise<SentryEvent | null>((resolve, reject) => {
80+
const processor = processors[index];
81+
if (event === null || !isFunction(processor)) {
82+
resolve(event);
8383
} else {
84-
const processor = processors[index];
85-
if (isPromise(processor)) {
86-
(processor({ ...event }, hint) as Promise<SentryEvent | null>).then((processedEvent: SentryEvent | null) => {
87-
if (index === processors.length - 1 && callback) {
88-
callback(processedEvent);
89-
} else {
90-
this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve);
91-
}
92-
});
93-
} else {
94-
const processedEvent = processor({ ...event }, hint) as SentryEvent | null;
95-
if (index === processors.length - 1 && callback) {
96-
callback(processedEvent);
84+
try {
85+
const result = processor({ ...event }, hint) as SentryEvent | null;
86+
if (isThenable(result)) {
87+
(result as Promise<SentryEvent | null>)
88+
.then((final: SentryEvent | null) => {
89+
this.notifyEventProcessors(processors, final, hint, index + 1)
90+
.then(resolve)
91+
.catch(reject);
92+
})
93+
.catch(reject);
9794
} else {
98-
this.notifyEventProcessors(processors, processedEvent, hint, index + 1, resolve);
95+
this.notifyEventProcessors(processors, result, hint, index + 1)
96+
.then(resolve)
97+
.catch(reject);
9998
}
99+
} catch (e) {
100+
reject(e);
100101
}
101102
}
102103
});
@@ -232,7 +233,7 @@ export class Scope {
232233
event: SentryEvent,
233234
hint?: SentryEventHint,
234235
maxBreadcrumbs?: number,
235-
): QuickPromise<SentryEvent | null> {
236+
): SyncPromise<SentryEvent | null> {
236237
if (this.extra && Object.keys(this.extra).length) {
237238
event.extra = { ...this.extra, ...event.extra };
238239
}

‎packages/hub/test/hub.test.ts

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { logger } from '@sentry/utils/logger';
33
import { getCurrentHub, Hub, Scope } from '../src';
44

55
const clientFn = jest.fn();
6-
const asyncClientFn = async () => Promise.reject('error');
76
const scope = new Scope();
87

98
describe('Hub', () => {
@@ -19,7 +18,6 @@ describe('Hub', () => {
1918

2019
test('pass in filled layer', () => {
2120
const hub = new Hub({
22-
asyncClientFn,
2321
clientFn,
2422
});
2523
expect(hub.getStack()).toHaveLength(1);
@@ -28,7 +26,6 @@ describe('Hub', () => {
2826
test('invoke client sync', () => {
2927
const hub = new Hub(
3028
{
31-
asyncClientFn,
3229
clientFn,
3330
},
3431
scope,
@@ -42,28 +39,13 @@ describe('Hub', () => {
4239

4340
test("don't invoke client sync with wrong func", () => {
4441
const hub = new Hub({
45-
asyncClientFn,
4642
clientFn,
4743
});
4844
// @ts-ignore
4945
hub.invokeClient('funca', true);
5046
expect(clientFn).not.toHaveBeenCalled();
5147
});
5248

53-
test('invoke client async catch error in case', done => {
54-
jest.spyOn(logger, 'error');
55-
const hub = new Hub({
56-
asyncClientFn,
57-
clientFn,
58-
});
59-
(hub as any).invokeClientAsync('asyncClientFn', true);
60-
setTimeout(() => {
61-
// tslint:disable-next-line
62-
expect(logger.error).toHaveBeenCalled();
63-
done();
64-
});
65-
});
66-
6749
test('isOlderThan', () => {
6850
const hub = new Hub();
6951
expect(hub.isOlderThan(0)).toBeFalsy();
@@ -190,7 +172,7 @@ describe('Hub', () => {
190172

191173
test('captureException', () => {
192174
const hub = new Hub();
193-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
175+
const spy = jest.spyOn(hub as any, 'invokeClient');
194176
hub.captureException('a');
195177
expect(spy).toHaveBeenCalled();
196178
expect(spy.mock.calls[0][0]).toBe('captureException');
@@ -199,7 +181,7 @@ describe('Hub', () => {
199181

200182
test('captureMessage', () => {
201183
const hub = new Hub();
202-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
184+
const spy = jest.spyOn(hub as any, 'invokeClient');
203185
hub.captureMessage('a');
204186
expect(spy).toHaveBeenCalled();
205187
expect(spy.mock.calls[0][0]).toBe('captureMessage');
@@ -211,7 +193,7 @@ describe('Hub', () => {
211193
extra: { b: 3 },
212194
};
213195
const hub = new Hub();
214-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
196+
const spy = jest.spyOn(hub as any, 'invokeClient');
215197
hub.captureEvent(event);
216198
expect(spy).toHaveBeenCalled();
217199
expect(spy.mock.calls[0][0]).toBe('captureEvent');
@@ -245,37 +227,38 @@ describe('Hub', () => {
245227
});
246228
});
247229

248-
test('pushScope inherit processors', async () => {
230+
test('pushScope inherit processors', () => {
231+
expect.assertions(1);
249232
const event: SentryEvent = {
250233
extra: { b: 3 },
251234
};
252235
const localScope = new Scope();
253236
localScope.setExtra('a', 'b');
254237
const hub = new Hub({ a: 'b' }, localScope);
255-
const callCounter = jest.fn();
238+
256239
localScope.addEventProcessor(async (processedEvent: SentryEvent) => {
257-
callCounter(1);
258240
processedEvent.dist = '1';
259241
return processedEvent;
260242
});
243+
261244
hub.pushScope();
262245
const pushedScope = hub.getStackTop().scope;
263-
if (pushedScope) {
264-
const final = await pushedScope.applyToEvent(event);
246+
247+
return pushedScope!.applyToEvent(event).then(final => {
265248
expect(final!.dist).toEqual('1');
266-
}
249+
});
267250
});
268251

269252
test('captureException should set event_id in hint', () => {
270253
const hub = new Hub();
271-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
254+
const spy = jest.spyOn(hub as any, 'invokeClient');
272255
hub.captureException('a');
273256
expect(spy.mock.calls[0][2].event_id).toBeTruthy();
274257
});
275258

276259
test('captureMessage should set event_id in hint', () => {
277260
const hub = new Hub();
278-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
261+
const spy = jest.spyOn(hub as any, 'invokeClient');
279262
hub.captureMessage('a');
280263
expect(spy.mock.calls[0][3].event_id).toBeTruthy();
281264
});
@@ -285,7 +268,7 @@ describe('Hub', () => {
285268
extra: { b: 3 },
286269
};
287270
const hub = new Hub();
288-
const spy = jest.spyOn(hub as any, 'invokeClientAsync');
271+
const spy = jest.spyOn(hub as any, 'invokeClient');
289272
hub.captureEvent(event);
290273
expect(spy.mock.calls[0][2].event_id).toBeTruthy();
291274
});

0 commit comments

Comments
 (0)