Skip to content

Commit 2cfa4f9

Browse files
author
Mat Jones
committed
fix race condition between catch and finally in do-try.ts
1 parent 8e2ba60 commit 2cfa4f9

3 files changed

Lines changed: 38 additions & 7 deletions

File tree

src/utilities/core-utils.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,14 @@ const _sleep = (milliseconds: number, debug: boolean = false) => {
9292
);
9393
};
9494

95+
const _sleepSync = (milliseconds: number) => {
96+
let now = Date.now(),
97+
start = now;
98+
while (now - start < milliseconds) {
99+
now = Date.now();
100+
}
101+
};
102+
95103
/**
96104
* Creates a timer instance that when stopped will supply elapsed time in milliseconds.
97105
* Useful for benchmarking or providing counters
@@ -134,6 +142,7 @@ export const CoreUtils = {
134142
objectToArray: _objectToArray,
135143
range: _.range,
136144
sleep: _sleep,
145+
sleepSync: _sleepSync,
137146
throttle: _.throttle,
138147
timer: _timer,
139148
times: _.times,

src/utilities/do-try.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,33 @@ import {
77
import { Do, DoSync } from "../utilities/do-try";
88
import { PolyfillUtils } from "../utilities/polyfill-utils";
99
import { StubResourceRecord } from "../tests/stubs/stub-resource-record";
10+
import { CoreUtils } from "../utilities/core-utils";
1011

1112
PolyfillUtils.registerPromiseFinallyPolyfill();
1213

1314
describe("do-try.ts", () => {
1415
describe("Do.try", () => {
16+
it("When a catch handler exists, finally is called after catch resolves", async () => {
17+
// Arrange
18+
let catchRanAtTimestamp: number;
19+
let finallyRanAtTimestamp: number;
20+
const workload = async () => {
21+
throw Error();
22+
};
23+
24+
// Act
25+
await Do.try(workload)
26+
.catch(() => {
27+
catchRanAtTimestamp = Date.now();
28+
CoreUtils.sleepSync(1000);
29+
})
30+
.finally(() => (finallyRanAtTimestamp = Date.now()))
31+
.getAwaiter();
32+
33+
// Assert
34+
expect(catchRanAtTimestamp).toBeLessThan(finallyRanAtTimestamp);
35+
});
36+
1537
it("When validation errors occur (i.e. error is a ResultRecord), then passes typed errors to catch handler", async () => {
1638
// Arrange
1739
const workload = async () => {
@@ -54,7 +76,7 @@ describe("do-try.ts", () => {
5476
it("When no errors occur, then catch handler is never called", async () => {
5577
// Arrange
5678
const catchHandler = jest.fn();
57-
const workload = jest.fn();
79+
const workload = async () => jest.fn()();
5880

5981
// Act
6082
await Do.try(workload)
@@ -68,7 +90,7 @@ describe("do-try.ts", () => {
6890
it("When no errors occur, then finally handler is still called", async () => {
6991
// Arrange
7092
const finallyHandler = jest.fn();
71-
const workload = jest.fn();
93+
const workload = async () => jest.fn()();
7294

7395
// Act
7496
await Do.try(workload)

src/utilities/do-try.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ import {
1717
* - error?: any -- if it's a Javascript error, will not be null
1818
*/
1919
class Do<TResourceType, TReturnVal = void> {
20-
private readonly promise: Promise<TReturnVal>;
20+
private promise: Promise<TReturnVal>;
2121

2222
private constructor(workload: AsyncWorkload<TReturnVal>) {
23-
this.promise = Promise.resolve(workload());
23+
this.promise = workload();
2424
}
2525

2626
/**
@@ -35,14 +35,14 @@ class Do<TResourceType, TReturnVal = void> {
3535
public catch(
3636
errorHandler: CatchHandler<TResourceType>
3737
): Do<TResourceType, TReturnVal> {
38-
this.promise.catch((err: any) => {
38+
this.promise = this.promise.catch((err: any) => {
3939
if (err instanceof ResultRecord) {
4040
errorHandler(err, undefined);
4141
return;
4242
}
4343

4444
errorHandler(undefined, err);
45-
});
45+
}) as Promise<TReturnVal>;
4646

4747
return this;
4848
}
@@ -56,7 +56,7 @@ class Do<TResourceType, TReturnVal = void> {
5656
public finally(
5757
finallyHandler: FinallyHandler
5858
): Do<TResourceType, TReturnVal> {
59-
this.promise.finally(finallyHandler);
59+
this.promise = this.promise.finally(finallyHandler);
6060
return this;
6161
}
6262

0 commit comments

Comments
 (0)