From 3ea131c103c50f2d4d1eeec0750afab7def4caa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Sat, 28 Feb 2026 09:56:21 -0300 Subject: [PATCH 01/12] First draft --- packages/@ember/-internals/glimmer/index.ts | 1 + .../glimmer/lib/component-managers/curly.ts | 1 + .../glimmer/lib/component-managers/mount.ts | 1 + .../glimmer/lib/component-managers/outlet.ts | 1 + .../glimmer/lib/component-managers/root.ts | 1 + .../lib/component-managers/route-template.ts | 1 + .../glimmer/lib/components/error-boundary.ts | 92 +++++++++++++++++++ .../glimmer/lib/components/internal.ts | 1 + .../-internals/glimmer/lib/setup-registry.ts | 2 + .../glimmer/lib/templates/error-boundary.ts | 11 +++ packages/@ember/component/index.ts | 2 +- .../lib/components/emberish-curly.ts | 1 + .../integration-tests/test/owner-test.ts | 1 + packages/@glimmer/component/src/index.ts | 2 + .../@glimmer/constants/lib/syscall-ops.ts | 4 +- .../@glimmer/debug/lib/opcode-metadata.ts | 8 ++ .../lib/managers/internal/component.d.ts | 10 +- .../@glimmer/interfaces/lib/vm-opcodes.d.ts | 6 +- .../@glimmer/manager/lib/public/component.ts | 1 + .../@glimmer/manager/lib/util/capabilities.ts | 8 +- .../manager/test/capabilities-test.ts | 10 +- .../@glimmer/manager/test/managers-test.ts | 1 + .../lib/opcode-builder/delegate.ts | 2 + .../lib/opcode-builder/helpers/components.ts | 46 +++++++--- packages/@glimmer/runtime/index.ts | 4 + .../runtime/lib/compiled/opcodes/component.ts | 77 +++++++++++++++- .../runtime/lib/component/error-boundary.ts | 38 ++++++++ .../runtime/lib/component/template-only.ts | 1 + packages/@glimmer/runtime/lib/vm/update.ts | 50 ++++++++++ packages/@glimmer/vm/lib/flags.ts | 2 + 30 files changed, 361 insertions(+), 25 deletions(-) create mode 100644 packages/@ember/-internals/glimmer/lib/components/error-boundary.ts create mode 100644 packages/@ember/-internals/glimmer/lib/templates/error-boundary.ts create mode 100644 packages/@glimmer/runtime/lib/component/error-boundary.ts diff --git a/packages/@ember/-internals/glimmer/index.ts b/packages/@ember/-internals/glimmer/index.ts index 4cf8bd70367..b8c21ea46b6 100644 --- a/packages/@ember/-internals/glimmer/index.ts +++ b/packages/@ember/-internals/glimmer/index.ts @@ -447,6 +447,7 @@ export { templateFactory as template, templateCacheCounters } from '@glimmer/opcode-compiler'; export { default as RootTemplate } from './lib/templates/root'; +export { default as ErrorBoundary } from './lib/components/error-boundary'; export { default as Input } from './lib/components/input'; export { default as LinkTo } from './lib/components/link-to'; export { default as Textarea } from './lib/components/textarea'; diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 3dff5f3bace..e27cccde10f 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -549,6 +549,7 @@ export const CURLY_CAPABILITIES: InternalComponentCapabilities = { wrapped: true, willDestroy: true, hasSubOwner: false, + errorBoundary: false, }; export const CURLY_COMPONENT_MANAGER = new CurlyComponentManager(); diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts b/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts index 1fa7a7c8e3c..d6abddf9b5e 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/mount.ts @@ -49,6 +49,7 @@ const CAPABILITIES = { wrapped: false, willDestroy: false, hasSubOwner: true, + errorBoundary: false, }; class MountManager diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts index e39b60250a3..8c23ce59df9 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/outlet.ts @@ -59,6 +59,7 @@ const CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; const CAPABILITIES_MASK = capabilityFlagsFrom(CAPABILITIES); diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/root.ts b/packages/@ember/-internals/glimmer/lib/component-managers/root.ts index 00aca8f633c..83f84ad741f 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/root.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/root.ts @@ -92,6 +92,7 @@ export const ROOT_CAPABILITIES: InternalComponentCapabilities = { wrapped: true, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; export class RootComponentDefinition implements ComponentDefinition { diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/route-template.ts b/packages/@ember/-internals/glimmer/lib/component-managers/route-template.ts index 10f291d7dfe..b2ad3739fe4 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/route-template.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/route-template.ts @@ -45,6 +45,7 @@ const CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; const CAPABILITIES_MASK = capabilityFlagsFrom(CAPABILITIES); diff --git a/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts b/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts new file mode 100644 index 00000000000..d0a79a91285 --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts @@ -0,0 +1,92 @@ +import type { + Bounds, + Destroyable, + DynamicScope, + Environment, + InternalComponentCapabilities, + InternalComponentManager, + Nullable, + Owner, + VMArguments, + WithCreateInstance, +} from '@glimmer/interfaces'; +import type { Reference } from '@glimmer/reference'; +import { setComponentTemplate, setInternalComponentManager } from '@glimmer/manager'; +import { createConstRef } from '@glimmer/reference'; +import { ErrorBoundaryState } from '@glimmer/runtime'; + +import ErrorBoundaryTemplate from '../templates/error-boundary'; + +const CAPABILITIES: InternalComponentCapabilities = { + dynamicLayout: false, + dynamicTag: false, + prepareArgs: false, + createArgs: false, + attributeHook: false, + elementHook: false, + createCaller: false, + dynamicScope: false, + updateHook: false, + createInstance: true, + wrapped: false, + willDestroy: false, + hasSubOwner: false, + errorBoundary: true, +}; + +class ErrorBoundaryManager + implements InternalComponentManager, WithCreateInstance +{ + getCapabilities(): InternalComponentCapabilities { + return CAPABILITIES; + } + + create( + _owner: Owner, + _definition: object, + _args: Nullable, + _env: Environment, + _dynamicScope: Nullable, + _caller: Nullable, + _hasDefaultBlock: boolean + ): ErrorBoundaryState { + return new ErrorBoundaryState(); + } + + didCreate(): void {} + didUpdate(): void {} + didRenderLayout(): void {} + didUpdateLayout(): void {} + + getDebugName(): string { + return 'ErrorBoundary'; + } + + getSelf(instance: ErrorBoundaryState): Reference { + return createConstRef(instance, 'this'); + } + + getDestroyable(_instance: ErrorBoundaryState): Nullable { + return null; + } + + update(_instance: ErrorBoundaryState, _dynamicScope: Nullable): void {} + + didSplatAttributes( + _instance: ErrorBoundaryState, + _element: ErrorBoundaryState, + _operations: Bounds + ): void {} +} + +const MANAGER = new ErrorBoundaryManager(); + +const ErrorBoundary = { + create() { + throw new Error('ErrorBoundary does not support .create(). It is managed internally.'); + }, +}; +setInternalComponentManager(MANAGER, ErrorBoundary); +setComponentTemplate(ErrorBoundaryTemplate, ErrorBoundary); + +export default ErrorBoundary; diff --git a/packages/@ember/-internals/glimmer/lib/components/internal.ts b/packages/@ember/-internals/glimmer/lib/components/internal.ts index 9bd6c132b6a..6ecbf263dee 100644 --- a/packages/@ember/-internals/glimmer/lib/components/internal.ts +++ b/packages/@ember/-internals/glimmer/lib/components/internal.ts @@ -169,6 +169,7 @@ const CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; class InternalManager diff --git a/packages/@ember/-internals/glimmer/lib/setup-registry.ts b/packages/@ember/-internals/glimmer/lib/setup-registry.ts index 1608fe1f185..0762242d4c9 100644 --- a/packages/@ember/-internals/glimmer/lib/setup-registry.ts +++ b/packages/@ember/-internals/glimmer/lib/setup-registry.ts @@ -2,6 +2,7 @@ import type { Registry } from '@ember/-internals/container'; import { privatize as P } from '@ember/-internals/container'; import { getOwner } from '@ember/-internals/owner'; import { assert } from '@ember/debug'; +import ErrorBoundary from './components/error-boundary'; import Input from './components/input'; import LinkTo from './components/link-to'; import Textarea from './components/textarea'; @@ -47,6 +48,7 @@ export function setupEngineRegistry(registry: Registry): void { registry.optionsForType('helper', { instantiate: false }); + registry.register('component:error-boundary', ErrorBoundary); registry.register('component:input', Input); registry.register('component:link-to', LinkTo); diff --git a/packages/@ember/-internals/glimmer/lib/templates/error-boundary.ts b/packages/@ember/-internals/glimmer/lib/templates/error-boundary.ts new file mode 100644 index 00000000000..0b7319aaedc --- /dev/null +++ b/packages/@ember/-internals/glimmer/lib/templates/error-boundary.ts @@ -0,0 +1,11 @@ +import { precompileTemplate } from '@ember/template-compilation'; +export default precompileTemplate( + `{{#if this.hasError}}{{yield this.error this.retry to="error"}}{{else}}{{yield}}{{/if}}`, + { + moduleName: 'packages/@ember/-internals/glimmer/lib/templates/error-boundary.hbs', + strictMode: true, + scope() { + return {}; + }, + } +); diff --git a/packages/@ember/component/index.ts b/packages/@ember/component/index.ts index 0834c935f80..9471ff653df 100644 --- a/packages/@ember/component/index.ts +++ b/packages/@ember/component/index.ts @@ -5,7 +5,7 @@ export { setComponentTemplate, getComponentTemplate } from '@glimmer/manager'; -export { Component as default, Input, Textarea } from '@ember/-internals/glimmer'; +export { Component as default, ErrorBoundary, Input, Textarea } from '@ember/-internals/glimmer'; export { componentCapabilities as capabilities, setComponentManager, diff --git a/packages/@glimmer-workspace/integration-tests/lib/components/emberish-curly.ts b/packages/@glimmer-workspace/integration-tests/lib/components/emberish-curly.ts index 61560bb3931..f7983a761ad 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/components/emberish-curly.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/components/emberish-curly.ts @@ -131,6 +131,7 @@ const EMBERISH_CURLY_CAPABILITIES: InternalComponentCapabilities = { wrapped: true, willDestroy: true, hasSubOwner: false, + errorBoundary: false, }; export class EmberishCurlyComponentManager diff --git a/packages/@glimmer-workspace/integration-tests/test/owner-test.ts b/packages/@glimmer-workspace/integration-tests/test/owner-test.ts index b80180164f5..f640a0e70a0 100644 --- a/packages/@glimmer-workspace/integration-tests/test/owner-test.ts +++ b/packages/@glimmer-workspace/integration-tests/test/owner-test.ts @@ -52,6 +52,7 @@ const CAPABILITIES = { wrapped: false, willDestroy: false, hasSubOwner: true, + errorBoundary: false, }; // eslint-disable-next-line @typescript-eslint/no-extraneous-class diff --git a/packages/@glimmer/component/src/index.ts b/packages/@glimmer/component/src/index.ts index 55936ebf3a2..dd7ee2aa8db 100644 --- a/packages/@glimmer/component/src/index.ts +++ b/packages/@glimmer/component/src/index.ts @@ -406,3 +406,5 @@ export default class Component extends _GlimmerComponent { setComponentManager((owner: Owner) => { return new GlimmerComponentManager(owner); }, Component); + +export { ErrorBoundary } from '@ember/component'; diff --git a/packages/@glimmer/constants/lib/syscall-ops.ts b/packages/@glimmer/constants/lib/syscall-ops.ts index 22e49360b86..5b101e7859b 100644 --- a/packages/@glimmer/constants/lib/syscall-ops.ts +++ b/packages/@glimmer/constants/lib/syscall-ops.ts @@ -52,6 +52,7 @@ import type { VmJumpIf, VmJumpUnless, VmLoad, + VmInvokeComponentLayoutGuarded, VmLog, VmMain, VmModifier, @@ -187,7 +188,8 @@ export const VM_IF_INLINE_OP = 109 satisfies VmIfInline; export const VM_NOT_OP = 110 satisfies VmNot; export const VM_GET_DYNAMIC_VAR_OP = 111 satisfies VmGetDynamicVar; export const VM_LOG_OP = 112 satisfies VmLog; -export const VM_SYSCALL_SIZE = 113 satisfies VmSize; +export const VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP = 113 satisfies VmInvokeComponentLayoutGuarded; +export const VM_SYSCALL_SIZE = 114 satisfies VmSize; export function isOp(value: number): value is VmOp { return value >= 16; diff --git a/packages/@glimmer/debug/lib/opcode-metadata.ts b/packages/@glimmer/debug/lib/opcode-metadata.ts index deeb0ff6bee..8cbb97104e7 100644 --- a/packages/@glimmer/debug/lib/opcode-metadata.ts +++ b/packages/@glimmer/debug/lib/opcode-metadata.ts @@ -47,6 +47,7 @@ import { VM_HAS_BLOCK_PARAMS_OP, VM_HELPER_OP, VM_IF_INLINE_OP, + VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, VM_INVOKE_COMPONENT_LAYOUT_OP, VM_INVOKE_STATIC_OP, VM_INVOKE_VIRTUAL_OP, @@ -705,6 +706,13 @@ if (LOCAL_DEBUG) { ops: ['state:register'], }; + METADATA[VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP] = { + name: 'InvokeComponentLayoutGuarded', + mnemonic: 'comp_invokelayout_guarded', + stackChange: 0, + ops: ['state:register'], + }; + METADATA[VM_BEGIN_COMPONENT_TRANSACTION_OP] = { name: 'BeginComponentTransaction', mnemonic: 'comp_begin', diff --git a/packages/@glimmer/interfaces/lib/managers/internal/component.d.ts b/packages/@glimmer/interfaces/lib/managers/internal/component.d.ts index f10165973ce..1852451bd5a 100644 --- a/packages/@glimmer/interfaces/lib/managers/internal/component.d.ts +++ b/packages/@glimmer/interfaces/lib/managers/internal/component.d.ts @@ -106,6 +106,12 @@ export interface InternalComponentCapabilities { * used for engines. */ hasSubOwner: boolean; + + /** + * Whether this component acts as an error boundary, catching errors thrown + * during rendering of its children and displaying fallback UI. + */ + errorBoundary: boolean; } /** @@ -126,6 +132,7 @@ export type CreateInstanceCapability = 0b0001000000000; export type WrappedCapability = 0b0010000000000; export type WillDestroyCapability = 0b0100000000000; export type HasSubOwnerCapability = 0b1000000000000; +export type ErrorBoundaryCapability = 0b10000000000000; export type InternalComponentCapability = | EmptyCapability @@ -141,7 +148,8 @@ export type InternalComponentCapability = | CreateInstanceCapability | WrappedCapability | WillDestroyCapability - | HasSubOwnerCapability; + | HasSubOwnerCapability + | ErrorBoundaryCapability; //////////// diff --git a/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts b/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts index 53058efcddb..4dd382c90f4 100644 --- a/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts +++ b/packages/@glimmer/interfaces/lib/vm-opcodes.d.ts @@ -111,7 +111,8 @@ export type VmIfInline = 109; export type VmNot = 110; export type VmGetDynamicVar = 111; export type VmLog = 112; -export type VmSize = 113; +export type VmInvokeComponentLayoutGuarded = 113; +export type VmSize = 114; export type VmOp = | VmHelper @@ -206,6 +207,7 @@ export type VmOp = | VmIfInline | VmNot | VmGetDynamicVar - | VmLog; + | VmLog + | VmInvokeComponentLayoutGuarded; export type SomeVmOp = VmOp | VmMachineOp; diff --git a/packages/@glimmer/manager/lib/public/component.ts b/packages/@glimmer/manager/lib/public/component.ts index f8f9be9b7a7..47eb8430d93 100644 --- a/packages/@glimmer/manager/lib/public/component.ts +++ b/packages/@glimmer/manager/lib/public/component.ts @@ -39,6 +39,7 @@ const CAPABILITIES = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; export function componentCapabilities( diff --git a/packages/@glimmer/manager/lib/util/capabilities.ts b/packages/@glimmer/manager/lib/util/capabilities.ts index 7254e129804..a3ca7eecbee 100644 --- a/packages/@glimmer/manager/lib/util/capabilities.ts +++ b/packages/@glimmer/manager/lib/util/capabilities.ts @@ -10,6 +10,7 @@ import type { DynamicScopeCapability, DynamicTagCapability, ElementHookCapability, + ErrorBoundaryCapability, Expand, HasSubOwnerCapability, InternalComponentCapability, @@ -62,7 +63,8 @@ export function capabilityFlagsFrom(capabilities: CapabilityOptions): Capability capability(capabilities, 'createInstance') | capability(capabilities, 'wrapped') | capability(capabilities, 'willDestroy') | - capability(capabilities, 'hasSubOwner')) as CapabilityMask; + capability(capabilities, 'hasSubOwner') | + capability(capabilities, 'errorBoundary')) as CapabilityMask; } function capability( @@ -99,7 +101,9 @@ export type InternalComponentCapabilityFor( _manager: InternalComponentManager, diff --git a/packages/@glimmer/manager/test/capabilities-test.ts b/packages/@glimmer/manager/test/capabilities-test.ts index 86aef75d329..d70fdbe849f 100644 --- a/packages/@glimmer/manager/test/capabilities-test.ts +++ b/packages/@glimmer/manager/test/capabilities-test.ts @@ -20,8 +20,9 @@ QUnit.test('encodes a capabilities object into a bitmap', (assert) => { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }), - 0b0000000000000, + 0b00000000000000, 'empty capabilities' ); @@ -40,8 +41,9 @@ QUnit.test('encodes a capabilities object into a bitmap', (assert) => { wrapped: true, willDestroy: true, hasSubOwner: true, + errorBoundary: true, }), - 0b1111111111111, + 0b11111111111111, 'all capabilities' ); @@ -60,8 +62,9 @@ QUnit.test('encodes a capabilities object into a bitmap', (assert) => { wrapped: true, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }), - 0b0010100100101, + 0b00010100100101, 'random sample' ); }); @@ -81,6 +84,7 @@ QUnit.test('allows querying bitmap for a capability', (assert) => { wrapped: true, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }); assert.true( diff --git a/packages/@glimmer/manager/test/managers-test.ts b/packages/@glimmer/manager/test/managers-test.ts index 2d28b602635..990dd12c769 100644 --- a/packages/@glimmer/manager/test/managers-test.ts +++ b/packages/@glimmer/manager/test/managers-test.ts @@ -78,6 +78,7 @@ module('Managers', () => { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; } diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/delegate.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/delegate.ts index fb83e191533..eaa24a7838c 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/delegate.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/delegate.ts @@ -18,6 +18,7 @@ export const DEFAULT_CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; export const MINIMAL_CAPABILITIES: InternalComponentCapabilities = { @@ -34,6 +35,7 @@ export const MINIMAL_CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; export interface ResolverDelegate { diff --git a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts index 3339e3540ff..015b28ab7db 100644 --- a/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts +++ b/packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/components.ts @@ -23,6 +23,7 @@ import { VM_GET_COMPONENT_LAYOUT_OP, VM_GET_COMPONENT_SELF_OP, VM_GET_COMPONENT_TAG_NAME_OP, + VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, VM_INVOKE_COMPONENT_LAYOUT_OP, VM_INVOKE_VIRTUAL_OP, VM_JUMP_UNLESS_OP, @@ -194,7 +195,9 @@ function InvokeStaticComponent( ): void { let { symbolTable } = layout; - let bailOut = hasCapability(capabilities, InternalComponentCapabilities.prepareArgs); + let bailOut = + hasCapability(capabilities, InternalComponentCapabilities.prepareArgs) || + hasCapability(capabilities, InternalComponentCapabilities.errorBoundary); if (bailOut) { InvokeNonStaticComponent(op, { @@ -392,17 +395,24 @@ export function InvokeNonStaticComponent( CompileArgs(op, positional, named, blocks, atNames); op(VM_PREPARE_ARGS_OP, $s0); - invokePreparedComponent(op, blocks.has('default'), bindableBlocks, bindableAtNames, () => { - if (layout) { - op(VM_PUSH_SYMBOL_TABLE_OP, symbolTableOperand(layout.symbolTable)); - op(VM_CONSTANT_OP, layoutOperand(layout)); - op(VM_COMPILE_BLOCK_OP); - } else { - op(VM_GET_COMPONENT_LAYOUT_OP, $s0); - } + invokePreparedComponent( + op, + blocks.has('default'), + bindableBlocks, + bindableAtNames, + () => { + if (layout) { + op(VM_PUSH_SYMBOL_TABLE_OP, symbolTableOperand(layout.symbolTable)); + op(VM_CONSTANT_OP, layoutOperand(layout)); + op(VM_COMPILE_BLOCK_OP); + } else { + op(VM_GET_COMPONENT_LAYOUT_OP, $s0); + } - op(VM_POPULATE_LAYOUT_OP, $s0); - }); + op(VM_POPULATE_LAYOUT_OP, $s0); + }, + capabilities + ); op(VM_LOAD_OP, $s0); } @@ -440,7 +450,8 @@ export function invokePreparedComponent( hasBlock: boolean, bindableBlocks: boolean, bindableAtNames: boolean, - populateLayout: Nullable<() => void> = null + populateLayout: Nullable<() => void> = null, + capabilities: CapabilityMask | true = true ): void { op(VM_BEGIN_COMPONENT_TRANSACTION_OP, $s0); op(VM_PUSH_DYNAMIC_SCOPE_OP); @@ -466,7 +477,16 @@ export function invokePreparedComponent( if (bindableBlocks) op(VM_SET_BLOCKS_OP, $s0); op(VM_POP_OP, 1); - op(VM_INVOKE_COMPONENT_LAYOUT_OP, $s0); + + if ( + capabilities !== true && + hasCapability(capabilities, InternalComponentCapabilities.errorBoundary) + ) { + op(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, $s0); + } else { + op(VM_INVOKE_COMPONENT_LAYOUT_OP, $s0); + } + op(VM_DID_RENDER_LAYOUT_OP, $s0); op(VM_POP_FRAME_OP); diff --git a/packages/@glimmer/runtime/index.ts b/packages/@glimmer/runtime/index.ts index 9ec4eb2b603..bdcf8914b2f 100644 --- a/packages/@glimmer/runtime/index.ts +++ b/packages/@glimmer/runtime/index.ts @@ -17,6 +17,10 @@ export { templateOnlyComponent, TemplateOnlyComponentManager, } from './lib/component/template-only'; +export { + ErrorBoundaryState, + type ErrorBoundaryStateInterface, +} from './lib/component/error-boundary'; export { CurriedValue, curry } from './lib/curried-value'; export { DOMChanges, diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index a8091130ec7..e6aa2f5e4c9 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -38,6 +38,7 @@ import { VM_GET_COMPONENT_LAYOUT_OP, VM_GET_COMPONENT_SELF_OP, VM_GET_COMPONENT_TAG_NAME_OP, + VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, VM_INVOKE_COMPONENT_LAYOUT_OP, VM_MAIN_OP, VM_POPULATE_LAYOUT_OP, @@ -68,7 +69,7 @@ import { CheckSyscallRegister, } from '@glimmer/debug'; import { debugToString, expect, localAssert, unwrap, unwrapTemplate } from '@glimmer/debug-util'; -import { registerDestructor } from '@glimmer/destroyable'; +import { associateDestroyableChild, destroyChildren, registerDestructor } from '@glimmer/destroyable'; import { managerHasCapability } from '@glimmer/manager'; import { isConstRef, valueForRef } from '@glimmer/reference'; import { assign, dict, EMPTY_STRING_ARRAY, enumerate } from '@glimmer/util'; @@ -78,8 +79,11 @@ import type { CurriedValue } from '../../curried-value'; import type { UpdatingVM } from '../../vm'; import type { VM } from '../../vm/append'; import type { BlockArgumentsImpl } from '../../vm/arguments'; +import { NewTreeBuilder } from '../../vm/element-builder'; +import { ErrorBoundaryOpcode } from '../../vm/update'; -import { ConcreteBounds } from '../../bounds'; +import { clear, ConcreteBounds } from '../../bounds'; +import type { ErrorBoundaryStateInterface } from '../../component/error-boundary'; import { hasCustomDebugRenderTreeLifecycle } from '../../component/interfaces'; import { resolveComponent } from '../../component/resolve'; import { isCurriedType, isCurriedValue, resolveCurriedValue } from '../../curried-value'; @@ -875,6 +879,75 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_OP, (vm, { op1: register }) => { vm.call(state.handle); }); +// Error Boundary Guarded Invocation +// Executes the component layout in a sub-VM wrapped in try-catch. +// On error during initial render, cleans up partial DOM and re-renders with error state. +APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register }) => { + let state = check(vm.fetchValue(check(register, CheckRegister)), CheckFinishedComponentInstance); + let errorState = state.state as ErrorBoundaryStateInterface; + + // Capture current scope (which has self, named args, blocks all set up) + // Use the layout handle's resolved address as the closure PC so the sub-VM + // starts executing from the layout code directly. + let layoutAddr = vm.context.program.heap.getaddr(state.handle); + let closure = vm.capture(0, layoutAddr); + let block = vm.tree().pushResettableBlock(); + + try { + let subTree = NewTreeBuilder.resume(vm.env, block); + let subVM = closure.evaluate(subTree); + + let children: UpdatingOpcode[] = []; + let errorBoundaryOp = new ErrorBoundaryOpcode( + closure, + vm.context, + block, + children, + errorState + ); + + let result = subVM.execute((subVM) => { + subVM.updateWith(errorBoundaryOp); + subVM.pushUpdating(children); + }); + + associateDestroyableChild(errorBoundaryOp, result.drop); + vm.associateDestroyable(errorBoundaryOp); + vm.updateWith(errorBoundaryOp); + vm.pushUpdating(children); + } catch (error) { + // Clean up any partial DOM from the failed render + destroyChildren(block); + clear(block); + + // Set error state so the template takes the error branch + errorState.setError(error); + + // Re-execute layout with error state (will render the error block) + let retryTree = NewTreeBuilder.resume(vm.env, block); + let retryVM = closure.evaluate(retryTree); + + let children: UpdatingOpcode[] = []; + let errorBoundaryOp = new ErrorBoundaryOpcode( + closure, + vm.context, + block, + children, + errorState + ); + + let result = retryVM.execute((retryVM) => { + retryVM.updateWith(errorBoundaryOp); + retryVM.pushUpdating(children); + }); + + associateDestroyableChild(errorBoundaryOp, result.drop); + vm.associateDestroyable(errorBoundaryOp); + vm.updateWith(errorBoundaryOp); + vm.pushUpdating(children); + } +}); + APPEND_OPCODES.add(VM_DID_RENDER_LAYOUT_OP, (vm, { op1: register }) => { let instance = check(vm.fetchValue(check(register, CheckRegister)), CheckComponentInstance); let { manager, state, capabilities } = instance; diff --git a/packages/@glimmer/runtime/lib/component/error-boundary.ts b/packages/@glimmer/runtime/lib/component/error-boundary.ts new file mode 100644 index 00000000000..9bef11ae898 --- /dev/null +++ b/packages/@glimmer/runtime/lib/component/error-boundary.ts @@ -0,0 +1,38 @@ +import { dirtyTagFor, tagFor } from '@glimmer/validator'; +import { consumeTag } from '@glimmer/validator'; + +export interface ErrorBoundaryStateInterface { + error: unknown; + hasError: boolean; + setError(error: unknown): void; + retry(): void; +} + +export class ErrorBoundaryState implements ErrorBoundaryStateInterface { + private _error: unknown = null; + private _hasError = false; + + get error(): unknown { + consumeTag(tagFor(this, '_error')); + return this._error; + } + + get hasError(): boolean { + consumeTag(tagFor(this, '_hasError')); + return this._hasError; + } + + setError(error: unknown) { + this._error = error; + dirtyTagFor(this, '_error'); + this._hasError = true; + dirtyTagFor(this, '_hasError'); + } + + retry = () => { + this._error = null; + dirtyTagFor(this, '_error'); + this._hasError = false; + dirtyTagFor(this, '_hasError'); + }; +} diff --git a/packages/@glimmer/runtime/lib/component/template-only.ts b/packages/@glimmer/runtime/lib/component/template-only.ts index ba3ee9720f3..a1f859142b8 100644 --- a/packages/@glimmer/runtime/lib/component/template-only.ts +++ b/packages/@glimmer/runtime/lib/component/template-only.ts @@ -17,6 +17,7 @@ const CAPABILITIES: InternalComponentCapabilities = { wrapped: false, willDestroy: false, hasSubOwner: false, + errorBoundary: false, }; export class TemplateOnlyComponentManager implements InternalComponentManager { diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 06386416da2..92ec2034f36 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -22,6 +22,7 @@ import { updateRef, valueForRef } from '@glimmer/reference'; import { logStep, Stack } from '@glimmer/util'; import { debug, resetTracking } from '@glimmer/validator'; +import type { ErrorBoundaryStateInterface } from '../component/error-boundary'; import type { Closure } from './append'; import type { AppendingBlockList } from './element-builder'; @@ -171,6 +172,55 @@ export class TryOpcode extends BlockOpcode implements ExceptionHandler { } } +export class ErrorBoundaryOpcode extends TryOpcode { + public type = 'error-boundary'; + + constructor( + state: Closure, + context: EvaluationContext, + bounds: ResettableBlock, + children: UpdatingOpcode[], + private errorState: ErrorBoundaryStateInterface + ) { + super(state, context, bounds, children); + } + + override evaluate(vm: UpdatingVM) { + vm.try(this.children, this); + } + + override handleException() { + try { + super.handleException(); + } catch (error) { + this.transitionToError(error); + } + } + + private transitionToError(error: unknown) { + let { + bounds, + context: { env }, + } = this; + + destroyChildren(this); + + this.errorState.setError(error); + + let tree = NewTreeBuilder.resume(env, bounds); + let vm = this.state.evaluate(tree); + + let children = (this.children = []); + + let result = vm.execute((vm) => { + vm.updateWith(this); + vm.pushUpdating(children); + }); + + associateDestroyableChild(this, result.drop); + } +} + export class ListItemOpcode extends TryOpcode { public retained = false; public index = -1; diff --git a/packages/@glimmer/vm/lib/flags.ts b/packages/@glimmer/vm/lib/flags.ts index 7ca9922c12f..9f9831ea29c 100644 --- a/packages/@glimmer/vm/lib/flags.ts +++ b/packages/@glimmer/vm/lib/flags.ts @@ -9,6 +9,7 @@ import type { DynamicTagCapability, ElementHookCapability, EmptyCapability, + ErrorBoundaryCapability, HasSubOwnerCapability, MACHINE_MASK as IMACHINE_MASK, MAX_SIZE as IMAX_SIZE, @@ -36,6 +37,7 @@ export const InternalComponentCapabilities = { wrapped: 0b0010000000000 satisfies WrappedCapability, willDestroy: 0b0100000000000 satisfies WillDestroyCapability, hasSubOwner: 0b1000000000000 satisfies HasSubOwnerCapability, + errorBoundary: 0b10000000000000 satisfies ErrorBoundaryCapability, } as const; export const ARG_SHIFT = 8 as const satisfies IARG_SHIFT; From 0469726aa2882b537fa6da63c6b29c83bee4c9a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Sat, 28 Feb 2026 21:17:40 -0300 Subject: [PATCH 02/12] [BUGFIX] Fix ErrorBoundary runtime: update-phase error handling and integration tests Fixes the updating opcode integration so ErrorBoundaryOpcode is properly placed in the parent VM's updating list (not pushed onto the stack where it gets lost). Adds JS exception handling in UpdatingVM._execute with tracking frame rollback, executeGuarded() for sub-VM isolation, debug render tree rollback, and backflow assertion bypass in setError(). Includes 7 integration tests covering initial render, rerender, retry, nested boundaries, and missing error block. --- .../components/error-boundary-test.ts | 186 ++++++++++++++++++ .../interfaces/lib/dom/attributes.d.ts | 2 + .../lib/runtime/debug-render-tree.d.ts | 6 + .../runtime/lib/compiled/opcodes/component.ts | 46 ++++- .../runtime/lib/component/error-boundary.ts | 11 +- .../@glimmer/runtime/lib/debug-render-tree.ts | 10 + packages/@glimmer/runtime/lib/vm/append.ts | 19 ++ .../runtime/lib/vm/element-builder.ts | 27 ++- packages/@glimmer/runtime/lib/vm/update.ts | 59 +++++- packages/@glimmer/validator/index.ts | 2 + packages/@glimmer/validator/lib/tracking.ts | 26 +++ 11 files changed, 377 insertions(+), 17 deletions(-) create mode 100644 packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts new file mode 100644 index 00000000000..3ee30c5fe96 --- /dev/null +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -0,0 +1,186 @@ +import { + AbstractStrictTestCase, + assertHTML, + buildOwner, + clickElement, + defComponent, + moduleFor, + runDestroy, +} from 'internal-test-helpers'; + +import { ErrorBoundary } from '@ember/component'; +import { on } from '@glimmer/runtime'; +import { tracked } from '@glimmer/tracking'; +import GlimmerishComponent from '../../utils/glimmerish-component'; + +import { run } from '@ember/runloop'; +import { associateDestroyableChild, registerDestructor } from '@glimmer/destroyable'; +import { renderComponent, type RenderResult } from '../../../lib/renderer'; +import type Owner from '@ember/owner'; + +// --- Test helper components --- + +const Throwing = defComponent('{{this.boom}}', { + component: class extends GlimmerishComponent { + get boom(): never { + throw new Error('render error'); + } + }, +}); + +const MaybeThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('conditional error'); + } + return 'ok'; + } + }, +}); + +// --- Test case base class --- + +class ErrorBoundaryTestCase extends AbstractStrictTestCase { + declare component: (RenderResult & { rerender: () => void }) | undefined; + owner: Owner; + + constructor(assert: QUnit['assert']) { + super(assert); + this.owner = buildOwner({}); + associateDestroyableChild(this, this.owner); + } + + get element() { + return document.querySelector('#qunit-fixture')!; + } + + assertChange({ change, expect }: { change: () => void; expect: string }) { + run(() => change()); + assertHTML(expect); + this.assertStableRerender(); + } + + renderComponent( + component: object, + options: { args?: Record; expect: string } + ) { + let { owner } = this; + + run(() => { + const result = renderComponent(component, { + owner, + args: options.args ?? {}, + env: { document: document, isInteractive: true, hasDOM: true }, + into: this.element, + }); + this.component = { + ...result, + rerender() { + // unused, but asserted against + }, + }; + registerDestructor(this, () => result.destroy()); + }); + + assertHTML(options.expect); + this.assertStableRerender(); + } +} + +// --- Tests --- + +moduleFor( + 'ErrorBoundary', + class extends ErrorBoundaryTestCase { + afterEach() { + if (this.component) { + runDestroy(this); + } + } + + '@test renders default block when no error'() { + let Root = defComponent( + '<:default>hello<:error as |err|>caught', + { scope: { ErrorBoundary } } + ); + + this.renderComponent(Root, { expect: 'hello' }); + } + + '@test catches error during initial render and shows error block'() { + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'caught' }); + } + + '@test passes error object to error block'() { + let Root = defComponent( + '<:default><:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'caught: render error' }); + } + + '@test catches error during rerender'() { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, MaybeThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + } + + '@test retry re-renders default content after error is fixed'() { + class State { + @tracked shouldThrow = true; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err retry|>', + { scope: { ErrorBoundary, MaybeThrow, state, on } } + ); + + this.renderComponent(Root, { expect: '' }); + + state.shouldThrow = false; + + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + } + + '@test nested boundaries — inner catches, outer unaffected'() { + let Root = defComponent( + '<:default>outer ok <:default><:error as |err|>inner caught<:error as |err|>outer caught', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'outer ok inner caught' }); + } + + '@test renders nothing when no error block provided'() { + let Root = defComponent('', { + scope: { ErrorBoundary, Throwing }, + }); + + this.renderComponent(Root, { expect: '' }); + } + } +); diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts index 47919a1e7ac..e04327d0f6a 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -45,6 +45,8 @@ export interface FixedBlock extends AppendingBlock {} */ export interface ResettableBlock extends FixedBlock { reset(env: Environment): Nullable; + /** Reset internal tracking state without DOM cleanup (for error boundary recovery). */ + resetPartial(): void; } export interface DOMStack { diff --git a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts index e3a7aafd754..940494b4573 100644 --- a/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/debug-render-tree.d.ts @@ -48,4 +48,10 @@ export interface DebugRenderTree { commit(): void; capture(): CapturedRenderNode[]; + + /** Return the current depth of the internal stack (for error boundary rollback). */ + getDepth(): number; + + /** Pop entries from the internal stack down to the given depth (for error boundary rollback). */ + rollbackTo(depth: number): void; } diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index e6aa2f5e4c9..0bddcee7172 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -79,6 +79,8 @@ import type { CurriedValue } from '../../curried-value'; import type { UpdatingVM } from '../../vm'; import type { VM } from '../../vm/append'; import type { BlockArgumentsImpl } from '../../vm/arguments'; +import { getTrackingDepth, restoreTrackingTo } from '@glimmer/validator'; + import { NewTreeBuilder } from '../../vm/element-builder'; import { ErrorBoundaryOpcode } from '../../vm/update'; @@ -893,8 +895,19 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } let closure = vm.capture(0, layoutAddr); let block = vm.tree().pushResettableBlock(); + // Record insertion point so we can clean up partial DOM on error. + let parent = block.parentElement(); + let insertionMarker = parent.lastChild; + + // Save debug render tree depth so we can roll back stale entries on error. + let renderTreeDepth = vm.env.debugRenderTree?.getDepth() ?? 0; + + // Save tracking frame depth so we can discard stale frames from a failed sub-VM. + let trackingDepth = getTrackingDepth(); + try { - let subTree = NewTreeBuilder.resume(vm.env, block); + // Use beginBlock (not resume) since this is a fresh block with no prior content. + let subTree = NewTreeBuilder.beginBlock(vm.env, block); let subVM = closure.evaluate(subTree); let children: UpdatingOpcode[] = []; @@ -906,7 +919,8 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } errorState ); - let result = subVM.execute((subVM) => { + // Use executeGuarded to avoid resetting the parent VM's tracking state. + let result = subVM.executeGuarded((subVM) => { subVM.updateWith(errorBoundaryOp); subVM.pushUpdating(children); }); @@ -914,17 +928,30 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } associateDestroyableChild(errorBoundaryOp, result.drop); vm.associateDestroyable(errorBoundaryOp); vm.updateWith(errorBoundaryOp); - vm.pushUpdating(children); } catch (error) { - // Clean up any partial DOM from the failed render - destroyChildren(block); - clear(block); + // Roll back stale tracking frames left by the failed sub-VM render. + restoreTrackingTo(trackingDepth); + + // Roll back stale debug render tree entries from the failed render. + vm.env.debugRenderTree?.rollbackTo(renderTreeDepth); + + // Remove any partial DOM nodes inserted during the failed render. + // We can't use clear(block) because child bounds may be partially initialized. + let cursor = insertionMarker ? insertionMarker.nextSibling : parent.firstChild; + while (cursor) { + let next = cursor.nextSibling; + parent.removeChild(cursor); + cursor = next; + } + + // Reset block to empty state for the retry render. + block.resetPartial(); // Set error state so the template takes the error branch errorState.setError(error); - // Re-execute layout with error state (will render the error block) - let retryTree = NewTreeBuilder.resume(vm.env, block); + // Re-execute layout with error state (will render the error block). + let retryTree = NewTreeBuilder.beginBlock(vm.env, block); let retryVM = closure.evaluate(retryTree); let children: UpdatingOpcode[] = []; @@ -936,7 +963,7 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } errorState ); - let result = retryVM.execute((retryVM) => { + let result = retryVM.executeGuarded((retryVM) => { retryVM.updateWith(errorBoundaryOp); retryVM.pushUpdating(children); }); @@ -944,7 +971,6 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } associateDestroyableChild(errorBoundaryOp, result.drop); vm.associateDestroyable(errorBoundaryOp); vm.updateWith(errorBoundaryOp); - vm.pushUpdating(children); } }); diff --git a/packages/@glimmer/runtime/lib/component/error-boundary.ts b/packages/@glimmer/runtime/lib/component/error-boundary.ts index 9bef11ae898..b22164a0857 100644 --- a/packages/@glimmer/runtime/lib/component/error-boundary.ts +++ b/packages/@glimmer/runtime/lib/component/error-boundary.ts @@ -1,5 +1,4 @@ -import { dirtyTagFor, tagFor } from '@glimmer/validator'; -import { consumeTag } from '@glimmer/validator'; +import { consumeTag, dirtyTag, dirtyTagFor, tagFor } from '@glimmer/validator'; export interface ErrorBoundaryStateInterface { error: unknown; @@ -24,9 +23,13 @@ export class ErrorBoundaryState implements ErrorBoundaryStateInterface { setError(error: unknown) { this._error = error; - dirtyTagFor(this, '_error'); + // Use dirtyTag with disableConsumptionAssertion=true because setError + // is called from an error boundary catch handler, where the tag was + // already consumed during the (failed) render. This backflow is + // intentional for error boundary recovery. + dirtyTag(tagFor(this, '_error'), true); this._hasError = true; - dirtyTagFor(this, '_hasError'); + dirtyTag(tagFor(this, '_hasError'), true); } retry = () => { diff --git a/packages/@glimmer/runtime/lib/debug-render-tree.ts b/packages/@glimmer/runtime/lib/debug-render-tree.ts index b48626fb8a3..3581d465e99 100644 --- a/packages/@glimmer/runtime/lib/debug-render-tree.ts +++ b/packages/@glimmer/runtime/lib/debug-render-tree.ts @@ -105,6 +105,16 @@ export default class DebugRenderTreeImpl< return this.captureRefs(this.roots); } + getDepth(): number { + return this.stack.size; + } + + rollbackTo(depth: number): void { + while (this.stack.size > depth) { + this.stack.pop(); + } + } + private reset(): void { if (this.stack.size !== 0) { // We probably encountered an error during the rendering loop. This will diff --git a/packages/@glimmer/runtime/lib/vm/append.ts b/packages/@glimmer/runtime/lib/vm/append.ts index 5d43072ccf5..dabf7606bf5 100644 --- a/packages/@glimmer/runtime/lib/vm/append.ts +++ b/packages/@glimmer/runtime/lib/vm/append.ts @@ -723,6 +723,25 @@ export class VM { /// EXECUTION + /** + * Execute the VM for an error boundary sub-VM. Unlike `execute`, this does + * NOT wrap in a tracking transaction or call `resetTracking()` on error, + * which would destroy the parent VM's tracking state. It only cleans up + * open blocks on error before re-throwing. + */ + executeGuarded(initialize?: (vm: this) => void): RenderResult { + try { + return this._execute(initialize); + } catch (e) { + // Clean up block stack without resetting tracking (preserve parent's state). + let elements = this.tree(); + while (elements.hasBlocks) { + elements.popBlock(); + } + throw e; + } + } + execute(initialize?: (vm: this) => void): RenderResult { if (DEBUG) { let hasErrored = true; diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index 160944fefa4..c0ebcd6abbf 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -105,6 +105,19 @@ export class NewTreeBuilder implements TreeBuilder { return stack; } + /** + * Creates a tree builder that renders into a fresh (empty) resettable block. + * Unlike `resume`, this does NOT call `block.reset()`, so it's safe for + * blocks that have never been rendered or have partially-initialized children. + */ + static beginBlock(env: Environment, block: ResettableBlock): NewTreeBuilder { + let parentNode = block.parentElement(); + let stack = new this(env, parentNode, null).initialize(); + stack.pushBlock(block); + + return stack; + } + constructor(env: Environment, parentNode: SimpleElement, nextSibling: Nullable) { this.pushElement(parentNode, nextSibling); this.env = env; @@ -515,7 +528,7 @@ export class ResettableBlockImpl extends AppendingBlockImpl implements Resettabl reset(): Nullable { destroy(this); - let nextSibling = clear(this); + let nextSibling = this.first ? clear(this) : null; this.first = null; this.last = null; @@ -523,6 +536,18 @@ export class ResettableBlockImpl extends AppendingBlockImpl implements Resettabl return nextSibling; } + + /** + * Reset the block's internal tracking state without touching the DOM. + * Used during error boundary recovery when the DOM has already been + * cleaned up manually (because child bounds may be partially initialized). + */ + resetPartial(): void { + destroy(this); + this.first = null; + this.last = null; + this.nesting = 0; + } } // FIXME: All the noops in here indicate a modelling problem diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 92ec2034f36..aa79b325563 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -20,7 +20,7 @@ import { associateDestroyableChild, destroy, destroyChildren } from '@glimmer/de import { LOCAL_DEBUG } from '@glimmer/local-debug-flags'; import { updateRef, valueForRef } from '@glimmer/reference'; import { logStep, Stack } from '@glimmer/util'; -import { debug, resetTracking } from '@glimmer/validator'; +import { debug, getTrackingDepth, resetTracking, restoreTrackingTo } from '@glimmer/validator'; import type { ErrorBoundaryStateInterface } from '../component/error-boundary'; import type { Closure } from './append'; @@ -79,7 +79,36 @@ export class UpdatingVM implements IUpdatingVM { continue; } - opcode.evaluate(this); + // Save tracking frame depth so we can discard stale frames if the + // opcode throws a JS exception (e.g., a tracked getter throwing). + let trackingDepth = getTrackingDepth(); + + try { + opcode.evaluate(this); + } catch (error) { + // Restore tracking frames to the depth before the failed opcode. + // Without this, stale frames corrupt the parent's tracking context. + restoreTrackingTo(trackingDepth); + + // Walk up the frame stack to find an error boundary that can handle + // JavaScript errors. We skip regular TryOpcode handlers because they + // would reset their block and re-render, which corrupts parent block + // references if the re-render also fails. + let handled = false; + + while (!frameStack.isEmpty()) { + if (this.frame.handleError(error)) { + frameStack.pop(); + handled = true; + break; + } + frameStack.pop(); + } + + if (!handled) { + throw error; + } + } } } @@ -197,6 +226,15 @@ export class ErrorBoundaryOpcode extends TryOpcode { } } + /** + * Handle a JavaScript error that escaped during updating evaluation. + * Called directly by the UpdatingVM when a JS exception occurs, + * skipping inner TryOpcode handlers that would corrupt block state. + */ + handleError(error: unknown) { + this.transitionToError(error); + } + private transitionToError(error: unknown) { let { bounds, @@ -491,4 +529,21 @@ class UpdatingVMFrame { this.exceptionHandler.handleException(); } } + + /** + * Try to handle a JavaScript error (not a tracked-value change). + * Returns true if the handler accepted the error, false otherwise. + * Only error boundary handlers accept JavaScript errors. + */ + handleError(error: unknown): boolean { + if ( + this.exceptionHandler && + 'handleError' in this.exceptionHandler && + typeof (this.exceptionHandler as any).handleError === 'function' + ) { + (this.exceptionHandler as any).handleError(error); + return true; + } + return false; + } } diff --git a/packages/@glimmer/validator/index.ts b/packages/@glimmer/validator/index.ts index a3abcaa5007..6412ede81b7 100644 --- a/packages/@glimmer/validator/index.ts +++ b/packages/@glimmer/validator/index.ts @@ -25,10 +25,12 @@ export { createCache, endTrackFrame, endUntrackFrame, + getTrackingDepth, getValue, isConst, isTracking, resetTracking, + restoreTrackingTo, track, untrack, } from './lib/tracking'; diff --git a/packages/@glimmer/validator/lib/tracking.ts b/packages/@glimmer/validator/lib/tracking.ts index d94ae456e6b..2527b4afd63 100644 --- a/packages/@glimmer/validator/lib/tracking.ts +++ b/packages/@glimmer/validator/lib/tracking.ts @@ -95,6 +95,32 @@ export function endUntrackFrame(): void { CURRENT_TRACKER = OPEN_TRACK_FRAMES.pop() || null; } +/** + * Return the current depth of the tracking frame stack. Used together with + * `restoreTrackingTo` for error boundary rollback: save the depth before a + * sub-VM runs and restore it if the sub-VM throws, discarding any stale + * tracking frames from the failed render. + */ +export function getTrackingDepth(): number { + return OPEN_TRACK_FRAMES.length; +} + +/** + * Pop tracking frames (and their associated DEBUG tracking transactions) back + * to the given depth. This discards stale frames left by a failed render. + */ +export function restoreTrackingTo(depth: number): void { + while (OPEN_TRACK_FRAMES.length > depth) { + OPEN_TRACK_FRAMES.pop(); + if (DEBUG) { + unwrap(debug.endTrackingTransaction)(); + } + } + CURRENT_TRACKER = OPEN_TRACK_FRAMES.length > 0 + ? OPEN_TRACK_FRAMES[OPEN_TRACK_FRAMES.length - 1] + : null; +} + // This function is only for handling errors and resetting to a valid state export function resetTracking(): string | void { while (OPEN_TRACK_FRAMES.length > 0) { From 96b072d4a3dfc6fa372cd698ba8e28e5d225a9e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Tue, 3 Mar 2026 23:03:03 -0300 Subject: [PATCH 03/12] [BUGFIX] Fix ErrorBoundary rerender recovery and expand test coverage Cache DOM boundaries before child opcodes execute so error recovery can clean up correctly even when inner TryOpcodes corrupt the bounds tree. Add beginBlock nextSibling parameter for proper insertion positioning. Add comprehensive integration tests for error boundary edge cases. --- package.json | 1 - .../components/error-boundary-test.ts | 374 +++++++++++++++++- .../runtime/lib/vm/element-builder.ts | 8 +- packages/@glimmer/runtime/lib/vm/update.ts | 43 +- 4 files changed, 419 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index e7f45dbf34f..79a3027e353 100644 --- a/package.json +++ b/package.json @@ -346,7 +346,6 @@ "@simple-dom/document/index.js": "ember-source/@simple-dom/document/index.js", "backburner.js/index.js": "ember-source/backburner.js/index.js", "dag-map/index.js": "ember-source/dag-map/index.js", - "ember-template-compiler/index.js": "ember-source/ember-template-compiler/index.js", "ember-testing/index.js": "ember-source/ember-testing/index.js", "ember-testing/lib/adapters/adapter.js": "ember-source/ember-testing/lib/adapters/adapter.js", "ember-testing/lib/adapters/qunit.js": "ember-source/ember-testing/lib/adapters/qunit.js", diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index 3ee30c5fe96..2aae4b99eea 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -4,19 +4,23 @@ import { buildOwner, clickElement, defComponent, + defineSimpleHelper, + defineSimpleModifier, moduleFor, runDestroy, } from 'internal-test-helpers'; -import { ErrorBoundary } from '@ember/component'; -import { on } from '@glimmer/runtime'; +import { ErrorBoundary, setComponentManager } from '@ember/component'; +import { array, on } from '@glimmer/runtime'; import { tracked } from '@glimmer/tracking'; import GlimmerishComponent from '../../utils/glimmerish-component'; import { run } from '@ember/runloop'; -import { associateDestroyableChild, registerDestructor } from '@glimmer/destroyable'; +import { associateDestroyableChild, destroy, registerDestructor } from '@glimmer/destroyable'; +import { componentCapabilities } from '@glimmer/manager'; import { renderComponent, type RenderResult } from '../../../lib/renderer'; import type Owner from '@ember/owner'; +import { setOwner } from '@ember/-internals/owner'; // --- Test helper components --- @@ -182,5 +186,369 @@ moduleFor( this.renderComponent(Root, { expect: '' }); } + + '@test tracked properties work after error recovery via retry'() { + class State { + @tracked shouldThrow = true; + } + let state = new State(); + + let Counter = defComponent( + '{{this.count}}', + { + component: class extends GlimmerishComponent { + @tracked count = 0; + increment = () => this.count++; + }, + scope: { on }, + } + ); + + let Root = defComponent( + '<:default><:error as |err retry|>', + { scope: { ErrorBoundary, MaybeThrow, Counter, state, on } } + ); + + this.renderComponent(Root, { expect: '' }); + + state.shouldThrow = false; + + this.assertChange({ + change: () => clickElement('.retry'), + expect: 'ok0', + }); + + this.assertChange({ + change: () => clickElement('button:not(.retry)'), + expect: 'ok1', + }); + } + + '@test sibling content survives error and recovery round-trip'() { + let Root = defComponent( + 'before<:default><:error as |err|>caughtafter', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'beforecaughtafter' }); + } + + '@test catches error from deeply nested grandchild component'() { + let Child = defComponent('', { scope: { Throwing } }); + let Parent = defComponent('', { scope: { Child } }); + + let Root = defComponent( + '<:default><:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, Parent } } + ); + + this.renderComponent(Root, { expect: 'caught: render error' }); + } + + '@test catches error from item in each loop'() { + let ItemComponent = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.item === 'bad') { + throw new Error('bad item'); + } + return (this as any).args.item; + } + }, + }); + + let Root = defComponent( + '<:default>{{#each (array "good" "bad" "also-good") as |item|}}{{/each}}<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, ItemComponent, array } } + ); + + this.renderComponent(Root, { expect: 'caught: bad item' }); + } + + '@test helper throws during rerender when tracked state changes'() { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let maybeThrowHelper = defineSimpleHelper((shouldThrow: boolean) => { + if (shouldThrow) throw new Error('helper rerender error'); + return 'helper ok'; + }); + + let Root = defComponent( + '<:default>{{maybeThrowHelper state.shouldThrow}}<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, maybeThrowHelper, state } } + ); + + this.renderComponent(Root, { expect: 'helper ok' }); + + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught: helper rerender error', + }); + } + + '@test catches error thrown by a helper'() { + let throwingHelper = defineSimpleHelper(() => { + throw new Error('helper error'); + }); + + let Root = defComponent( + '<:default>{{throwingHelper}}<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, throwingHelper } } + ); + + this.renderComponent(Root, { expect: 'caught: helper error' }); + } + + '@test modifiers install correctly inside error boundary'(assert: Assert) { + let trackingModifier = defineSimpleModifier((element: Element) => { + assert.step('modifier installed'); + element.setAttribute('data-modified', 'true'); + }); + + let Root = defComponent( + '<:default>
content
<:error as |err|>caught', + { scope: { ErrorBoundary, trackingModifier } } + ); + + this.renderComponent(Root, { expect: '
content
' }); + assert.verifySteps(['modifier installed']); + } + + '@test computed properties and tracked dependencies work inside boundary'() { + class State { + @tracked firstName = 'Ada'; + @tracked lastName = 'Lovelace'; + } + let state = new State(); + + let FullName = defComponent('{{this.fullName}}', { + component: class extends GlimmerishComponent { + get fullName() { + return `${(this as any).args.first} ${(this as any).args.last}`; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, FullName, state } } + ); + + this.renderComponent(Root, { expect: 'Ada Lovelace' }); + + this.assertChange({ + change: () => (state.firstName = 'Grace'), + expect: 'Grace Lovelace', + }); + + this.assertChange({ + change: () => (state.lastName = 'Hopper'), + expect: 'Grace Hopper', + }); + } + + '@test multiple sibling boundaries — one errors, other stays intact'() { + let Root = defComponent( + '<:default><:error as |err|>first caught<:default>second ok<:error as |err|>second caught', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'first caughtsecond ok' }); + } + + '@test error boundary preserves error block across unrelated rerenders'() { + class State { + @tracked counter = 0; + } + let state = new State(); + + let Root = defComponent( + '{{state.counter}}<:default><:error as |err|>caught', + { scope: { ErrorBoundary, Throwing, state } } + ); + + this.renderComponent(Root, { expect: '0caught' }); + + this.assertChange({ + change: () => state.counter++, + expect: '1caught', + }); + + this.assertChange({ + change: () => state.counter++, + expect: '2caught', + }); + } + + '@test outer tracked state continues to work across boundary error transitions'() { + class State { + @tracked label = 'hello'; + } + let state = new State(); + + let Root = defComponent( + '{{state.label}}<:default><:error as |err|>caught', + { scope: { ErrorBoundary, Throwing, state } } + ); + + this.renderComponent(Root, { expect: 'hellocaught' }); + + this.assertChange({ + change: () => (state.label = 'world'), + expect: 'worldcaught', + }); + } + + '@test catches error from conditional branch during initial render'() { + let Root = defComponent( + '<:default>{{#if true}}{{else}}safe{{/if}}<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'caught: render error' }); + } + + '@test catches error when tracked array mutation causes throw during rerender'() { + class State { + @tracked items = ['a', 'b']; + } + let state = new State(); + + let ItemComponent = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.item === 'bomb') { + throw new Error('bomb item'); + } + return (this as any).args.item; + } + }, + }); + + let Root = defComponent( + '<:default>{{#each state.items as |item|}}{{/each}}<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, ItemComponent, state } } + ); + + this.renderComponent(Root, { expect: 'ab' }); + + this.assertChange({ + change: () => (state.items = ['a', 'b', 'bomb']), + expect: 'caught: bomb item', + }); + } + + // Modifier errors are not caught by ErrorBoundary because modifiers install + // during transaction.commit(), which runs after VM execution completes. + // ErrorBoundary only catches errors during the VM render phase. + '@skip catches error thrown by a modifier'() { + let throwingModifier = defineSimpleModifier(() => { + throw new Error('modifier error'); + }); + + let Root = defComponent( + '<:default>
content
<:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, throwingModifier } } + ); + + this.renderComponent(Root, { expect: 'caught: modifier error' }); + } + + '@test destructors are called when boundary transitions to error state'(assert: Assert) { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + // Use a component manager with destructor capability so the component + // instance enters the destroyable hierarchy. The default + // GlimmerishComponentManager has no destructor support, so + // registerDestructor on the component instance would never fire. + class DestroyableComponent { + args: any; + constructor(owner: any, args: any) { + setOwner(this, owner); + this.args = args; + registerDestructor(this, () => assert.step('destroyed'), true); + } + + get value() { + if (this.args.shouldThrow) { + throw new Error('conditional error'); + } + return 'alive'; + } + } + + setComponentManager( + () => ({ + capabilities: componentCapabilities('3.13', { destructor: true }), + createComponent(Factory: any, args: any) { + return new Factory(undefined, args.named); + }, + getContext(component: any) { + return component; + }, + destroyComponent(component: any) { + destroy(component); + }, + }), + DestroyableComponent + ); + + let Tracked = defComponent('{{this.value}}', { + component: DestroyableComponent as any, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, Tracked, state } } + ); + + this.renderComponent(Root, { expect: 'alive' }); + + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + assert.verifySteps(['destroyed']); + } + + '@test retry that still throws shows error block again'() { + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, Throwing, on } } + ); + + this.renderComponent(Root, { expect: 'caught ' }); + + this.assertChange({ + change: () => clickElement('button'), + expect: 'caught ', + }); + } + + '@test deeply nested components with conditionals — error in conditional branch'() { + class State { + @tracked showDanger = false; + } + let state = new State(); + + let Root = defComponent( + '<:default>{{#if state.showDanger}}{{else}}safe{{/if}}<:error as |err|>caught', + { scope: { ErrorBoundary, Throwing, state } } + ); + + this.renderComponent(Root, { expect: 'safe' }); + + this.assertChange({ + change: () => (state.showDanger = true), + expect: 'caught', + }); + } } ); diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index c0ebcd6abbf..4c9d59993dd 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -109,10 +109,14 @@ export class NewTreeBuilder implements TreeBuilder { * Creates a tree builder that renders into a fresh (empty) resettable block. * Unlike `resume`, this does NOT call `block.reset()`, so it's safe for * blocks that have never been rendered or have partially-initialized children. + * + * @param nextSibling - Optional insertion point. When provided, new content + * is inserted before this node instead of appended at the end. Used by + * error boundary recovery to maintain correct position among siblings. */ - static beginBlock(env: Environment, block: ResettableBlock): NewTreeBuilder { + static beginBlock(env: Environment, block: ResettableBlock, nextSibling?: Nullable): NewTreeBuilder { let parentNode = block.parentElement(); - let stack = new this(env, parentNode, null).initialize(); + let stack = new this(env, parentNode, nextSibling ?? null).initialize(); stack.pushBlock(block); return stack; diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index aa79b325563..dab987bc2ca 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -11,6 +11,7 @@ import type { ResettableBlock, Scope, SimpleComment, + SimpleNode, UpdatingOpcode, UpdatingVM as IUpdatingVM, } from '@glimmer/interfaces'; @@ -204,6 +205,14 @@ export class TryOpcode extends BlockOpcode implements ExceptionHandler { export class ErrorBoundaryOpcode extends TryOpcode { public type = 'error-boundary'; + // Cache DOM references from the last successful render so we can clean up + // even when the bounds tree is corrupted by a failed inner re-render. + // We cache the first node, and the nextSibling AFTER the last node, so that + // cleanup removes everything from firstNode up to (but not including) + // nextSibling — including any dynamically inserted nodes like list markers. + private lastFirstNode: SimpleNode | null = null; + private lastNextSibling: SimpleNode | null = null; + constructor( state: Closure, context: EvaluationContext, @@ -215,6 +224,17 @@ export class ErrorBoundaryOpcode extends TryOpcode { } override evaluate(vm: UpdatingVM) { + // Snapshot current DOM boundaries before child opcodes run. + // We need the nextSibling after lastNode (not lastNode itself) because + // child opcodes may insert temporary nodes (e.g., list sync markers) + // after lastNode. Using nextSibling ensures cleanup covers those too. + try { + this.lastFirstNode = this.bounds.firstNode(); + this.lastNextSibling = this.bounds.lastNode().nextSibling; + } catch { + // Bounds not yet initialized (first render) — no cache needed. + } + vm.try(this.children, this); } @@ -245,7 +265,28 @@ export class ErrorBoundaryOpcode extends TryOpcode { this.errorState.setError(error); - let tree = NewTreeBuilder.resume(env, bounds); + // Clean up DOM manually rather than using bounds.reset() (via resume()), + // because the bounds tree may be corrupted: inner TryOpcodes or + // ListBlockOpcodes can leave child blocks with null first/last pointers, + // and list sync may have inserted temporary marker nodes outside the + // bounds tree. Walking the DOM directly using cached node references + // handles both cases. + let parent = bounds.parentElement(); + + if (this.lastFirstNode && this.lastFirstNode.parentNode === parent) { + let current: SimpleNode | null = this.lastFirstNode; + let stop = this.lastNextSibling; + + while (current && current !== stop) { + let next: SimpleNode | null = current.nextSibling; + parent.removeChild(current); + current = next; + } + } + + bounds.resetPartial(); + let tree = NewTreeBuilder.beginBlock(env, bounds, this.lastNextSibling); + let vm = this.state.evaluate(tree); let children = (this.children = []); From 1400df39f83448005c6ed74fac00353e15bff41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Tue, 3 Mar 2026 23:25:52 -0300 Subject: [PATCH 04/12] [PERF] Remove try-catch from ErrorBoundary hot path and clear stale DOM refs Replace try-catch in evaluate() with unconditional snapshot + debug assertion, since bounds are always initialized. Null out cached DOM references after error recovery to avoid retaining detached nodes. --- packages/@glimmer/runtime/lib/vm/update.ts | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index dab987bc2ca..7af6299929d 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -228,12 +228,18 @@ export class ErrorBoundaryOpcode extends TryOpcode { // We need the nextSibling after lastNode (not lastNode itself) because // child opcodes may insert temporary nodes (e.g., list sync markers) // after lastNode. Using nextSibling ensures cleanup covers those too. - try { - this.lastFirstNode = this.bounds.firstNode(); - this.lastNextSibling = this.bounds.lastNode().nextSibling; - } catch { - // Bounds not yet initialized (first render) — no cache needed. + // + // Bounds are always initialized here because: + // - Initial render completes (with finalize()) before UpdatingVM is created. + // - transitionToError() re-renders synchronously, repopulating bounds. + if (LOCAL_DEBUG) { + expect( + this.bounds.firstNode(), + 'BUG: ErrorBoundaryOpcode.evaluate() called with uninitialized bounds' + ); } + this.lastFirstNode = this.bounds.firstNode(); + this.lastNextSibling = this.bounds.lastNode().nextSibling; vm.try(this.children, this); } @@ -297,6 +303,11 @@ export class ErrorBoundaryOpcode extends TryOpcode { }); associateDestroyableChild(this, result.drop); + + // Clear cached DOM references — they pointed at the pre-error content + // which has been removed. Fresh values are captured in the next evaluate(). + this.lastFirstNode = null; + this.lastNextSibling = null; } } From bd06d8131fbdf70f1c3517aadc34057ea679a123 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Tue, 3 Mar 2026 23:37:30 -0300 Subject: [PATCH 05/12] Restore accidentally removed ember-template-compiler mapping in package.json The ember-template-compiler/index.js exposed dependency mapping was unintentionally removed in an earlier commit. --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 79a3027e353..e7f45dbf34f 100644 --- a/package.json +++ b/package.json @@ -346,6 +346,7 @@ "@simple-dom/document/index.js": "ember-source/@simple-dom/document/index.js", "backburner.js/index.js": "ember-source/backburner.js/index.js", "dag-map/index.js": "ember-source/dag-map/index.js", + "ember-template-compiler/index.js": "ember-source/ember-template-compiler/index.js", "ember-testing/index.js": "ember-source/ember-testing/index.js", "ember-testing/lib/adapters/adapter.js": "ember-source/ember-testing/lib/adapters/adapter.js", "ember-testing/lib/adapters/qunit.js": "ember-source/ember-testing/lib/adapters/qunit.js", From 39adb0d64c94e80631bc132e10f225ad8aea9d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Wed, 4 Mar 2026 18:23:28 -0300 Subject: [PATCH 06/12] [BUGFIX] Prevent tracking state corruption during error recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace vm.execute() with vm.executeGuarded() in nested VM contexts to prevent resetTracking() from destroying parent tracking frames. In DEBUG mode, AppendingVM.execute() calls resetTracking() on error, which wipes ALL tracking state including the parent UpdatingVM's frames. This made ErrorBoundary recovery impossible and caused "attempted to close a tracking frame" crashes. Fixes: - Backtracking assertions on tracked mutation after error recovery - Double-trigger tracking frame corruption (error → error → retry) - Error-in-error-block rendering failures bubbling incorrectly - ListBlockOpcode.insertItem tracking corruption when new items throw Changes: - TryOpcode.handleException(): use executeGuarded() so errors propagate cleanly to the UpdatingVM's catch handler without wiping tracking state - ErrorBoundaryOpcode.handleException(): restore tracking before re-render, use executeGuarded(), add robust DOM cleanup with lastPreviousSibling fallback for when inner TryOpcodes detach cached nodes via bounds.reset() - ErrorBoundaryOpcode.handleError(): restore tracking and consumed tags before transitioning to error state - ErrorBoundaryOpcode.transitionToError(): use executeGuarded(), add previousSibling-based DOM cleanup fallback, roll back debug render tree - ListBlockOpcode.insertItem(): use executeGuarded() - Add resetConsumedTags() to debug.ts to discard stale CONSUMED_TAGS entries after popping tracking frames during error recovery Adjusts assertion count in "readable error stack" test (7→6): the single outer console.error from UpdatingVM.execute() now captures full tracking info since the inner call no longer prematurely wipes it. Adds 5 regression tests covering all fix scenarios. --- .../components/error-boundary-test.ts | 161 ++++++++++++++++++ .../lib/suites/components.ts | 2 +- packages/@glimmer/runtime/lib/vm/update.ts | 137 +++++++++++++-- packages/@glimmer/validator/lib/debug.ts | 14 ++ 4 files changed, 303 insertions(+), 11 deletions(-) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index 2aae4b99eea..0223c8330f4 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -550,5 +550,166 @@ moduleFor( expect: 'caught', }); } + + // --- Regression tests for tracking state corruption bugs --- + + '@test rerender error then fix state and retry recovers'() { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, MaybeThrow, state, on } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error via rerender + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Fix state and retry — must not cause backtracking assertion + state.shouldThrow = false; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + } + + '@test repeated rerender errors do not corrupt tracking state'() { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, MaybeThrow, state, on } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // First trigger + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Second trigger (same value — still dirties tag, causes revalidation) + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Fix and retry + state.shouldThrow = false; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + } + + '@test error in error block fallback bubbles to parent boundary'() { + let ThrowingFallback = defComponent('{{this.boom}}', { + component: class extends GlimmerishComponent { + get boom(): never { + throw new Error('fallback error'); + } + }, + }); + + let Root = defComponent( + '<:default><:default><:error as |err|><:error as |err|>outer caught: {{err.message}}', + { scope: { ErrorBoundary, Throwing, ThrowingFallback } } + ); + + this.renderComponent(Root, { expect: 'outer caught: fallback error' }); + } + + '@test each loop insert error then retry recovers'() { + class State { + @tracked items = ['a', 'b']; + } + let state = new State(); + + let ItemComponent = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.item === 'bomb') { + throw new Error('bomb'); + } + return (this as any).args.item; + } + }, + }); + + let Root = defComponent( + '<:default>{{#each state.items as |item|}}{{/each}}<:error as |err retry|>caught ', + { scope: { ErrorBoundary, ItemComponent, state, on } } + ); + + this.renderComponent(Root, { expect: 'ab' }); + + // Add bad item — triggers insertItem path + this.assertChange({ + change: () => (state.items = ['a', 'b', 'bomb']), + expect: 'caught ', + }); + + // Fix and retry + state.items = ['a', 'b']; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ab', + }); + } + + '@test each loop repeated insert errors then retry recovers'() { + class State { + @tracked items = ['a', 'b']; + } + let state = new State(); + + let ItemComponent = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.item === 'bomb') { + throw new Error('bomb'); + } + return (this as any).args.item; + } + }, + }); + + let Root = defComponent( + '<:default>{{#each state.items as |item|}}{{/each}}<:error as |err retry|>caught ', + { scope: { ErrorBoundary, ItemComponent, state, on } } + ); + + this.renderComponent(Root, { expect: 'ab' }); + + // First bad mutation + this.assertChange({ + change: () => (state.items = ['a', 'b', 'bomb']), + expect: 'caught ', + }); + + // Second bad mutation while in error state + this.assertChange({ + change: () => (state.items = ['a', 'bomb', 'c']), + expect: 'caught ', + }); + + // Fix and retry + state.items = ['x', 'y']; + this.assertChange({ + change: () => clickElement('button'), + expect: 'xy', + }); + } } ); diff --git a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts index 71ced2a1246..f0a79bccd84 100644 --- a/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts +++ b/packages/@glimmer-workspace/integration-tests/lib/suites/components.ts @@ -879,7 +879,7 @@ export class GlimmerishComponents extends RenderTest { }; try { - assert.expect(7); + assert.expect(6); this.registerComponent( 'Glimmer', diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 7af6299929d..64e9bfa6875 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -193,7 +193,13 @@ export class TryOpcode extends BlockOpcode implements ExceptionHandler { let children = (this.children = []); - let result = vm.execute((vm) => { + // Use executeGuarded() instead of execute() to prevent resetTracking() + // from being called if the re-render throws. resetTracking() wipes ALL + // tracking state (including parent frames), which corrupts the UpdatingVM's + // tracking context and causes "attempted to close a tracking frame, but one + // was not open" errors. With executeGuarded(), errors propagate to the + // UpdatingVM's catch handler, which can route them to an error boundary. + let result = vm.executeGuarded((vm) => { vm.updateWith(this); vm.pushUpdating(children); }); @@ -207,11 +213,17 @@ export class ErrorBoundaryOpcode extends TryOpcode { // Cache DOM references from the last successful render so we can clean up // even when the bounds tree is corrupted by a failed inner re-render. - // We cache the first node, and the nextSibling AFTER the last node, so that - // cleanup removes everything from firstNode up to (but not including) - // nextSibling — including any dynamically inserted nodes like list markers. + // We cache the first node, its previous sibling, and the nextSibling AFTER + // the last node, so that cleanup removes everything from firstNode up to + // (but not including) nextSibling — including any dynamically inserted nodes + // like list markers. The previousSibling is needed because inner TryOpcodes + // may detach lastFirstNode from the DOM via bounds.reset() before the error + // reaches us. private lastFirstNode: SimpleNode | null = null; + private lastPreviousSibling: SimpleNode | null = null; private lastNextSibling: SimpleNode | null = null; + private lastRenderTreeDepth = 0; + private lastTrackingDepth = 0; constructor( state: Closure, @@ -239,16 +251,97 @@ export class ErrorBoundaryOpcode extends TryOpcode { ); } this.lastFirstNode = this.bounds.firstNode(); + this.lastPreviousSibling = this.lastFirstNode.previousSibling; this.lastNextSibling = this.bounds.lastNode().nextSibling; + this.lastRenderTreeDepth = vm.env.debugRenderTree?.getDepth() ?? 0; + this.lastTrackingDepth = getTrackingDepth(); vm.try(this.children, this); } override handleException() { + let { + bounds, + context: { env }, + } = this; + let parent = bounds.parentElement(); + + // Restore tracking to the depth captured in evaluate(), BEFORE children + // ran. When vm.throw() is called (e.g. by an Assert opcode), it + // short-circuits the children's frame — any tracking frames opened by + // BeginTrackFrameOpcode will never see their matching EndTrackFrameOpcode. + // Without this, stale frames (and their DEBUG tracking transactions) leak + // onto OPEN_TRACK_FRAMES / TRANSACTION_STACK, keeping CONSUMED_TAGS alive + // and causing spurious backtracking assertions on later mutations. + restoreTrackingTo(this.lastTrackingDepth); + // Discard consumed-tag entries from the popped tracking frames. The + // CONSUMED_TAGS WeakMap can't be selectively pruned, so replace it + // with a fresh one. Without this, stale entries (e.g. a @tracked + // property consumed during the failed render) cause backtracking + // assertions when the property is later mutated outside the render. + if (DEBUG) { + debug.resetConsumedTags?.(); + } + let trackingDepth = this.lastTrackingDepth; + + // Save insertion marker: the sibling just before the boundary's content. + // After resume() removes old DOM via bounds.reset(), any nodes between + // this marker and lastNextSibling are partial leftovers from a failed render. + let insertionMarker = this.lastFirstNode + ? this.lastFirstNode.previousSibling + : null; + + destroyChildren(this); + try { - super.handleException(); + // Attempt a normal re-render like TryOpcode.handleException(), but use + // executeGuarded() instead of execute(). In DEBUG mode, execute() calls + // resetTracking() on error, which wipes out the parent UpdatingVM's + // tracking transaction (TRANSACTION_STACK / CONSUMED_TAGS), causing + // spurious backtracking assertions on later tracked property mutations. + let tree = NewTreeBuilder.resume(env, bounds); + let vm = this.state.evaluate(tree); + let children = (this.children = []); + let result = vm.executeGuarded((vm) => { + vm.updateWith(this); + vm.pushUpdating(children); + }); + associateDestroyableChild(this, result.drop); } catch (error) { - this.transitionToError(error); + // Restore tracking frames opened by the failed re-render attempt. + restoreTrackingTo(trackingDepth); + + // Roll back stale debug render tree entries. + env.debugRenderTree?.rollbackTo(this.lastRenderTreeDepth); + + // Remove partial DOM nodes left by the failed re-render. + // resume() already removed old content via bounds.reset(), so any nodes + // between insertionMarker and lastNextSibling are from the failed render. + let cursor: SimpleNode | null = insertionMarker + ? insertionMarker.nextSibling + : parent.firstChild; + let stop = this.lastNextSibling; + while (cursor && cursor !== stop) { + let next: SimpleNode | null = cursor.nextSibling; + parent.removeChild(cursor); + cursor = next; + } + + bounds.resetPartial(); + this.errorState.setError(error); + + let retryTree = NewTreeBuilder.beginBlock(env, bounds, this.lastNextSibling); + let retryVM = this.state.evaluate(retryTree); + let children = (this.children = []); + let result = retryVM.executeGuarded((vm) => { + vm.updateWith(this); + vm.pushUpdating(children); + }); + associateDestroyableChild(this, result.drop); + + this.lastFirstNode = null; + this.lastPreviousSibling = null; + this.lastNextSibling = null; } } @@ -258,6 +351,14 @@ export class ErrorBoundaryOpcode extends TryOpcode { * skipping inner TryOpcode handlers that would corrupt block state. */ handleError(error: unknown) { + // Restore tracking to the depth from evaluate(), before children ran. + // _execute's catch only restores to the depth of the failing opcode, + // which doesn't cover tracking frames opened by earlier opcodes + // (like BeginTrackFrameOpcode) in the children's frame. + restoreTrackingTo(this.lastTrackingDepth); + if (DEBUG) { + debug.resetConsumedTags?.(); + } this.transitionToError(error); } @@ -279,8 +380,18 @@ export class ErrorBoundaryOpcode extends TryOpcode { // handles both cases. let parent = bounds.parentElement(); - if (this.lastFirstNode && this.lastFirstNode.parentNode === parent) { - let current: SimpleNode | null = this.lastFirstNode; + if (this.lastFirstNode) { + // Determine cleanup start: if lastFirstNode is still in the DOM, start + // there. If it was detached (by an inner TryOpcode's bounds.reset()), + // use the cached previousSibling to find the current start point. + let current: SimpleNode | null; + if (this.lastFirstNode.parentNode === parent) { + current = this.lastFirstNode; + } else if (this.lastPreviousSibling) { + current = this.lastPreviousSibling.nextSibling; + } else { + current = parent.firstChild; + } let stop = this.lastNextSibling; while (current && current !== stop) { @@ -290,6 +401,11 @@ export class ErrorBoundaryOpcode extends TryOpcode { } } + // Roll back the debug render tree stack to discard stale entries left by + // DebugRenderTreeUpdateOpcodes that pushed but never got their matching + // DebugRenderTreeDidRenderOpcode pop due to the error. + env.debugRenderTree?.rollbackTo(this.lastRenderTreeDepth); + bounds.resetPartial(); let tree = NewTreeBuilder.beginBlock(env, bounds, this.lastNextSibling); @@ -297,7 +413,7 @@ export class ErrorBoundaryOpcode extends TryOpcode { let children = (this.children = []); - let result = vm.execute((vm) => { + let result = vm.executeGuarded((vm) => { vm.updateWith(this); vm.pushUpdating(children); }); @@ -307,6 +423,7 @@ export class ErrorBoundaryOpcode extends TryOpcode { // Clear cached DOM references — they pointed at the pre-error content // which has been removed. Fresh values are captured in the next evaluate(). this.lastFirstNode = null; + this.lastPreviousSibling = null; this.lastNextSibling = null; } } @@ -500,7 +617,7 @@ export class ListBlockOpcode extends BlockOpcode { let vm = state.evaluate(elementStack); - vm.execute((vm) => { + vm.executeGuarded((vm) => { let opcode = vm.enterItem(item); opcode.index = children.length; diff --git a/packages/@glimmer/validator/lib/debug.ts b/packages/@glimmer/validator/lib/debug.ts index 3578a060562..36b9f4c70c8 100644 --- a/packages/@glimmer/validator/lib/debug.ts +++ b/packages/@glimmer/validator/lib/debug.ts @@ -12,6 +12,7 @@ interface DebugTransaction { runInTrackingTransaction?: undefined | ((fn: () => T, debuggingContext?: string | false) => T); resetTrackingTransaction?: undefined | (() => string); + resetConsumedTags?: undefined | (() => void); setTrackingTransactionEnv?: | undefined | ((env: { debugMessage?(obj?: unknown, keyName?: string): string }) => void); @@ -88,6 +89,19 @@ if (DEBUG) { } }; + /** + * Replace CONSUMED_TAGS with a fresh WeakMap, discarding all entries. + * Used during error boundary recovery: tracking frames from a failed render + * are popped via restoreTrackingTo(), but their consumed-tag entries persist + * in the WeakMap because it can't be selectively pruned. Without this, + * stale entries cause spurious backtracking assertions on later mutations. + */ + debug.resetConsumedTags = () => { + if (TRANSACTION_STACK.length > 0) { + CONSUMED_TAGS = new WeakMap(); + } + }; + debug.resetTrackingTransaction = () => { let stack = ''; From f83f3f9f0885deb4c597e9ecfcdafb86816e840a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Wed, 4 Mar 2026 19:50:49 -0300 Subject: [PATCH 07/12] Fix CI type checking, linting, and formatting errors - Add handleError() to ExceptionHandler interface, removing unsafe `any` casts in UpdatingVMFrame.handleError() - Cast tagFor() results to UpdatableTag in ErrorBoundaryState.setError() to satisfy dirtyTag() parameter type - Cast array access in restoreTrackingTo() to fix Tracker | undefined not assignable to Tracker | null - Extract inline decorated class in test to fix "decorators are not valid here" TS error - Change defineSimpleHelper param from boolean to unknown to match expected signature - Remove unused imports (destroyChildren, clear) from component.ts - Run prettier on all modified files --- .../components/error-boundary-test.ts | 17 ++++++++-------- .../interfaces/lib/runtime/render.d.ts | 1 + .../runtime/lib/compiled/opcodes/component.ts | 20 ++++--------------- .../runtime/lib/component/error-boundary.ts | 5 +++-- .../runtime/lib/vm/element-builder.ts | 6 +++++- packages/@glimmer/runtime/lib/vm/update.ts | 12 +++-------- packages/@glimmer/validator/lib/tracking.ts | 7 ++++--- 7 files changed, 28 insertions(+), 40 deletions(-) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index 0223c8330f4..02a0d25e286 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -65,10 +65,7 @@ class ErrorBoundaryTestCase extends AbstractStrictTestCase { this.assertStableRerender(); } - renderComponent( - component: object, - options: { args?: Record; expect: string } - ) { + renderComponent(component: object, options: { args?: Record; expect: string }) { let { owner } = this; run(() => { @@ -193,13 +190,15 @@ moduleFor( } let state = new State(); + class CounterComponent extends GlimmerishComponent { + @tracked count = 0; + increment = () => this.count++; + } + let Counter = defComponent( '{{this.count}}', { - component: class extends GlimmerishComponent { - @tracked count = 0; - increment = () => this.count++; - }, + component: CounterComponent, scope: { on }, } ); @@ -271,7 +270,7 @@ moduleFor( } let state = new State(); - let maybeThrowHelper = defineSimpleHelper((shouldThrow: boolean) => { + let maybeThrowHelper = defineSimpleHelper((shouldThrow: unknown) => { if (shouldThrow) throw new Error('helper rerender error'); return 'helper ok'; }); diff --git a/packages/@glimmer/interfaces/lib/runtime/render.d.ts b/packages/@glimmer/interfaces/lib/runtime/render.d.ts index bd8ad0a2023..a5835eed660 100644 --- a/packages/@glimmer/interfaces/lib/runtime/render.d.ts +++ b/packages/@glimmer/interfaces/lib/runtime/render.d.ts @@ -6,6 +6,7 @@ import type { Environment } from './environment.js'; export interface ExceptionHandler { handleException(): void; + handleError?(error: unknown): void; } export interface RenderResult extends Bounds, ExceptionHandler { diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index 0bddcee7172..c43461bf516 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -69,7 +69,7 @@ import { CheckSyscallRegister, } from '@glimmer/debug'; import { debugToString, expect, localAssert, unwrap, unwrapTemplate } from '@glimmer/debug-util'; -import { associateDestroyableChild, destroyChildren, registerDestructor } from '@glimmer/destroyable'; +import { associateDestroyableChild, registerDestructor } from '@glimmer/destroyable'; import { managerHasCapability } from '@glimmer/manager'; import { isConstRef, valueForRef } from '@glimmer/reference'; import { assign, dict, EMPTY_STRING_ARRAY, enumerate } from '@glimmer/util'; @@ -84,7 +84,7 @@ import { getTrackingDepth, restoreTrackingTo } from '@glimmer/validator'; import { NewTreeBuilder } from '../../vm/element-builder'; import { ErrorBoundaryOpcode } from '../../vm/update'; -import { clear, ConcreteBounds } from '../../bounds'; +import { ConcreteBounds } from '../../bounds'; import type { ErrorBoundaryStateInterface } from '../../component/error-boundary'; import { hasCustomDebugRenderTreeLifecycle } from '../../component/interfaces'; import { resolveComponent } from '../../component/resolve'; @@ -911,13 +911,7 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } let subVM = closure.evaluate(subTree); let children: UpdatingOpcode[] = []; - let errorBoundaryOp = new ErrorBoundaryOpcode( - closure, - vm.context, - block, - children, - errorState - ); + let errorBoundaryOp = new ErrorBoundaryOpcode(closure, vm.context, block, children, errorState); // Use executeGuarded to avoid resetting the parent VM's tracking state. let result = subVM.executeGuarded((subVM) => { @@ -955,13 +949,7 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } let retryVM = closure.evaluate(retryTree); let children: UpdatingOpcode[] = []; - let errorBoundaryOp = new ErrorBoundaryOpcode( - closure, - vm.context, - block, - children, - errorState - ); + let errorBoundaryOp = new ErrorBoundaryOpcode(closure, vm.context, block, children, errorState); let result = retryVM.executeGuarded((retryVM) => { retryVM.updateWith(errorBoundaryOp); diff --git a/packages/@glimmer/runtime/lib/component/error-boundary.ts b/packages/@glimmer/runtime/lib/component/error-boundary.ts index b22164a0857..aa4cec83a23 100644 --- a/packages/@glimmer/runtime/lib/component/error-boundary.ts +++ b/packages/@glimmer/runtime/lib/component/error-boundary.ts @@ -1,3 +1,4 @@ +import type { UpdatableTag } from '@glimmer/validator'; import { consumeTag, dirtyTag, dirtyTagFor, tagFor } from '@glimmer/validator'; export interface ErrorBoundaryStateInterface { @@ -27,9 +28,9 @@ export class ErrorBoundaryState implements ErrorBoundaryStateInterface { // is called from an error boundary catch handler, where the tag was // already consumed during the (failed) render. This backflow is // intentional for error boundary recovery. - dirtyTag(tagFor(this, '_error'), true); + dirtyTag(tagFor(this, '_error') as UpdatableTag, true); this._hasError = true; - dirtyTag(tagFor(this, '_hasError'), true); + dirtyTag(tagFor(this, '_hasError') as UpdatableTag, true); } retry = () => { diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index 4c9d59993dd..b65c729f92e 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -114,7 +114,11 @@ export class NewTreeBuilder implements TreeBuilder { * is inserted before this node instead of appended at the end. Used by * error boundary recovery to maintain correct position among siblings. */ - static beginBlock(env: Environment, block: ResettableBlock, nextSibling?: Nullable): NewTreeBuilder { + static beginBlock( + env: Environment, + block: ResettableBlock, + nextSibling?: Nullable + ): NewTreeBuilder { let parentNode = block.parentElement(); let stack = new this(env, parentNode, nextSibling ?? null).initialize(); stack.pushBlock(block); diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 64e9bfa6875..30a78e9ced9 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -287,9 +287,7 @@ export class ErrorBoundaryOpcode extends TryOpcode { // Save insertion marker: the sibling just before the boundary's content. // After resume() removes old DOM via bounds.reset(), any nodes between // this marker and lastNextSibling are partial leftovers from a failed render. - let insertionMarker = this.lastFirstNode - ? this.lastFirstNode.previousSibling - : null; + let insertionMarker = this.lastFirstNode ? this.lastFirstNode.previousSibling : null; destroyChildren(this); @@ -705,12 +703,8 @@ class UpdatingVMFrame { * Only error boundary handlers accept JavaScript errors. */ handleError(error: unknown): boolean { - if ( - this.exceptionHandler && - 'handleError' in this.exceptionHandler && - typeof (this.exceptionHandler as any).handleError === 'function' - ) { - (this.exceptionHandler as any).handleError(error); + if (this.exceptionHandler?.handleError) { + this.exceptionHandler.handleError(error); return true; } return false; diff --git a/packages/@glimmer/validator/lib/tracking.ts b/packages/@glimmer/validator/lib/tracking.ts index 2527b4afd63..f164a6034b7 100644 --- a/packages/@glimmer/validator/lib/tracking.ts +++ b/packages/@glimmer/validator/lib/tracking.ts @@ -116,9 +116,10 @@ export function restoreTrackingTo(depth: number): void { unwrap(debug.endTrackingTransaction)(); } } - CURRENT_TRACKER = OPEN_TRACK_FRAMES.length > 0 - ? OPEN_TRACK_FRAMES[OPEN_TRACK_FRAMES.length - 1] - : null; + CURRENT_TRACKER = + OPEN_TRACK_FRAMES.length > 0 + ? (OPEN_TRACK_FRAMES[OPEN_TRACK_FRAMES.length - 1] as Tracker) + : null; } // This function is only for handling errors and resetting to a valid state From 882f5055153d9f7abe009758ca9d5f05062d2526 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Wed, 4 Mar 2026 22:16:10 -0300 Subject: [PATCH 08/12] Add DEBUG console.error logging when ErrorBoundary catches errors --- .../components/error-boundary-test.ts | 73 +++++++++++++++++++ .../runtime/lib/compiled/opcodes/component.ts | 5 ++ packages/@glimmer/runtime/lib/vm/update.ts | 9 +++ 3 files changed, 87 insertions(+) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index 02a0d25e286..bc5fe6a13e1 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -118,6 +118,79 @@ moduleFor( this.renderComponent(Root, { expect: 'caught' }); } + /* eslint-disable no-console */ + '@test logs caught error to console.error in DEBUG mode during initial render'(assert: Assert) { + let originalConsoleError = console.error; + let errors: unknown[][] = []; + console.error = (...args: unknown[]) => errors.push(args); + + try { + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, Throwing } } + ); + + this.renderComponent(Root, { expect: 'caught' }); + + assert.ok(errors.length > 0, 'console.error was called'); + assert.strictEqual( + errors[0]![0], + 'An error was caught by :', + 'logs the expected message' + ); + assert.ok(errors[0]![1] instanceof Error, 'logs the error object'); + assert.strictEqual( + (errors[0]![1] as Error).message, + 'render error', + 'logs the correct error' + ); + } finally { + console.error = originalConsoleError; + } + } + + '@test logs caught error to console.error in DEBUG mode during rerender'(assert: Assert) { + let originalConsoleError = console.error; + let errors: unknown[][] = []; + + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, MaybeThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Start capturing after initial render + console.error = (...args: unknown[]) => errors.push(args); + + try { + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + assert.ok(errors.length > 0, 'console.error was called during rerender'); + assert.strictEqual( + errors[0]![0], + 'An error was caught by :', + 'logs the expected message' + ); + assert.strictEqual( + (errors[0]![1] as Error).message, + 'conditional error', + 'logs the correct error' + ); + } finally { + console.error = originalConsoleError; + } + } + /* eslint-enable no-console */ + '@test passes error object to error block'() { let Root = defComponent( '<:default><:error as |err|>caught: {{err.message}}', diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts index c43461bf516..296776c3032 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/component.ts @@ -923,6 +923,11 @@ APPEND_OPCODES.add(VM_INVOKE_COMPONENT_LAYOUT_GUARDED_OP, (vm, { op1: register } vm.associateDestroyable(errorBoundaryOp); vm.updateWith(errorBoundaryOp); } catch (error) { + if (DEBUG) { + // eslint-disable-next-line no-console + console.error('An error was caught by :', error); + } + // Roll back stale tracking frames left by the failed sub-VM render. restoreTrackingTo(trackingDepth); diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 30a78e9ced9..00afe0f8e05 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -306,6 +306,11 @@ export class ErrorBoundaryOpcode extends TryOpcode { }); associateDestroyableChild(this, result.drop); } catch (error) { + if (DEBUG) { + // eslint-disable-next-line no-console + console.error('An error was caught by :', error); + } + // Restore tracking frames opened by the failed re-render attempt. restoreTrackingTo(trackingDepth); @@ -349,6 +354,10 @@ export class ErrorBoundaryOpcode extends TryOpcode { * skipping inner TryOpcode handlers that would corrupt block state. */ handleError(error: unknown) { + if (DEBUG) { + // eslint-disable-next-line no-console + console.error('An error was caught by :', error); + } // Restore tracking to the depth from evaluate(), before children ran. // _execute's catch only restores to the depth of the failing opcode, // which doesn't cover tracking frames opened by earlier opcodes From 626cf138746f6d0fbe4eef1a402ea3d7ff8611f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Thu, 5 Mar 2026 12:58:26 -0300 Subject: [PATCH 09/12] [BUGFIX] Remove ErrorBoundary re-export from @glimmer/component ErrorBoundary should only be importable from @ember/component, matching the pattern of Input and Textarea. The value re-export was pulling 51 source files into type-tests compilation, causing 349 type errors. --- packages/@glimmer/component/src/index.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/@glimmer/component/src/index.ts b/packages/@glimmer/component/src/index.ts index dd7ee2aa8db..55936ebf3a2 100644 --- a/packages/@glimmer/component/src/index.ts +++ b/packages/@glimmer/component/src/index.ts @@ -406,5 +406,3 @@ export default class Component extends _GlimmerComponent { setComponentManager((owner: Owner) => { return new GlimmerComponentManager(owner); }, Component); - -export { ErrorBoundary } from '@ember/component'; From 11d38f6ce2e0fbaefd079cf6afe4e2dbd171ba53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Thu, 5 Mar 2026 13:46:44 -0300 Subject: [PATCH 10/12] Skip DEBUG-only console.error tests in production builds The console.error logging is guarded by DEBUG in the source, so these tests can't pass in production builds. --- .../integration/components/error-boundary-test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index bc5fe6a13e1..c4c5e57f61b 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -10,6 +10,7 @@ import { runDestroy, } from 'internal-test-helpers'; +import { DEBUG } from '@glimmer/env'; import { ErrorBoundary, setComponentManager } from '@ember/component'; import { array, on } from '@glimmer/runtime'; import { tracked } from '@glimmer/tracking'; @@ -120,6 +121,11 @@ moduleFor( /* eslint-disable no-console */ '@test logs caught error to console.error in DEBUG mode during initial render'(assert: Assert) { + if (!DEBUG) { + assert.expect(0); + return; + } + let originalConsoleError = console.error; let errors: unknown[][] = []; console.error = (...args: unknown[]) => errors.push(args); @@ -150,6 +156,11 @@ moduleFor( } '@test logs caught error to console.error in DEBUG mode during rerender'(assert: Assert) { + if (!DEBUG) { + assert.expect(0); + return; + } + let originalConsoleError = console.error; let errors: unknown[][] = []; From a31e0130261d4b140aa6cf12374eef5595df3a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Thu, 5 Mar 2026 17:29:10 -0300 Subject: [PATCH 11/12] [BUGFIX] Isolate modifier install errors in transaction commit A single modifier throwing during transaction.commit() previously aborted the entire loop, leaving all subsequently-scheduled modifiers uninstalled. Wrap each install/update in try-catch, collect the first error, and re-throw after all modifiers have been processed. --- packages/@glimmer/runtime/lib/environment.ts | 76 +++++++++++++------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/packages/@glimmer/runtime/lib/environment.ts b/packages/@glimmer/runtime/lib/environment.ts index dfc6ad3a765..73e51f308cd 100644 --- a/packages/@glimmer/runtime/lib/environment.ts +++ b/packages/@glimmer/runtime/lib/environment.ts @@ -58,39 +58,63 @@ class TransactionImpl implements Transaction { let { scheduledInstallModifiers, scheduledUpdateModifiers } = this; + // Isolate each modifier install/update in a try-catch so that a single + // failing modifier does not prevent the remaining modifiers from being + // installed. Without this, all modifiers scheduled after the throwing one + // would be silently skipped, leaving their associated DOM elements without + // event listeners or other modifier behaviour. The first error encountered + // is re-thrown after all modifiers have been processed. + let firstError: unknown = null; + for (const { manager, state, definition } of scheduledInstallModifiers) { - let modifierTag = manager.getTag(state); - - if (modifierTag !== null) { - let tag = track( - () => manager.install(state), - DEBUG && - `- While rendering:\n (instance of a \`${ - definition.resolvedName || manager.getDebugName(definition.state) - }\` modifier)` - ); - updateTag(modifierTag, tag); - } else { - manager.install(state); + try { + let modifierTag = manager.getTag(state); + + if (modifierTag !== null) { + let tag = track( + () => manager.install(state), + DEBUG && + `- While rendering:\n (instance of a \`${ + definition.resolvedName || manager.getDebugName(definition.state) + }\` modifier)` + ); + updateTag(modifierTag, tag); + } else { + manager.install(state); + } + } catch (e) { + if (firstError === null) { + firstError = e; + } } } for (const { manager, state, definition } of scheduledUpdateModifiers) { - let modifierTag = manager.getTag(state); - - if (modifierTag !== null) { - let tag = track( - () => manager.update(state), - DEBUG && - `- While rendering:\n (instance of a \`${ - definition.resolvedName || manager.getDebugName(definition.state) - }\` modifier)` - ); - updateTag(modifierTag, tag); - } else { - manager.update(state); + try { + let modifierTag = manager.getTag(state); + + if (modifierTag !== null) { + let tag = track( + () => manager.update(state), + DEBUG && + `- While rendering:\n (instance of a \`${ + definition.resolvedName || manager.getDebugName(definition.state) + }\` modifier)` + ); + updateTag(modifierTag, tag); + } else { + manager.update(state); + } + } catch (e) { + if (firstError === null) { + firstError = e; + } } } + + if (firstError !== null) { + throw firstError; + } } } From 5e71e714c060fed92ce4b93f30a1a6ba05dd0c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= Date: Fri, 6 Mar 2026 16:41:48 -0300 Subject: [PATCH 12/12] Add @retryWith support to ErrorBoundary for automatic error recovery --- .../glimmer/lib/components/error-boundary.ts | 30 +- .../components/error-boundary-test.ts | 383 ++++++++++++++++++ .../runtime/lib/component/error-boundary.ts | 82 ++++ packages/@glimmer/runtime/lib/vm/update.ts | 39 +- packages/@glimmer/validator/lib/tracking.ts | 15 +- 5 files changed, 522 insertions(+), 27 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts b/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts index d0a79a91285..02c1fd3f8c4 100644 --- a/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts +++ b/packages/@ember/-internals/glimmer/lib/components/error-boundary.ts @@ -12,7 +12,7 @@ import type { } from '@glimmer/interfaces'; import type { Reference } from '@glimmer/reference'; import { setComponentTemplate, setInternalComponentManager } from '@glimmer/manager'; -import { createConstRef } from '@glimmer/reference'; +import { createConstRef, valueForRef } from '@glimmer/reference'; import { ErrorBoundaryState } from '@glimmer/runtime'; import ErrorBoundaryTemplate from '../templates/error-boundary'; @@ -21,12 +21,12 @@ const CAPABILITIES: InternalComponentCapabilities = { dynamicLayout: false, dynamicTag: false, prepareArgs: false, - createArgs: false, + createArgs: true, attributeHook: false, elementHook: false, createCaller: false, dynamicScope: false, - updateHook: false, + updateHook: true, createInstance: true, wrapped: false, willDestroy: false, @@ -44,13 +44,21 @@ class ErrorBoundaryManager create( _owner: Owner, _definition: object, - _args: Nullable, + args: Nullable, _env: Environment, _dynamicScope: Nullable, _caller: Nullable, _hasDefaultBlock: boolean ): ErrorBoundaryState { - return new ErrorBoundaryState(); + let state = new ErrorBoundaryState(); + + if (args && args.named.has('retryWith')) { + let retryWithRef = args.named.get('retryWith'); + state.retryWithRef = retryWithRef; + state._lastRetryWithValue = valueForRef(retryWithRef); + } + + return state; } didCreate(): void {} @@ -70,7 +78,17 @@ class ErrorBoundaryManager return null; } - update(_instance: ErrorBoundaryState, _dynamicScope: Nullable): void {} + update(instance: ErrorBoundaryState, _dynamicScope: Nullable): void { + // Consume the @retryWith ref's tag at the ROOT tracking level (outside + // the EB component's JumpIfNotModified scope). This ensures the root's + // combined tag includes the retryWith value, so root JumpIfNotModified + // falls through when @retryWith changes — which in turn allows the EB's + // opcodes to run. + // The actual reset logic is in ErrorBoundaryOpcode.evaluate(). + if (instance.retryWithRef) { + valueForRef(instance.retryWithRef); + } + } didSplatAttributes( _instance: ErrorBoundaryState, diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts index c4c5e57f61b..85eb6b664bc 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts +++ b/packages/@ember/-internals/glimmer/tests/integration/components/error-boundary-test.ts @@ -794,5 +794,388 @@ moduleFor( expect: 'xy', }); } + + // --- @retryWith tests --- + + '@test @retryWith resets error state when value changes'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked routeName = 'route-a'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, ConditionalThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught: route error', + }); + + // Change @retryWith value AND fix the error condition — boundary should reset + this.assertChange({ + change: () => { + state.shouldThrow = false; + state.routeName = 'route-b'; + }, + expect: 'ok', + }); + } + + '@test @retryWith does not reset if value unchanged'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked routeName = 'route-a'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, ConditionalThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + // Fix the throw condition but DON'T change @retryWith — should stay in error + this.assertChange({ + change: () => (state.shouldThrow = false), + expect: 'caught', + }); + } + + '@test @retryWith re-catches if new value also causes error'() { + let state = new (class { + @tracked routeName = 'route-a'; + })(); + + // Always throws regardless of route + let Root = defComponent( + '<:default><:error as |err|>caught: {{err.message}}', + { scope: { ErrorBoundary, Throwing, state } } + ); + + this.renderComponent(Root, { expect: 'caught: render error' }); + + // Change @retryWith — boundary resets, but immediately catches again + this.assertChange({ + change: () => (state.routeName = 'route-b'), + expect: 'caught: render error', + }); + } + + '@test @retryWith with retry that still throws re-catches'() { + let state = new (class { + @tracked routeName = 'route-a'; + })(); + + // Always throws + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, Throwing, state, on } } + ); + + this.renderComponent(Root, { expect: 'caught ' }); + + // Retry while error still exists — should re-catch + this.assertChange({ + change: () => clickElement('button'), + expect: 'caught ', + }); + } + + '@test @retryWith rerender error then retry without fixing re-catches'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked routeName = 'route-a'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, ConditionalThrow, state, on } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error via tracked state change + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Retry WITHOUT fixing state — error should be re-caught + this.assertChange({ + change: () => clickElement('button'), + expect: 'caught ', + }); + } + + '@test @retryWith with retry after fixing state recovers'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked routeName = 'route-a'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, ConditionalThrow, state, on } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Fix state and retry — should recover + state.shouldThrow = false; + + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + } + + '@test multiple error-recovery cycles do not require extra retry clicks'() { + class State { + @tracked shouldThrow = false; + } + let state = new State(); + + let Root = defComponent( + '<:default><:error as |err retry|>caught ', + { scope: { ErrorBoundary, MaybeThrow, state, on } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // --- Cycle 1: error → retry without fixing → re-catch → fix → retry → recover --- + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Retry without fixing — should re-catch + this.assertChange({ + change: () => clickElement('button'), + expect: 'caught ', + }); + + // Fix and retry — should recover + state.shouldThrow = false; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + + // --- Cycle 2: same sequence, should still recover in same number of clicks --- + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + // Retry without fixing — should re-catch + this.assertChange({ + change: () => clickElement('button'), + expect: 'caught ', + }); + + // Fix and retry — should recover (NOT require extra clicks) + state.shouldThrow = false; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + + // --- Cycle 3: one more to be sure --- + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught ', + }); + + state.shouldThrow = false; + this.assertChange({ + change: () => clickElement('button'), + expect: 'ok', + }); + } + + '@test @retryWith with array value resets when element changes'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked valA = 'a'; + @tracked valB = 'b'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, ConditionalThrow, state, array } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + // Change one array element AND fix the error — should reset + this.assertChange({ + change: () => { + state.shouldThrow = false; + state.valA = 'changed'; + }, + expect: 'ok', + }); + } + + '@test @retryWith with array value does not reset if elements unchanged'() { + let state = new (class { + @tracked shouldThrow = false; + @tracked valA = 'a'; + @tracked valB = 'b'; + })(); + + let ConditionalThrow = defComponent('{{this.value}}', { + component: class extends GlimmerishComponent { + get value() { + if ((this as any).args.shouldThrow) { + throw new Error('route error'); + } + return 'ok'; + } + }, + }); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, ConditionalThrow, state, array } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + // Fix throw condition but DON'T change array elements — should stay in error + this.assertChange({ + change: () => (state.shouldThrow = false), + expect: 'caught', + }); + } + + '@test @retryWith with undefined value works without error'() { + let state = new (class { + @tracked shouldThrow = false; + })(); + + // @retryWith is not passed — tests that undefined/missing arg is handled + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, MaybeThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error — should still catch normally + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + } + + '@test ErrorBoundary without @retryWith stays in error state'() { + let state = new (class { + @tracked shouldThrow = false; + })(); + + let Root = defComponent( + '<:default><:error as |err|>caught', + { scope: { ErrorBoundary, MaybeThrow, state } } + ); + + this.renderComponent(Root, { expect: 'ok' }); + + // Trigger error + this.assertChange({ + change: () => (state.shouldThrow = true), + expect: 'caught', + }); + + // Fix the condition — but without @retryWith, boundary stays in error + this.assertChange({ + change: () => (state.shouldThrow = false), + expect: 'caught', + }); + } } ); diff --git a/packages/@glimmer/runtime/lib/component/error-boundary.ts b/packages/@glimmer/runtime/lib/component/error-boundary.ts index aa4cec83a23..af8202d4395 100644 --- a/packages/@glimmer/runtime/lib/component/error-boundary.ts +++ b/packages/@glimmer/runtime/lib/component/error-boundary.ts @@ -1,4 +1,6 @@ +import type { Reference } from '@glimmer/reference'; import type { UpdatableTag } from '@glimmer/validator'; +import { valueForRef } from '@glimmer/reference'; import { consumeTag, dirtyTag, dirtyTagFor, tagFor } from '@glimmer/validator'; export interface ErrorBoundaryStateInterface { @@ -6,12 +8,61 @@ export interface ErrorBoundaryStateInterface { hasError: boolean; setError(error: unknown): void; retry(): void; + checkRetryWith(): boolean; +} + +/** + * Shallow-compare two values. Supports primitives (===), arrays (element-wise), + * and plain objects (own-property value-wise). + */ +function shallowEqual(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a == null || b == null) return false; + + if (Array.isArray(a) && Array.isArray(b)) { + if (a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) return false; + } + return true; + } + + if (typeof a === 'object' && typeof b === 'object') { + let keysA = Object.keys(a); + let keysB = Object.keys(b); + if (keysA.length !== keysB.length) return false; + for (let key of keysA) { + if ( + (a as Record)[key] !== (b as Record)[key] + ) { + return false; + } + } + return true; + } + + return false; +} + +/** + * Snapshot a value for later comparison. Clones arrays and plain objects + * to prevent reference aliasing. + */ +function snapshot(value: unknown): unknown { + if (Array.isArray(value)) return value.slice(); + if (value !== null && typeof value === 'object') return { ...value }; + return value; } export class ErrorBoundaryState implements ErrorBoundaryStateInterface { private _error: unknown = null; private _hasError = false; + // @retryWith support: tracks a reference whose value, when changed, + // automatically clears the error state and retries the default block. + retryWithRef: Reference | null = null; + _lastRetryWithValue: unknown = undefined; + get error(): unknown { consumeTag(tagFor(this, '_error')); return this._error; @@ -39,4 +90,35 @@ export class ErrorBoundaryState implements ErrorBoundaryStateInterface { this._hasError = false; dirtyTagFor(this, '_hasError'); }; + + /** + * Check if the @retryWith value has changed. If it has and the boundary + * is in error state, clear the error state and return true so the caller + * (ErrorBoundaryOpcode.evaluate()) can re-render via handleException(). + * + * Also consumes the retryWith ref's tag (via valueForRef), keeping it in + * the current tracking frame. This ensures the EB component's + * JumpIfNotModified detects future @retryWith changes. + */ + checkRetryWith(): boolean { + if (this.retryWithRef === null) return false; + + let current = valueForRef(this.retryWithRef); + + if (!shallowEqual(current, this._lastRetryWithValue)) { + this._lastRetryWithValue = snapshot(current); + + if (this._hasError) { + this._error = null; + // Dirty with disableConsumptionAssertion=true because we may be + // inside a tracking frame that already consumed these tags. + dirtyTag(tagFor(this, '_error') as UpdatableTag, true); + this._hasError = false; + dirtyTag(tagFor(this, '_hasError') as UpdatableTag, true); + return true; + } + } + + return false; + } } diff --git a/packages/@glimmer/runtime/lib/vm/update.ts b/packages/@glimmer/runtime/lib/vm/update.ts index 00afe0f8e05..e316def3763 100644 --- a/packages/@glimmer/runtime/lib/vm/update.ts +++ b/packages/@glimmer/runtime/lib/vm/update.ts @@ -237,13 +237,6 @@ export class ErrorBoundaryOpcode extends TryOpcode { override evaluate(vm: UpdatingVM) { // Snapshot current DOM boundaries before child opcodes run. - // We need the nextSibling after lastNode (not lastNode itself) because - // child opcodes may insert temporary nodes (e.g., list sync markers) - // after lastNode. Using nextSibling ensures cleanup covers those too. - // - // Bounds are always initialized here because: - // - Initial render completes (with finalize()) before UpdatingVM is created. - // - transitionToError() re-renders synchronously, repopulating bounds. if (LOCAL_DEBUG) { expect( this.bounds.firstNode(), @@ -256,6 +249,20 @@ export class ErrorBoundaryOpcode extends TryOpcode { this.lastRenderTreeDepth = vm.env.debugRenderTree?.getDepth() ?? 0; this.lastTrackingDepth = getTrackingDepth(); + // Always consume hasError so its tag is captured in the EB's tracking + // frame. Without this, after error recovery the EB's JumpIfNotModified + // combined tag would lose hasError and never detect future changes. + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + this.errorState.hasError; + + // Check @retryWith: consumes the retryWith ref's tag (keeping it in the + // EB's tracking frame) and, if the value changed while in error state, + // clears the error and returns true to trigger a re-render. + if (this.errorState.checkRetryWith()) { + this.handleException(); + return; + } + vm.try(this.children, this); } @@ -306,11 +313,6 @@ export class ErrorBoundaryOpcode extends TryOpcode { }); associateDestroyableChild(this, result.drop); } catch (error) { - if (DEBUG) { - // eslint-disable-next-line no-console - console.error('An error was caught by :', error); - } - // Restore tracking frames opened by the failed re-render attempt. restoreTrackingTo(trackingDepth); @@ -331,6 +333,11 @@ export class ErrorBoundaryOpcode extends TryOpcode { } bounds.resetPartial(); + + if (DEBUG) { + // eslint-disable-next-line no-console + console.error('An error was caught by :', error); + } this.errorState.setError(error); let retryTree = NewTreeBuilder.beginBlock(env, bounds, this.lastNextSibling); @@ -354,10 +361,6 @@ export class ErrorBoundaryOpcode extends TryOpcode { * skipping inner TryOpcode handlers that would corrupt block state. */ handleError(error: unknown) { - if (DEBUG) { - // eslint-disable-next-line no-console - console.error('An error was caught by :', error); - } // Restore tracking to the depth from evaluate(), before children ran. // _execute's catch only restores to the depth of the failing opcode, // which doesn't cover tracking frames opened by earlier opcodes @@ -377,6 +380,10 @@ export class ErrorBoundaryOpcode extends TryOpcode { destroyChildren(this); + if (DEBUG) { + // eslint-disable-next-line no-console + console.error('An error was caught by :', error); + } this.errorState.setError(error); // Clean up DOM manually rather than using bounds.reset() (via resume()), diff --git a/packages/@glimmer/validator/lib/tracking.ts b/packages/@glimmer/validator/lib/tracking.ts index f164a6034b7..67d684dc8c5 100644 --- a/packages/@glimmer/validator/lib/tracking.ts +++ b/packages/@glimmer/validator/lib/tracking.ts @@ -108,18 +108,23 @@ export function getTrackingDepth(): number { /** * Pop tracking frames (and their associated DEBUG tracking transactions) back * to the given depth. This discards stale frames left by a failed render. + * + * OPEN_TRACK_FRAMES stores the *previous* CURRENT_TRACKER when each frame was + * opened. So the entry at index N is the tracker that was active at depth N + * (the tracker that was current before depth N+1 was opened). When popping + * back to `depth`, the last popped entry is the tracker that was active at + * the target depth — that's what CURRENT_TRACKER should be restored to. */ export function restoreTrackingTo(depth: number): void { while (OPEN_TRACK_FRAMES.length > depth) { - OPEN_TRACK_FRAMES.pop(); + // Each popped entry is the CURRENT_TRACKER that was saved when + // beginTrackFrame opened the next deeper frame. The last popped + // entry is the tracker for the target depth. + CURRENT_TRACKER = OPEN_TRACK_FRAMES.pop() as Tracker | null; if (DEBUG) { unwrap(debug.endTrackingTransaction)(); } } - CURRENT_TRACKER = - OPEN_TRACK_FRAMES.length > 0 - ? (OPEN_TRACK_FRAMES[OPEN_TRACK_FRAMES.length - 1] as Tracker) - : null; } // This function is only for handling errors and resetting to a valid state