Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(SingleInstancePromise): Make run() reject when the function rejects #940

Merged
merged 1 commit into from
Jun 11, 2024
Merged

fix(SingleInstancePromise): Make run() reject when the function rejects #940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 68 additions & 37 deletions src/util/SingleInstancePromise.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,25 @@
/**
* @typedef QueueEntry
* @property {(result: TReturn) => void} resolve
* @property {(result: unknown) => void} reject
* @property {TArgs} args
*/

#promiseFn;

#once;
get once() {
return this.#once;
}

Check warning on line 20 in src/util/SingleInstancePromise.js

View check run for this annotation

Codecov / codecov/patch

src/util/SingleInstancePromise.js#L18-L20 

Added lines #L18 - L20 were not covered by tests

/** @type {QueueEntry[]} */
#queue = [];
#isEmptyingQueue = false;
/** @type {{resolved: boolean, result: TReturn} | undefined} */
#onceReturnValue = undefined;
/** @type {Set<() => void>} */
#onFinishCbs = new Set();

/**
* @param {TFunc} promiseFn
* @param {object} opts
Expand All @@ -20,17 +36,10 @@
constructor(promiseFn, {
once = false,
} = {}) {
this.once = once;
this.promiseFn = promiseFn;
this.#once = once;
this.#promiseFn = promiseFn;

/** @type {QueueEntry[]} */
this._queue = [];
this._isEmptyingQueue = false;
this.hasRun = false;
/** @type {TReturn | undefined} */
this._onceReturnValue = undefined;
/** @type {Set<() => void>} */
this._onFinishCbs = new Set();
}

/**
Expand All @@ -42,12 +51,16 @@
* @returns {Promise<TReturn>}
*/
async run(...args) {
if (this.hasRun && this.once) {
return /** @type {TReturn} */ (this._onceReturnValue);
if (this.hasRun && this.#once && this.#onceReturnValue) {
if (this.#onceReturnValue.resolved) {
return /** @type {TReturn} */ (this.#onceReturnValue);
} else {
throw this.#onceReturnValue.result;
}
}

/** @type {Promise<TReturn>} */
const myPromise = new Promise((resolve) => this._queue.push({ resolve, args }));
const myPromise = new Promise((resolve, reject) => this.#queue.push({ resolve, reject, args }));
this._emptyQueue();
return await myPromise;
}
Expand All @@ -56,37 +69,55 @@
* @private
*/
async _emptyQueue() {
if (this._isEmptyingQueue) return;
this._isEmptyingQueue = true;

while (this._queue.length > 0) {
if (this.once && this.hasRun) {
const returnValue = /** @type {TReturn} */ (this._onceReturnValue);
this._queue.forEach((entry) => entry.resolve(returnValue));
this._queue = [];
if (this.#isEmptyingQueue) return;
this.#isEmptyingQueue = true;

while (this.#queue.length > 0) {
if (this.#once && this.hasRun && this.#onceReturnValue) {
if (this.#onceReturnValue.resolved) {
const returnValue = /** @type {TReturn} */ (this.#onceReturnValue.result);
this.#queue.forEach((entry) => entry.resolve(returnValue));
} else {
const error = this.#onceReturnValue.result;
this.#queue.forEach((entry) => entry.reject(error));
}

Check warning on line 83 in src/util/SingleInstancePromise.js

View check run for this annotation

Codecov / codecov/patch

src/util/SingleInstancePromise.js#L80-L83 

Added lines #L80 - L83 were not covered by tests
this.#queue = [];
break;
}
const queueCopy = this._queue;
this._queue = [];
const queueCopy = this.#queue;
this.#queue = [];

const lastEntry = /** @type {QueueEntry} */ (queueCopy.at(-1));

this._isEmptyingQueue = true;
const result = await this.promiseFn(...lastEntry.args);
this._isEmptyingQueue = false;
this.#isEmptyingQueue = true;
let resolved = false;
let result;
try {
result = await this.#promiseFn(...lastEntry.args);
resolved = true;
} catch (e) {
result = e;
}
this.#isEmptyingQueue = false;
this.hasRun = true;
this._onFinishCbs.forEach((cb) => cb());
this._onFinishCbs.clear();
this.#onFinishCbs.forEach((cb) => cb());
this.#onFinishCbs.clear();

if (this.once) {
this._onceReturnValue = result;
if (this.#once) {
this.#onceReturnValue = { resolved, result };
}

for (const { resolve } of queueCopy) {
resolve(result);
if (resolved) {
for (const { resolve } of queueCopy) {
resolve(result);
}
} else {
for (const { reject } of queueCopy) {
reject(result);
}
}
}
this._isEmptyingQueue = false;
this.#isEmptyingQueue = false;
}

/**
Expand All @@ -111,11 +142,11 @@
* @returns {Promise<void>}
*/
async waitForFinish() {
if (this.once) {
if (this.#once) {
throw new Error("waitForFinish() would stay pending forever when once has been set, use waitForFinishOnce() instead.");
}
/** @type {Promise<void>} */
const promise = new Promise((r) => this._onFinishCbs.add(r));
const promise = new Promise((r) => this.#onFinishCbs.add(r));
await promise;
}

Expand Down Expand Up @@ -146,7 +177,7 @@
async waitForFinishOnce() {
if (this.hasRun) return;
/** @type {Promise<void>} */
const promise = new Promise((r) => this._onFinishCbs.add(r));
const promise = new Promise((r) => this.#onFinishCbs.add(r));
await promise;
}

Expand Down Expand Up @@ -177,9 +208,9 @@
* ```
*/
async waitForFinishIfRunning() {
while (this._isEmptyingQueue) {
while (this.#isEmptyingQueue) {
/** @type {Promise<void>} */
const promise = new Promise((r) => this._onFinishCbs.add(r));
const promise = new Promise((r) => this.#onFinishCbs.add(r));
await promise;
}
}
Expand Down
134 changes: 126 additions & 8 deletions test/unit/src/util/SingleInstancePromise.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@ import { assertPromiseResolved } from "../../../../src/util/asserts.js";
import { waitForMicrotasks } from "../../../../src/util/waitForMicroTasks.js";

function basicSpyFn() {
/** @type {((result: string) => void)?} */
let resolvePromise = null;
/** @type {(result: string) => void} */
let resolvePromise;
/** @type {(result: Error) => void} */
let rejectPromise;
/**
* @param {string} param
*/
const fn = async (param) => {
/** @type {Promise<string>} */
const promise = new Promise((r) => {
resolvePromise = r;
const promise = new Promise((resolve, reject) => {
resolvePromise = resolve;
rejectPromise = reject;
});
const promiseResult = await promise;
return param + promiseResult;
Expand All @@ -23,11 +26,15 @@ function basicSpyFn() {
/** @param {string} result */
async resolvePromise(result) {
await waitForMicrotasks();
if (!resolvePromise) {
throw new Error("Spy function hasn't been called yet");
}
resolvePromise(result);
},
/**
* @param {Error} result
*/
async rejectPromise(result) {
await waitForMicrotasks();
rejectPromise(result);
},
spyFn,
};
}
Expand Down Expand Up @@ -146,7 +153,7 @@ async function runOnceMatrix(test) {
}

Deno.test({
name: "waitForFinish resolves when the any run is done",
name: "waitForFinish resolves when the run is done",
async fn() {
const basic = basicSpyFn();
const instance = new SingleInstancePromise(basic.spyFn);
Expand All @@ -169,6 +176,36 @@ Deno.test({
},
});

Deno.test({
name: "waitForFinish resolves when the function rejects",
async fn() {
const basic = basicSpyFn();
const instance = new SingleInstancePromise(basic.spyFn);

await assertPromiseResolved(instance.waitForFinish(), false);
const assertRejectsPromise1 = assertRejects(async () => {
await instance.run("");
});
const promise = instance.waitForFinish();
await assertPromiseResolved(promise, false);
await basic.rejectPromise(new Error("The error message"));
await assertPromiseResolved(promise, true);
await assertPromiseResolved(instance.waitForFinish(), false);
await assertRejectsPromise1;

// Running a second time to make sure that waitForFinish() becomes pending again
const assertRejectsPromise2 = assertRejects(async () => {
await instance.run("");
});
const promise2 = instance.waitForFinish();
await assertPromiseResolved(promise2, false);
await basic.rejectPromise(new Error("second error"));
await assertPromiseResolved(promise2, true);
await assertPromiseResolved(instance.waitForFinish(), false);
await assertRejectsPromise2;
},
});

Deno.test({
name: "waitForFinish throws when once is true",
async fn() {
Expand Down Expand Up @@ -201,6 +238,34 @@ Deno.test({
},
});

Deno.test({
name: "waitForFinishOnce resolves when the first run rejects",
async fn() {
await runOnceMatrix(async ({ instance, rejectPromise }) => {
const promise1 = instance.waitForFinishOnce();
await assertPromiseResolved(promise1, false);
const assertRejectsPromise1 = assertRejects(async () => {
await instance.run("");
});
await assertPromiseResolved(promise1, false);
await assertPromiseResolved(instance.waitForFinishOnce(), false);
await rejectPromise(new Error("First error"));
await assertPromiseResolved(promise1, true);
await assertPromiseResolved(instance.waitForFinishOnce(), true);
await assertRejectsPromise1;

// Running a second time to make sure that waitForFinish() stays resolved
const assertRejectsPromise2 = assertRejects(async () => {
await instance.run("");
});
await assertPromiseResolved(instance.waitForFinishOnce(), true);
await rejectPromise(new Error("second error"));
await assertPromiseResolved(instance.waitForFinishOnce(), true);
await assertRejectsPromise2;
});
},
});

Deno.test({
name: "waitForFinishIfRunning resolves when the function is not running",
async fn() {
Expand Down Expand Up @@ -254,6 +319,39 @@ Deno.test({
},
});

Deno.test({
name: "waitForFinishIfRunning resolves when the function rejects",
async fn() {
await runOnceMatrix(async ({ instance, rejectPromise, once }) => {
const promise1 = instance.waitForFinishIfRunning();
await assertPromiseResolved(promise1, true);
await assertPromiseResolved(instance.waitForFinishIfRunning(), true);
const assertRejectsPromise1 = assertRejects(async () => {
await instance.run("");
});
const promise2 = instance.waitForFinishIfRunning();
await assertPromiseResolved(promise2, false);
await assertPromiseResolved(instance.waitForFinishIfRunning(), false);
await rejectPromise(new Error("first error"));
await assertPromiseResolved(promise2, true);
await assertPromiseResolved(instance.waitForFinishIfRunning(), true);
await assertRejectsPromise1;

// Running a second time to make sure that waitForFinishIfRunning() becomes pending again
const assertRejectsPromise2 = assertRejects(async () => {
await instance.run("");
});
const promise3 = instance.waitForFinishIfRunning();
await assertPromiseResolved(promise3, once);
await assertPromiseResolved(instance.waitForFinishIfRunning(), once);
await rejectPromise(new Error("second error"));
await assertPromiseResolved(promise3, true);
await assertPromiseResolved(instance.waitForFinishIfRunning(), true);
await assertRejectsPromise2;
});
},
});

Deno.test({
name: "promises are resolved in the correct order",
async fn() {
Expand Down Expand Up @@ -287,3 +385,23 @@ Deno.test({
});
},
});

Deno.test({
name: "run rejects when the fuction rejects",
async fn() {
await runOnceMatrix(async ({ instance, rejectPromise, once }) => {
const assertRejectsPromise1 = assertRejects(async () => {
await instance.run();
}, Error, "The error message");
await rejectPromise(new Error("The error message"));
await assertRejectsPromise1;

const expectedMessage = once ? "The error message" : "The second error message";
const assertRejectsPromise2 = assertRejects(async () => {
await instance.run();
}, Error, expectedMessage);
await rejectPromise(new Error("The second error message"));
await assertRejectsPromise2;
});
},
});
Loading