From afc07f10208b1c8ff400136525dacdea66189a30 Mon Sep 17 00:00:00 2001 From: Brian Kim Date: Wed, 29 Apr 2020 12:24:35 -0400 Subject: [PATCH] catch some unhandled promise rejections --- src/__tests__/errors.tsx | 8 ++------ src/index.ts | 31 +++++++++++++++++-------------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/__tests__/errors.tsx b/src/__tests__/errors.tsx index 11f4f8ea9..a2a77512e 100644 --- a/src/__tests__/errors.tsx +++ b/src/__tests__/errors.tsx @@ -64,7 +64,7 @@ describe("errors", () => { ); }); - // TODO: figure out how to catch the promise rejection + // TODO: figure out how to test for an unhandled promise rejection // eslint-disable-next-line test.skip("async generator throws independently", async () => { const error = new Error("async generator throws independently"); @@ -79,9 +79,7 @@ describe("errors", () => { await new Promise(() => {}); }); - // TODO: figure out why this causes an unhandled rejection - // eslint-disable-next-line - test.skip("async generator throws in async generator", async () => { + test("async generator throws in async generator", async () => { const error = new Error("async generator throws in async generator"); /* eslint-disable require-yield */ async function* Thrower() { @@ -98,8 +96,6 @@ describe("errors", () => { await expect(renderer.render(, document.body)).rejects.toBe( error, ); - - await new Promise(() => {}); }); test("sync function throws, sync generator catches", () => { diff --git a/src/index.ts b/src/index.ts index 0a17e1c1a..69547aa8b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -202,9 +202,9 @@ abstract class ParentNode implements NodeBase { abstract readonly renderer: Renderer; abstract parent: ParentNode | undefined; // When children update asynchronously, we race their result against the next - // update of children. The onNextChildren property is set to the resolve + // update of children. The onNextResult property is set to the resolve // function of the promise which the current update is raced against. - private onNextChildren: + private onNextResult: | ((result?: Promise) => unknown) | undefined = undefined; protected props: Props | undefined = undefined; @@ -473,19 +473,19 @@ abstract class ParentNode implements NodeBase { this.keyedChildren = nextKeyedChildren; if (updates === undefined) { this.commit(); - if (this.onNextChildren !== undefined) { - this.onNextChildren(); - this.onNextChildren = undefined; + if (this.onNextResult !== undefined) { + this.onNextResult(); + this.onNextResult = undefined; } } else { const result = Promise.all(updates).then(() => void this.commit()); // void :( - if (this.onNextChildren !== undefined) { - this.onNextChildren(result); - this.onNextChildren = undefined; + if (this.onNextResult !== undefined) { + this.onNextResult(result.catch(() => undefined)); // void :( + this.onNextResult = undefined; } const nextResult = new Promise( - (resolve) => (this.onNextChildren = resolve), + (resolve) => (this.onNextResult = resolve), ); return Promise.race([result, nextResult]); } @@ -791,8 +791,12 @@ class ComponentNode extends ParentNode { () => undefined, // void :( ); const result = iteration.then((iteration) => { - this.previousResult = this.updateChildren(iteration.value); - return this.previousResult; + const result = this.updateChildren(iteration.value); + if (isPromiseLike(result)) { + this.previousResult = result.catch(() => undefined); // void + } + + return result; }); return [pending, result]; @@ -817,9 +821,8 @@ class ComponentNode extends ParentNode { this.enqueuedPending = undefined; this.enqueuedResult = undefined; if (this.componentType === AsyncGen && !this.finished && !this.unmounted) { - // We catch and rethrow the error to trigger an unhandled promise rejection. - // We should probably do a larger audit of error handling and making sure error messages are helpful, but this - (this.run() as Promise).catch((err) => { + Promise.resolve(this.run()).catch((err) => { + // We catch and rethrow the error to trigger an unhandled promise rejection. if (!this.updating) { throw err; }