Skip to content

Commit

Permalink
[BUGFIX] Prevents infinite rerenders when errors occur during render
Browse files Browse the repository at this point in the history
This adds a preventative measure against infinite rerenders that happen
due to an unrecoverable error during rendering. This type of error is
really annoying from a DX perspective and can really throw developers
for a loop. The most common cases appear to be caused by the Ember
Inspector, which polls after a render has occured whether on not it was
succesful, and in doing so also dirties state, causing a rerender.

An example of this is seen in this [example reproduction](https://github.com/xg-wang/getter-error)
with the following steps:

1. Load the app
2. Click the To View link to enter the page with the error
3. Open up the Ember Inspector to activate it
4. Reload the page
  • Loading branch information
Chris Garrett committed Nov 9, 2020
1 parent 66c56e4 commit 2f8cde9
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
35 changes: 32 additions & 3 deletions packages/@ember/-internals/glimmer/lib/renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getOwner, Owner } from '@ember/-internals/owner';
import { getViewElement, getViewId, OwnedTemplateMeta } from '@ember/-internals/views';
import { assert } from '@ember/debug';
import { backburner, getCurrentRunLoop } from '@ember/runloop';
import { DEBUG } from '@glimmer/env';
import {
Bounds,
Cursor,
Expand Down Expand Up @@ -77,6 +78,34 @@ export class DynamicScope implements GlimmerDynamicScope {
}
}

// This wrapper logic prevents us from rerendering in case of a hard failure
// during render. This prevents infinite revalidation type loops from occuring,
// and ensures that errors are not swallowed by subsequent follow on failures.
function errorLoopTransaction(fn: () => void) {
if (DEBUG) {
return () => {
let didError = true;

try {
fn();
didError = false;
} finally {
if (didError) {
// Noop the function so that we won't keep calling it and causing
// infinite looping failures;
fn = () => {
console.warn(
'Attempted to rerender, but the Ember application has had an unrecoverable error occur during render. You should reload the application after fixing the cause of the error.'
);
};
}
}
};
} else {
return fn;
}
}

class RootState {
public id: string;
public result: RenderResult | undefined;
Expand All @@ -102,7 +131,7 @@ class RootState {
this.result = undefined;
this.destroyed = false;

this.render = () => {
this.render = errorLoopTransaction(() => {
let layout = unwrapTemplate(template).asLayout();
let handle = layout.compile(context);

Expand All @@ -118,8 +147,8 @@ class RootState {
let result = (this.result = iterator.sync());

// override .render function after initial render
this.render = () => result.rerender({ alwaysRevalidate: false });
};
this.render = errorLoopTransaction(() => result.rerender({ alwaysRevalidate: false }));
});
}

isFor(possibleRoot: unknown): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1662,7 +1662,7 @@ if (ENV._DEBUG_RENDER_TREE) {
template: 'Hello World',
});

await assert.rejectsAssertion(this.visit('/'), /oops!/);
await assert.rejects(this.visit('/'), /oops!/);

assert.deepEqual(captureRenderTree(this.owner), [], 'there was no output');
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { DEBUG } from '@glimmer/env';

import { moduleFor, RenderingTestCase, runTask } from 'internal-test-helpers';

import { set } from '@ember/-internals/metal';
Expand Down Expand Up @@ -43,7 +45,11 @@ moduleFor(

runTask(() => set(this.context, 'switch', true));

this.assertText('hello');
if (DEBUG) {
this.assertText('', 'it does not rerender after error in development');
} else {
this.assertText('hello', 'it rerenders after error in production');
}
}

['@skip it can recover resets the transaction when an error is thrown during rerender'](
Expand Down Expand Up @@ -91,7 +97,11 @@ moduleFor(

runTask(() => set(this.context, 'switch', true));

this.assertText('hello');
if (DEBUG) {
this.assertText('', 'it does not rerender after error in development');
} else {
this.assertText('hello', 'it does rerender after error in production');
}
}

['@test it can recover resets the transaction when an error is thrown during didInsertElement'](
Expand Down

0 comments on commit 2f8cde9

Please sign in to comment.