Skip to content

Commit 221551c

Browse files
amortemousqueamortemousque
authored
✅ [RUM-6813] Use promise in collectAsyncCalls instead of a callback (#3168)
1 parent dbb1690 commit 221551c

File tree

6 files changed

+129
-140
lines changed

6 files changed

+129
-140
lines changed

‎packages/core/src/domain/error/trackRuntimeError.spec.ts

Lines changed: 33 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,16 @@ describe('trackRuntimeError', () => {
3131
window.onerror = originalOnErrorHandler
3232
})
3333

34-
it('should collect unhandled error', (done) => {
34+
it('should collect unhandled error', async () => {
3535
setTimeout(() => {
3636
throw new Error(ERROR_MESSAGE)
3737
})
38-
collectAsyncCalls(onErrorSpy, 1, () => {
39-
expect(notifyError).toHaveBeenCalledOnceWith(jasmine.objectContaining({ message: ERROR_MESSAGE }))
40-
done()
41-
})
38+
39+
await collectAsyncCalls(onErrorSpy)
40+
expect(notifyError).toHaveBeenCalledOnceWith(jasmine.objectContaining({ message: ERROR_MESSAGE }))
4241
})
4342

44-
it('should collect unhandled rejection', (done) => {
43+
it('should collect unhandled rejection', async () => {
4544
if (!('onunhandledrejection' in window)) {
4645
pending('onunhandledrejection not supported')
4746
}
@@ -52,10 +51,8 @@ describe('trackRuntimeError', () => {
5251
void Promise.reject(new Error(ERROR_MESSAGE))
5352
})
5453

55-
collectAsyncCalls(onUnhandledrejectionSpy, 1, () => {
56-
expect(notifyError).toHaveBeenCalledOnceWith(jasmine.objectContaining({ message: ERROR_MESSAGE }))
57-
done()
58-
})
54+
await collectAsyncCalls(onUnhandledrejectionSpy)
55+
expect(notifyError).toHaveBeenCalledOnceWith(jasmine.objectContaining({ message: ERROR_MESSAGE }))
5956
})
6057
})
6158

@@ -82,55 +79,50 @@ describe('instrumentOnError', () => {
8279
stopCollectingUnhandledError()
8380
})
8481

85-
it('should call original error handler', (done) => {
82+
it('should call original error handler', async () => {
8683
setTimeout(() => {
8784
throw new Error(ERROR_MESSAGE)
8885
})
89-
collectAsyncCalls(onErrorSpy, 1, () => {
90-
expect(onErrorSpy).toHaveBeenCalled()
91-
done()
92-
})
86+
87+
await collectAsyncCalls(onErrorSpy)
88+
expect(onErrorSpy).toHaveBeenCalled()
9389
})
9490

95-
it('should notify unhandled error instance', (done) => {
91+
it('should notify unhandled error instance', async () => {
9692
const error = new Error(ERROR_MESSAGE)
9793
setTimeout(() => {
9894
throw error
9995
})
100-
collectAsyncCalls(onErrorSpy, 1, () => {
101-
const [stack, originalError] = callbackSpy.calls.mostRecent().args
102-
expect(originalError).toBe(error)
103-
expect(stack).toBeDefined()
104-
done()
105-
})
96+
97+
await collectAsyncCalls(onErrorSpy)
98+
expect(onErrorSpy).toHaveBeenCalled()
99+
const [stack, originalError] = callbackSpy.calls.mostRecent().args
100+
expect(originalError).toBe(error)
101+
expect(stack).toBeDefined()
106102
})
107103

108-
it('should notify unhandled string', (done) => {
104+
it('should notify unhandled string', async () => {
109105
const error = 'foo' as any
110106
setTimeout(() => {
111107
// eslint-disable-next-line no-throw-literal
112108
throw error
113109
})
114-
collectAsyncCalls(onErrorSpy, 1, () => {
115-
const [stack, originalError] = callbackSpy.calls.mostRecent().args
116-
expect(originalError).toBe(error)
117-
expect(stack).toBeDefined()
118-
done()
119-
})
110+
await collectAsyncCalls(onErrorSpy)
111+
const [stack, originalError] = callbackSpy.calls.mostRecent().args
112+
expect(originalError).toBe(error)
113+
expect(stack).toBeDefined()
120114
})
121115

122-
it('should notify unhandled object', (done) => {
116+
it('should notify unhandled object', async () => {
123117
const error = { a: 'foo' } as any
124118
setTimeout(() => {
125119
// eslint-disable-next-line no-throw-literal
126120
throw error
127121
})
128-
collectAsyncCalls(onErrorSpy, 1, () => {
129-
const [stack, originalError] = callbackSpy.calls.mostRecent().args
130-
expect(originalError).toBe(error)
131-
expect(stack).toBeDefined()
132-
done()
133-
})
122+
await collectAsyncCalls(onErrorSpy)
123+
const [stack, originalError] = callbackSpy.calls.mostRecent().args
124+
expect(originalError).toBe(error)
125+
expect(stack).toBeDefined()
134126
})
135127

136128
describe('uncaught exception handling', () => {
@@ -164,18 +156,16 @@ describe('instrumentOnError', () => {
164156
})
165157

166158
describe('should handle direct onerror calls', () => {
167-
it('with objects', (done) => {
159+
it('with objects', async () => {
168160
const error = { foo: 'bar' } as any
169161
setTimeout(() => {
170162
window.onerror!(error)
171163
})
172164

173-
collectAsyncCalls(onErrorSpy, 1, () => {
174-
const [stack, originalError] = callbackSpy.calls.mostRecent().args
175-
expect(originalError).toBe(error)
176-
expect(stack).toBeDefined()
177-
done()
178-
})
165+
await collectAsyncCalls(onErrorSpy)
166+
const [stack, originalError] = callbackSpy.calls.mostRecent().args
167+
expect(originalError).toBe(error)
168+
expect(stack).toBeDefined()
179169
})
180170

181171
describe('with undefined arguments', () => {

‎packages/core/src/transport/httpRequest.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('httpRequest', () => {
7070
})
7171
})
7272

73-
it('should use retry strategy', (done) => {
73+
it('should use retry strategy', async () => {
7474
if (!interceptor.isFetchKeepAliveSupported()) {
7575
pending('no fetch keepalive support')
7676
}
@@ -90,7 +90,7 @@ describe('httpRequest', () => {
9090

9191
request.send({ data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: 10 })
9292

93-
collectAsyncCalls(fetchSpy, 2, () => done())
93+
await collectAsyncCalls(fetchSpy, 2)
9494
})
9595
})
9696

‎packages/core/test/collectAsyncCalls.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,34 @@ import { getCurrentJasmineSpec } from './getCurrentJasmineSpec'
22

33
export function collectAsyncCalls<F extends jasmine.Func>(
44
spy: jasmine.Spy<F>,
5-
expectedCallsCount: number,
6-
callback: (calls: jasmine.Calls<F>) => void
7-
) {
8-
const currentSpec = getCurrentJasmineSpec()
9-
if (!currentSpec) {
10-
throw new Error('collectAsyncCalls should be called within jasmine code')
11-
}
5+
expectedCallsCount = 1
6+
): Promise<jasmine.Calls<F>> {
7+
return new Promise((resolve, reject) => {
8+
const currentSpec = getCurrentJasmineSpec()
9+
if (!currentSpec) {
10+
reject(new Error('collectAsyncCalls should be called within jasmine code'))
11+
return
12+
}
1213

13-
if (spy.calls.count() === expectedCallsCount) {
14-
spy.and.callFake(extraCallDetected as F)
15-
callback(spy.calls)
16-
} else if (spy.calls.count() > expectedCallsCount) {
17-
extraCallDetected()
18-
} else {
19-
spy.and.callFake((() => {
14+
const checkCalls = () => {
2015
if (spy.calls.count() === expectedCallsCount) {
2116
spy.and.callFake(extraCallDetected as F)
22-
callback(spy.calls)
17+
resolve(spy.calls)
18+
} else if (spy.calls.count() > expectedCallsCount) {
19+
extraCallDetected()
2320
}
21+
}
22+
23+
checkCalls()
24+
25+
spy.and.callFake((() => {
26+
checkCalls()
2427
}) as F)
25-
}
2628

27-
function extraCallDetected() {
28-
fail(`Unexpected extra call for spec '${currentSpec!.fullName}'`)
29-
}
29+
function extraCallDetected() {
30+
const message = `Unexpected extra call for spec '${currentSpec!.fullName}'`
31+
fail(message)
32+
reject(new Error(message))
33+
}
34+
})
3035
}

‎packages/rum/src/boot/startRecording.spec.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,7 @@ describe('startRecording', () => {
252252
}
253253

254254
async function readSentRequests(expectedSentRequestCount: number) {
255-
const calls = await new Promise<jasmine.Calls<HttpRequest['sendOnExit']>>((resolve) =>
256-
collectAsyncCalls(requestSendSpy, expectedSentRequestCount, resolve)
257-
)
255+
const calls = await collectAsyncCalls(requestSendSpy, expectedSentRequestCount)
258256
return Promise.all(calls.all().map((call) => readReplayPayload(call.args[0])))
259257
}
260258
})

‎packages/rum/src/domain/record/record.spec.ts

Lines changed: 67 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ describe('record', () => {
4040
})
4141
})
4242

43-
it('captures stylesheet rules', (done) => {
43+
it('captures stylesheet rules', async () => {
4444
const styleElement = appendElement('<style></style>') as HTMLStyleElement
4545

4646
startRecording()
@@ -59,92 +59,88 @@ describe('record', () => {
5959
styleSheet.insertRule('body { color: #ccc; }')
6060
}, 10)
6161

62-
collectAsyncCalls(emitSpy, recordsPerFullSnapshot() + 6, () => {
63-
const records = getEmittedRecords()
64-
let i = 0
62+
await collectAsyncCalls(emitSpy, recordsPerFullSnapshot() + 6)
6563

66-
expect(records[i++].type).toEqual(RecordType.Meta)
67-
expect(records[i++].type).toEqual(RecordType.Focus)
68-
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
64+
const records = getEmittedRecords()
65+
let i = 0
6966

70-
if (window.visualViewport) {
71-
expect(records[i++].type).toEqual(RecordType.VisualViewport)
72-
}
67+
expect(records[i++].type).toEqual(RecordType.Meta)
68+
expect(records[i++].type).toEqual(RecordType.Focus)
69+
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
7370

74-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
75-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
76-
jasmine.objectContaining({
77-
source: IncrementalSource.StyleSheetRule,
78-
adds: [{ rule: 'body { background: #000; }', index: undefined }],
79-
})
80-
)
81-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
82-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
83-
jasmine.objectContaining({
84-
source: IncrementalSource.StyleSheetRule,
85-
adds: [{ rule: 'body { background: #111; }', index: undefined }],
86-
})
87-
)
88-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
89-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
90-
jasmine.objectContaining({
91-
source: IncrementalSource.StyleSheetRule,
92-
removes: [{ index: 0 }],
93-
})
94-
)
95-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
96-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
97-
jasmine.objectContaining({
98-
source: IncrementalSource.StyleSheetRule,
99-
adds: [{ rule: 'body { color: #fff; }', index: undefined }],
100-
})
101-
)
102-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
103-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
104-
jasmine.objectContaining({
105-
source: IncrementalSource.StyleSheetRule,
106-
removes: [{ index: 0 }],
107-
})
108-
)
109-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
110-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
111-
jasmine.objectContaining({
112-
source: IncrementalSource.StyleSheetRule,
113-
adds: [{ rule: 'body { color: #ccc; }', index: undefined }],
114-
})
115-
)
71+
if (window.visualViewport) {
72+
expect(records[i++].type).toEqual(RecordType.VisualViewport)
73+
}
11674

117-
done()
118-
})
75+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
76+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
77+
jasmine.objectContaining({
78+
source: IncrementalSource.StyleSheetRule,
79+
adds: [{ rule: 'body { background: #000; }', index: undefined }],
80+
})
81+
)
82+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
83+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
84+
jasmine.objectContaining({
85+
source: IncrementalSource.StyleSheetRule,
86+
adds: [{ rule: 'body { background: #111; }', index: undefined }],
87+
})
88+
)
89+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
90+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
91+
jasmine.objectContaining({
92+
source: IncrementalSource.StyleSheetRule,
93+
removes: [{ index: 0 }],
94+
})
95+
)
96+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
97+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
98+
jasmine.objectContaining({
99+
source: IncrementalSource.StyleSheetRule,
100+
adds: [{ rule: 'body { color: #fff; }', index: undefined }],
101+
})
102+
)
103+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
104+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
105+
jasmine.objectContaining({
106+
source: IncrementalSource.StyleSheetRule,
107+
removes: [{ index: 0 }],
108+
})
109+
)
110+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
111+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data).toEqual(
112+
jasmine.objectContaining({
113+
source: IncrementalSource.StyleSheetRule,
114+
adds: [{ rule: 'body { color: #ccc; }', index: undefined }],
115+
})
116+
)
119117
})
120118

121-
it('flushes pending mutation records before taking a full snapshot', (done) => {
119+
it('flushes pending mutation records before taking a full snapshot', async () => {
122120
startRecording()
123121

124122
appendElement('<hr/>')
125123

126124
// trigger full snapshot by starting a new view
127125
newView()
128126

129-
collectAsyncCalls(emitSpy, 1 + 2 * recordsPerFullSnapshot(), () => {
130-
const records = getEmittedRecords()
131-
let i = 0
127+
await collectAsyncCalls(emitSpy, 1 + 2 * recordsPerFullSnapshot())
132128

133-
expect(records[i++].type).toEqual(RecordType.Meta)
134-
expect(records[i++].type).toEqual(RecordType.Focus)
135-
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
129+
const records = getEmittedRecords()
130+
let i = 0
136131

137-
if (window.visualViewport) {
138-
expect(records[i++].type).toEqual(RecordType.VisualViewport)
139-
}
140-
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
141-
expect((records[i++] as BrowserIncrementalSnapshotRecord).data.source).toEqual(IncrementalSource.Mutation)
142-
expect(records[i++].type).toEqual(RecordType.Meta)
143-
expect(records[i++].type).toEqual(RecordType.Focus)
144-
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
132+
expect(records[i++].type).toEqual(RecordType.Meta)
133+
expect(records[i++].type).toEqual(RecordType.Focus)
134+
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
145135

146-
done()
147-
})
136+
if (window.visualViewport) {
137+
expect(records[i++].type).toEqual(RecordType.VisualViewport)
138+
}
139+
expect(records[i].type).toEqual(RecordType.IncrementalSnapshot)
140+
expect((records[i++] as BrowserIncrementalSnapshotRecord).data.source).toEqual(IncrementalSource.Mutation)
141+
expect(records[i++].type).toEqual(RecordType.Meta)
142+
expect(records[i++].type).toEqual(RecordType.Focus)
143+
expect(records[i++].type).toEqual(RecordType.FullSnapshot)
148144
})
149145

150146
describe('Shadow dom', () => {

0 commit comments

Comments
 (0)