Skip to content

Commit

Permalink
[BUGFIX lts] Cleans up the DebugRenderTree more thoroughly on errors
Browse files Browse the repository at this point in the history
The DebugRenderTree is currently left in a bad state after an error
occurs during render. Specifically, if an error occurs and we call
`capture` on the tree, it will throw errors because the state hasn't
been setup correctly. In real world development, this can happen
whenever the Ember Inspector is open and an app has errored, as the
inspector polls regardless of the state of the app and doesn't take
error state into account. This results in a follow on error to the
original error, which is confusing and can make it more difficult to
debug.

This fixes the issue by removing the affected root altogether, so that
it will not attempt to capture in the first place.
  • Loading branch information
Chris Garrett committed Oct 15, 2020
1 parent da458df commit 64902de
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { assert } from '@ember/debug';
import { Arguments, Bounds, CapturedArguments, Option } from '@glimmer/interfaces';
import { reifyArgs } from '@glimmer/runtime';
import { destroy, reifyArgs } from '@glimmer/runtime';
import { expect, Stack, unwrapTemplate } from '@glimmer/util';
import { SimpleElement, SimpleNode } from '@simple-dom/interface';
import { OwnedTemplate } from '../template';
Expand Down Expand Up @@ -128,6 +128,15 @@ export default class DebugRenderTree<Bucket extends object = object> {

// TODO: We could warn here? But this happens all the time in our tests?

// Clean up the root reference to prevent errors from happening if we
// attempt to capture the render tree (Ember Inspector may do this)
let root = expect(this.stack.toArray()[0], 'expected root state when resetting render tree');
let ref = this.refs.get(root);

if (ref !== undefined) {
this.roots.delete(ref);
}

while (!this.stack.isEmpty()) {
this.stack.pop();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1644,6 +1644,28 @@ if (ENV._DEBUG_RENDER_TREE) {
this.assert.strictEqual(actual, expected, `Matching ${path}`);
}
}

async '@test cleans up correctly after errors'(assert: Assert) {
this.addTemplate(
'application',
strip`
<HelloWorld @name="first" />
`
);

this.addComponent('hello-world', {
ComponentClass: Component.extend({
init() {
throw new Error('oops!');
}
}),
template: 'Hello World',
});

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

assert.deepEqual(captureRenderTree(this.owner), [], 'there was no output');
}
}
);
}

0 comments on commit 64902de

Please sign in to comment.