Starting work on error recovery

Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>

.editorconfig

@@ -13,6 +13,9 @@ insert_final_newline = true
 indent_style = space
 indent_size = 2
 
+[*.ts]
+max_line_length = 100
+
 [*.json]
 max_line_length = 80
 

.vscode/extensions.json

@@ -0,0 +1,3 @@
+{
+  "recommendations": ["rohit-gohri.format-code-action"]
+}

.vscode/settings.json

@@ -13,7 +13,6 @@
   },
   "editor.formatOnSave": true,
   "eslint.enable": true,
-  "eslint.packageManager": "npm",
   "eslint.codeAction.showDocumentation": {
     "enable": true
   },
@@ -34,10 +33,6 @@
   },
   "files.insertFinalNewline": true,
   "files.trimTrailingWhitespace": true,
-  "files.watcherExclude": {
-    "**/.git/objects/**": true,
-    "**/.git/subtree-cache/**": true
-  },
   "javascript.updateImportsOnFileMove.enabled": "always",
   "typescript.updateImportsOnFileMove.enabled": "always",
 
@@ -46,10 +41,13 @@
 
   "typescript.preferences.importModuleSpecifierEnding": "minimal",
   "typescript.preferences.useAliasesForRenames": false,
-
-  "typescript.tsc.autoDetect": "on",
   "typescript.tsdk": "node_modules/typescript/lib",
   "typescript.tsserver.experimental.enableProjectDiagnostics": false,
   "typescript.workspaceSymbols.scope": "currentProject",
-  "eslint.problems.shortenToSingleLine": true
+  "eslint.problems.shortenToSingleLine": true,
+  "inline-bookmarks.expert.custom.words.mapping": {
+    "red": ["@fixme(?=\\s|$)", "@audit(?=\\s|$)"]
+  },
+  "inline-bookmarks.view.exclude.gitIgnore": true,
+  "inline-bookmarks.enable": true
 }

package.json

@@ -133,6 +133,6 @@
   },
   "volta": {
     "node": "20.1.0",
-    "pnpm": "8.5.0"
+    "pnpm": "8.6.12"
   }
 }

packages/@glimmer-workspace/integration-tests/lib/suites/error-recovery.ts

@@ -0,0 +1,158 @@
+import { tracked } from '../..';
+import { RenderTest } from '../render-test';
+import { test } from '../test-decorator';
+
+export class ErrorRecoverySuite extends RenderTest {
+  static suiteName = 'ErrorRecovery';
+
+  afterEach() {}
+
+  @test
+  'by default, errors are rethrown and DOM is cleaned up'() {
+    const actions = new Actions();
+
+    class Woops {
+      get woops() {
+        actions.record('get woops');
+        throw Error('woops');
+      }
+    }
+
+    this.render('message: [{{this.woops.woops}}]', {
+      woops: new Woops(),
+    });
+
+    actions.expect(['get woops']);
+    this.#assertRenderError('woops');
+
+    // this.rerender();
+
+    // actions.expect(['get woops']);
+    // this.assertRenderError('woops');
+  }
+
+  @test
+  'errors are rethrown during initial render and DOM is cleaned up'() {
+    const actions = new Actions();
+
+    class Counter {
+      @tracked count = 0;
+
+      get message() {
+        actions.record(`get message`);
+        if (this.count === 0) {
+          throw Error('woops');
+        } else {
+          return String(this.count);
+        }
+      }
+    }
+
+    let counter = new Counter();
+
+    this.render('message: [{{this.counter.message}}]', {
+      counter,
+    });
+    this.#assertRenderError('woops');
+
+    actions.expect(['get message']);
+
+    // counter.count++;
+
+    // this.rerender();
+
+    // this.assertHTML('message: [1]');
+    // actions.expect(['get message']);
+  }
+
+  @test
+  'errors are rethrown during rerender and DOM is cleaned up'() {
+    const actions = new Actions();
+
+    class Counter {
+      @tracked count = 0;
+
+      get message() {
+        actions.record(`get message`);
+        if (this.count === 1) {
+          throw Error('woops');
+        } else {
+          return String(this.count);
+        }
+      }
+    }
+
+    let counter = new Counter();
+
+    this.render('message: [{{this.counter.message}}]', {
+      counter,
+    });
+
+    this.assertHTML('message: [0]');
+    actions.expect(['get message']);
+
+    // counter.count++;
+
+    // this.rerender();
+
+    // this.#assertRenderError('woops');
+
+    // actions.expect(['get message']);
+
+    // counter.count++;
+
+    // this.rerender();
+
+    // this.assertHTML('message: [2]');
+    // actions.expect(['get message']);
+  }
+
+  // @test
+  // 'helper destructors run on rollback'(assert: Assert) {
+  //   assert.false(true, 'TODO');
+  // }
+
+  // @test
+  // 'the tracking frame is cleared when an error was thrown'(assert: Assert) {
+  //   assert.false(true, 'TODO');
+  // }
+
+  #assertRenderError(message: string) {
+    if (this.renderResult === null) {
+      this.assert.ok(
+        null,
+        `expecting a RenderResult but didn't have one. Did you call this.render()`
+      );
+      return;
+    }
+
+    if (this.renderResult.error === null) {
+      this.assert.ok(null, `expecting a render error, but none was found`);
+      return;
+    }
+
+    let error = this.renderResult.error.error;
+
+    if (error instanceof Error) {
+      this.assertHTML('<!---->');
+      this.assert.equal(error.message, message);
+    } else {
+      this.assert.ok(error, `expecting the render error to be a Error object`);
+    }
+  }
+}
+
+class Actions {
+  #actions: string[] = [];
+
+  record(action: string) {
+    this.#actions.push(action);
+  }
+
+  expect(expected: string[]) {
+    let actual = this.#actions;
+    this.#actions = [];
+
+    QUnit.assert.deepEqual(actual, expected);
+  }
+}

packages/@glimmer-workspace/integration-tests/test/jit-suites-test.ts

@@ -14,8 +14,10 @@ import {
   WithDynamicVarsSuite,
   YieldSuite,
 } from '..';
+import { ErrorRecoverySuite } from '../lib/suites/error-recovery';
 
 jitSuite(DebuggerSuite);
+jitSuite(ErrorRecoverySuite);
 jitSuite(EachSuite);
 jitSuite(InElementSuite);
 

packages/@glimmer/interfaces/lib/dom/attributes.d.ts

@@ -18,6 +18,7 @@ export interface LiveBlock extends Bounds {
   didAppendNode(node: SimpleNode): void;
   didAppendBounds(bounds: Bounds): void;
   finalize(stack: ElementBuilder): void;
+  clear?(stack: ElementBuilder): void;
 }
 
 export interface SimpleLiveBlock extends LiveBlock {
@@ -26,7 +27,7 @@ export interface SimpleLiveBlock extends LiveBlock {
   lastNode(): SimpleNode;
 }
 
-export type RemoteLiveBlock = SimpleLiveBlock
+export type RemoteLiveBlock = SimpleLiveBlock;
 
 export interface UpdatableBlock extends SimpleLiveBlock {
   reset(env: Environment): Nullable<SimpleNode>;
@@ -94,6 +95,8 @@ export interface ElementBuilder extends Cursor, DOMStack, TreeOperations {
   pushBlockList(list: Bounds[]): LiveBlock;
   popBlock(): LiveBlock;
 
+  unwind(): LiveBlock;
+
   didAppendBounds(bounds: Bounds): void;
 }
 

packages/@glimmer/interfaces/lib/runtime/render.d.ts

@@ -1,6 +1,6 @@
 import type { SimpleElement, SimpleNode } from '@simple-dom/interface';
 
-import type { RichIteratorResult } from '../core';
+import type { Nullable, RichIteratorResult } from '../core';
 import type { Bounds } from '../dom/bounds';
 import type { Environment } from './environment';
 
@@ -11,6 +11,7 @@ export interface ExceptionHandler {
 export interface RenderResult extends Bounds, ExceptionHandler {
   readonly env: Environment;
   readonly drop: object;
+  readonly error: Nullable<{ error: unknown }>;
 
   rerender(options?: { alwaysRevalidate: false }): void;
 

packages/@glimmer/runtime/lib/bounds.ts

@@ -69,15 +69,23 @@ export function clear(bounds: Bounds): Nullable<SimpleNode> {
   let first = bounds.firstNode();
   let last = bounds.lastNode();
 
-  let current: SimpleNode = first;
+  return clearRange(parent, first, last.nextSibling);
+}
+
+export function clearRange(
+  parent: SimpleElement,
+  from: SimpleNode,
+  nextSibling: Nullable<SimpleNode>
+): Nullable<SimpleNode> {
+  let current: SimpleNode = from;
 
   // eslint-disable-next-line no-constant-condition
   while (true) {
     let next = current.nextSibling;
 
     parent.removeChild(current);
 
-    if (current === last) {
+    if (next === nextSibling) {
       return next;
     }
 

packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts

@@ -17,7 +17,7 @@ import type {
   Owner,
   UpdatingOpcode,
   UpdatingVM,
-} from "@glimmer/interfaces";
+} from '@glimmer/interfaces';
 import { createComputeRef, isConstRef, type Reference, valueForRef } from '@glimmer/reference';
 import { assign, debugToString, expect, isObject } from '@glimmer/util';
 import {
@@ -96,6 +96,7 @@ APPEND_OPCODES.add(Op.CloseElement, (vm) => {
 
   if (modifiers) {
     modifiers.forEach((modifier) => {
+      // @audit unwind
       vm.env.scheduleInstallModifier(modifier);
       let { manager, state } = modifier;
       let d = manager.getDestroyable(state);

packages/@glimmer/runtime/lib/compiled/opcodes/vm.ts

@@ -307,15 +307,15 @@ export class BeginTrackFrameOpcode implements UpdatingOpcode {
   constructor(private debugLabel?: string) {}
 
   evaluate() {
-    beginTrackFrame(this.debugLabel);
+    beginTrackFrame(this.debugLabel); // @audit
   }
 }
 
 export class EndTrackFrameOpcode implements UpdatingOpcode {
   constructor(private target: JumpIfNotModifiedOpcode) {}
 
   evaluate() {
-    let tag = endTrackFrame();
+    let tag = endTrackFrame(); // @audit
     this.target.didModify(tag);
   }
 }

packages/@glimmer/runtime/lib/vm/append.ts

@@ -386,14 +386,14 @@ export class VM implements PublicVM, InternalVM {
     opcodes.push(new BeginTrackFrameOpcode(name));
     this[STACKS].cache.push(guard);
 
-    beginTrackFrame(name);
+    beginTrackFrame(name); // @audit
   }
 
   commitCacheGroup() {
     let opcodes = this.updating();
     let guard = expect(this[STACKS].cache.pop(), 'VM BUG: Expected a cache group');
 
-    let tag = endTrackFrame();
+    let tag = endTrackFrame(); // @audit
     opcodes.push(new EndTrackFrameOpcode(guard));
 
     guard.finalize(tag, opcodes.length);
@@ -604,10 +604,7 @@ export class VM implements PublicVM, InternalVM {
     let { env, elementStack } = this;
     let opcode = this[INNER_VM].nextStatement();
     let result: RichIteratorResult<null, RenderResult>;
-    if (opcode !== null) {
-      this[INNER_VM].evaluateOuter(opcode, this);
-      result = { done: false, value: null };
-    } else {
+    if (opcode === null) {
       // Unload the stack
       this.stack.reset();
 
@@ -617,10 +614,23 @@ export class VM implements PublicVM, InternalVM {
           env,
           this.popUpdating(),
           elementStack.popBlock(),
-          this.destructor
+          this.destructor,
+          null
         ),
       };
+    } else {
+      try {
+        this[INNER_VM].evaluateOuter(opcode, this);
+        result = { done: false, value: null };
+      } catch (error) {
+        // @todo run cleanup
+        result = {
+          done: true,
+          value: new RenderResultImpl(env, [], elementStack.unwind(), {}, { error }),
+        };
+      }
     }
+
     return result;
   }