From f6d03642fedb2b3781691788fe3f9e6794d3892b Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 7 Nov 2024 14:04:52 -0500 Subject: [PATCH 1/2] WIP error trap --- packages/host/app/components/error-trap.gts | 112 ++++++++++++++ .../components/error-trap-test.gts | 143 ++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 packages/host/app/components/error-trap.gts create mode 100644 packages/host/tests/integration/components/error-trap-test.gts diff --git a/packages/host/app/components/error-trap.gts b/packages/host/app/components/error-trap.gts new file mode 100644 index 0000000000..b1966082c9 --- /dev/null +++ b/packages/host/app/components/error-trap.gts @@ -0,0 +1,112 @@ +import { getComponentTemplate } from '@ember/component'; + +import type Owner from '@ember/owner'; +import { getOwner } from '@ember/owner'; +import Component from '@glimmer/component'; +// @ts-expect-error +import { createConstRef } from '@glimmer/reference'; +// @ts-expect-error +import { renderMain, inTransaction } from '@glimmer/runtime'; + +import { type ComponentLike } from '@glint/template'; + +import { modifier } from 'ember-modifier'; + +import { tracked } from 'tracked-built-ins'; + +import { CardError } from '@cardstack/runtime-common/error'; + +function render( + element: Element, + owner: Owner, + Content: ComponentLike<{ Args: { params: Params } }>, + params: Params, +): void { + // this needs to be a template-only component because the way we're invoking it + // just grabs the template and would drop any associated class. + const root = ; + + // clear any previous render work + removeChildren(element); + + let { _runtime, _context, _owner, _builder } = owner.lookup( + 'renderer:-dom', + ) as any; + let self = createConstRef({}, 'this'); + let layout = (getComponentTemplate as any)(root)(_owner).asLayout(); + let iterator = renderMain( + _runtime, + _context, + _owner, + self, + _builder(_runtime.env, { element }), + layout, + ); + let vm = (iterator as any).vm; + + try { + inTransaction(_runtime.env, () => vm._execute()); + } catch (err: any) { + // This is to compensate for the commitCacheGroup op code that is not called because + // of the error being thrown here. we do this so we can keep the TRANSACTION_STACK + // balanced (which would otherwise cause consumed tags to leak into subsequent frames). + // I'm not adding this to a "finally" because when there is no error, the VM will + // process an op code that will do this organically. It's only when there is an error + // that we need to step in and do this by hand. Within the vm[STACKS] is a the stack + // for the cache group. We need to call a commit for each item in this stack. + let vmSymbols = Object.fromEntries( + Object.getOwnPropertySymbols(vm).map((s) => [s.toString(), s]), + ); + let stacks = vm[vmSymbols['Symbol(STACKS)']]; + let stackSize = stacks.cache.stack.length; + for (let i = 0; i < stackSize; i++) { + vm.commitCacheGroup(); + } + + let error = new CardError( + `Encountered error rendering HTML for card: ${err.message}`, + ); + error.additionalErrors = [err]; + throw error; + } +} + +function removeChildren(element: Element) { + let child = element.firstChild; + while (child) { + element.removeChild(child); + child = element.firstChild; + } +} + +export default class ErrorTrap extends Component<{ + Args: { + content: ComponentLike<{ Args: { params: T } }>; + params: T; + }; +}> { + @tracked failed = false; + + firstRender = true; + + render = modifier((element) => { + if (!this.firstRender) { + return; + } + this.firstRender = false; + try { + render(element, getOwner(this)!, this.args.content, this.args.params); + this.failed = false; + } catch (err) { + removeChildren(element); + this.failed = true; + } + }); + + +} diff --git a/packages/host/tests/integration/components/error-trap-test.gts b/packages/host/tests/integration/components/error-trap-test.gts new file mode 100644 index 0000000000..d6023ca81d --- /dev/null +++ b/packages/host/tests/integration/components/error-trap-test.gts @@ -0,0 +1,143 @@ +import { TemplateOnlyComponent } from '@ember/component/template-only'; + +import { settled } from '@ember/test-helpers'; +import Component from '@glimmer/component'; + +import { module, test } from 'qunit'; + +import { TrackedObject } from 'tracked-built-ins'; + +import ErrorTrap from '@cardstack/host/components/error-trap'; + +import { renderComponent } from '../../helpers/render-component'; +import { setupRenderingTest } from '../../helpers/setup'; + +module('Integration | error-trap', function (hooks) { + setupRenderingTest(hooks); + + test('passes through when there is no error', async function (assert) { + const content = satisfies TemplateOnlyComponent<{ + Args: { params: { message: string } }; + }>; + + const params = new TrackedObject({ + message: 'hello', + }); + + await renderComponent(); + assert.dom('[data-test="message"]').containsText('hello'); + }); + + test('re-renders normally', async function (assert) { + const content = satisfies TemplateOnlyComponent<{ + Args: { params: { message: string } }; + }>; + + const params = new TrackedObject({ + message: 'hello', + }); + + await renderComponent(); + params.message = 'goodbye'; + await settled(); + assert.dom('[data-test="message"]').containsText('goodbye'); + }); + + test('traps error on initial render', async function (assert) { + class Content extends Component<{ + Args: { params: { mode: boolean } }; + }> { + get message() { + if (this.args.params.mode) { + return 'Everything OK'; + } else { + throw new Error('intentional exception'); + } + } + + + } + + const params = new TrackedObject({ + mode: false, + }); + + await renderComponent(); + assert.dom('[data-test-error-trap]').exists(); + }); + + test('traps error on re-render', async function (assert) { + class Content extends Component<{ + Args: { params: { mode: boolean } }; + }> { + get message() { + if (this.args.params.mode) { + return 'Everything OK'; + } else { + throw new Error('intentional exception'); + } + } + + + } + + const params = new TrackedObject({ + mode: true, + }); + + await renderComponent(); + assert.dom('[data-test="message"]').containsText('Everything OK'); + + params.mode = false; + await settled(); + assert.dom('[data-test-error-trap]').exists(); + }); + + test('can recover', async function (assert) { + class Content extends Component<{ + Args: { params: { mode: boolean } }; + }> { + get message() { + if (this.args.params.mode) { + return 'Everything OK'; + } else { + throw new Error('intentional exception'); + } + } + + + } + + const params = new TrackedObject({ + mode: false, + }); + + await renderComponent(); + + assert.dom('[data-test-error-trap]').exists(); + + params.mode = true; + await settled(); + assert.dom('[data-test="message"]').containsText('Everything OK'); + }); +}); From f27e211fc71d0eb930c9e0eed6201d5958143985 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 7 Nov 2024 14:29:14 -0500 Subject: [PATCH 2/2] starting re-render support --- packages/host/app/components/error-trap.gts | 36 +++++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/packages/host/app/components/error-trap.gts b/packages/host/app/components/error-trap.gts index b1966082c9..f22c96fa59 100644 --- a/packages/host/app/components/error-trap.gts +++ b/packages/host/app/components/error-trap.gts @@ -21,7 +21,7 @@ function render( owner: Owner, Content: ComponentLike<{ Args: { params: Params } }>, params: Params, -): void { +): { rerender: () => void } { // this needs to be a template-only component because the way we're invoking it // just grabs the template and would drop any associated class. const root = ; @@ -45,7 +45,16 @@ function render( let vm = (iterator as any).vm; try { - inTransaction(_runtime.env, () => vm._execute()); + let result: any; + inTransaction(_runtime.env, () => { + result = vm._execute(); + }); + return { + rerender() { + // NEXT: this needs to get wrapped with our own inTransaction just like the initial render so it doesn't interact with the default tracking frames. + result.rerender({ alwaysRevalidate: false }); + }, + }; } catch (err: any) { // This is to compensate for the commitCacheGroup op code that is not called because // of the error being thrown here. we do this so we can keep the TRANSACTION_STACK @@ -87,24 +96,31 @@ export default class ErrorTrap extends Component<{ }> { @tracked failed = false; - firstRender = true; + renderer: { rerender(): void } | undefined; - render = modifier((element) => { - if (!this.firstRender) { - return; - } - this.firstRender = false; + attach = modifier((element) => { try { - render(element, getOwner(this)!, this.args.content, this.args.params); + if (this.renderer) { + this.renderer.rerender(); + } else { + this.renderer = render( + element, + getOwner(this)!, + this.args.content, + this.args.params, + ); + } this.failed = false; } catch (err) { + debugger; removeChildren(element); this.failed = true; + this.renderer = undefined; } });