Skip to content

Commit

Permalink
catch some unhandled promise rejections
Browse files Browse the repository at this point in the history
  • Loading branch information
brainkim committed Apr 29, 2020
1 parent babb532 commit afc07f1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 20 deletions.
8 changes: 2 additions & 6 deletions src/__tests__/errors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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() {
Expand All @@ -98,8 +96,6 @@ describe("errors", () => {
await expect(renderer.render(<Component />, document.body)).rejects.toBe(
error,
);

await new Promise(() => {});
});

test("sync function throws, sync generator catches", () => {
Expand Down
31 changes: 17 additions & 14 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,9 +202,9 @@ abstract class ParentNode<T> implements NodeBase<T> {
abstract readonly renderer: Renderer<T>;
abstract parent: ParentNode<T> | 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<undefined>) => unknown)
| undefined = undefined;
protected props: Props | undefined = undefined;
Expand Down Expand Up @@ -473,19 +473,19 @@ abstract class ParentNode<T> implements NodeBase<T> {
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<undefined>(
(resolve) => (this.onNextChildren = resolve),
(resolve) => (this.onNextResult = resolve),
);
return Promise.race([result, nextResult]);
}
Expand Down Expand Up @@ -791,8 +791,12 @@ class ComponentNode<T> extends ParentNode<T> {
() => 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];
Expand All @@ -817,9 +821,8 @@ class ComponentNode<T> extends ParentNode<T> {
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<any>).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;
}
Expand Down

0 comments on commit afc07f1

Please sign in to comment.