From e1b803e74103065ee73df997e02297df7cef699e Mon Sep 17 00:00:00 2001 From: Chris Garrett Date: Wed, 14 Oct 2020 18:44:14 -0700 Subject: [PATCH] [BUGFIX lts] Cleans up the DebugRenderTree more thoroughly on errors 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. --- .../glimmer/lib/utils/debug-render-tree.ts | 9 ++++++++ .../application/debug-render-tree-test.ts | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/@ember/-internals/glimmer/lib/utils/debug-render-tree.ts b/packages/@ember/-internals/glimmer/lib/utils/debug-render-tree.ts index 51437f49dc0..a47ad15d265 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/debug-render-tree.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/debug-render-tree.ts @@ -128,6 +128,15 @@ export default class DebugRenderTree { // 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(); } diff --git a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts index 01227f70938..70784295787 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/application/debug-render-tree-test.ts @@ -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` + + ` + ); + + 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'); + } } ); }