feat(error-boundary): Basic stack error recovery works

This commit also calls the specified handler asynchronously when an
error occurs.

.vscode/settings.json

@@ -97,7 +97,7 @@
       }
     }
   },
-  "inline-bookmarks.view.showVisibleFilesOnly": false,
+  "inline-bookmarks.view.showVisibleFilesOnly": true,
   "commit-message-editor.staticTemplate": [
     "feat: Short description",
     "",

bin/opcodes.json

@@ -13,11 +13,11 @@
     "Jump",
     "Return",
     "ReturnTo",
-    "PushTryFrame",
     "PopTryFrame",
     "UnwindTypeFrame"
   ],
   "syscall": [
+    "PushTryFrame",
     "Helper",
     "SetNamedVariables",
     "SetBlocks",

packages/@glimmer-workspace/integration-tests/lib/setup-harness.ts

@@ -10,53 +10,38 @@ export async function setupQunit() {
   const testing = Testing.withConfig(
     {
       id: 'smoke_tests',
-      label: 'Enable Smoke Tests',
+      label: 'Smoke Tests',
       tooltip: 'Enable Smoke Tests',
     },
     {
       id: 'ci',
-      label: 'Enable CI Mode',
-      tooltip: 'CI mode makes tests run faster by sacrificing UI responsiveness',
+      label: 'CI Mode',
+      tooltip:
+        'CI mode emits tap output and makes tests run faster by sacrificing UI responsiveness',
     },
+    {
+      id: 'enable_internals_logging',
+      label: 'Log Deep Internals',
+      tooltip: 'Logs internals that are used in the development of the trace logs',
+    },
+
     {
       id: 'enable_trace_logging',
-      label: 'Enable Trace Logging',
+      label: 'Trace Logs',
       tooltip: 'Trace logs emit information about the internal VM state',
-      value: {
-        trace: 'trace level logs (default)',
-        explain: 'also include explanations',
-      },
     },
+
     {
-      id: 'enable_tap',
-      label: 'Enable Tap',
-      value: {
-        in_ci: 'only in CI (default)',
-        never: 'never',
-        always: 'always',
-      },
+      id: 'enable_trace_explanations',
+      label: '+ Explanations',
+      tooltip: 'Also explain the trace logs',
     }
   );
 
   const runner = autoRegister();
 
   testing.begin(() => {
-    function isTapEnabled() {
-      let tap = testing.config.enable_tap;
-      let ci = testing.config.ci;
-
-      switch (tap) {
-        case 'in_ci':
-        case undefined:
-          return !!ci;
-        case 'never':
-          return false;
-        case 'always':
-          return true;
-      }
-    }
-
-    if (isTapEnabled()) {
+    if (testing.config.ci) {
       const tap = qunitLib.reporters.tap;
       tap.init(runner, { log: console.info });
     }
@@ -110,20 +95,6 @@ export async function setupQunit() {
     smokeTest: hasFlag('smoke_test'),
   };
 }
-function isTapEnabled() {
-  const tap = getSpecificFlag('enable_tap');
-  const ci = hasSpecificFlag('ci');
-
-  switch (tap) {
-    case 'in_ci':
-    case undefined:
-      return !!ci;
-    case 'never':
-      return false;
-    case 'always':
-      return true;
-  }
-}
 
 class Testing<Q extends typeof QUnit> {
   static withConfig<const C extends readonly UrlConfig[]>(...configs: C): Testing<WithConfig<C>> {
@@ -152,19 +123,15 @@ class Testing<Q extends typeof QUnit> {
 }
 
 function hasFlag(flag: string): boolean {
-  switch (flag) {
-    case 'enable_tap':
-      return hasSpecificFlag('enable_tap') || hasSpecificFlag('ci');
-    default:
-      return hasSpecificFlag(flag);
-  }
+  return hasSpecificFlag(flag);
 }
 
 function hasSpecificFlag(flag: string): boolean {
   let location = typeof window !== 'undefined' && window.location;
   return location && new RegExp(`[?&]${flag}`).test(location.search);
 }
 
+// eslint-disable-next-line unused-imports/no-unused-vars
 function getSpecificFlag(flag: string): string | undefined {
   let location = typeof window !== 'undefined' && window.location;
   if (!location) {
@@ -193,5 +160,10 @@ function withConfig<const C extends readonly UrlConfig[]>(...configs: C): Expand
     QUnit.config.urlConfig.push(config);
   }
 
+  const index = QUnit.config.urlConfig.findIndex((c) => c.id === 'noglobals');
+  if (index !== -1) {
+    QUnit.config.urlConfig.splice(index, 1);
+  }
+
   return QUnit as any;
 }

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

@@ -35,17 +35,14 @@ export class ErrorRecoverySuite extends RenderTest {
       }
     }
 
-    this.render('{{#-try}}message: [{{this.woops.woops}}]{{/-try}}', {
+    this.render('{{#-try this.handler}}message: [{{this.woops.woops}}]{{/-try}}', {
       woops: new Woops(),
+      handler: (_err: unknown, _retry: () => void) => {
+        actions.record('error handled');
+      },
     });
 
-    actions.expect(['get woops']);
-    this.#assertRenderError('woops');
-
-    // this.rerender();
-
-    // actions.expect(['get woops']);
-    // this.assertRenderError('woops');
+    actions.expect(['get woops', 'error handled']);
   }
 
   // @test
@@ -133,30 +130,6 @@ export class ErrorRecoverySuite extends RenderTest {
   // '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 {

packages/@glimmer/compiler/lib/passes/2-encoding/content.ts

@@ -208,12 +208,14 @@ export class ContentEncoder {
   HandleError({ handler, block, inverse }: mir.HandleError): WireFormat.Statements.HandleError {
     return [
       SexpOpcodes.HandleError,
-      EXPR.Literal(
-        new ASTv2.LiteralExpression({
-          value: null,
-          loc: handler?.loc ?? SourceSpan.synthetic('null'),
-        })
-      ),
+      handler
+        ? EXPR.expr(handler)
+        : EXPR.Literal(
+            new ASTv2.LiteralExpression({
+              value: null,
+              loc: SourceSpan.synthetic('null'),
+            })
+          ),
       CONTENT.NamedBlock(block)[1],
       inverse ? CONTENT.NamedBlock(inverse)[1] : null,
     ];

packages/@glimmer/compiler/lib/wire-format-debug.ts

@@ -279,7 +279,9 @@ export default class WireFormatDebugger {
     }
   }
 
-  private formatElementParams(opcodes: Nullable<WireFormat.ElementParameter[]>): Nullable<unknown[]> {
+  private formatElementParams(
+    opcodes: Nullable<WireFormat.ElementParameter[]>
+  ): Nullable<unknown[]> {
     if (opcodes === null) return null;
     return opcodes.map((o) => this.formatOpcode(o));
   }

packages/@glimmer/debug/lib/debug.ts

@@ -237,8 +237,12 @@ export function debug(c: DebugConstants, op: RuntimeOp): [string, Dict<DebugOper
           case 'handle':
             out[operand.name] = { type: 'constant', value: c.getValue(actualOperand) };
             break;
+          case 'imm/pc':
+            out[operand.name] = { type: 'instruction', value: actualOperand };
+            break;
           case 'const/str':
             out[operand.name] = { type: 'string', value: c.getValue<string>(actualOperand) };
+            break;
           case 'const/str?':
             out[operand.name] = {
               type: 'string',

packages/@glimmer/debug/lib/generated/op-list.ts

@@ -11,7 +11,6 @@ export const DebugOpList = [
   'Jump',
   'Return',
   'ReturnTo',
-  'PushTryFrame',
   'PopTryFrame',
   'UnwindTypeFrame',
   null,
@@ -20,6 +19,8 @@ export const DebugOpList = [
   null,
   null,
   null,
+  null,
+  'PushTryFrame',
   'Helper',
   'SetNamedVariables',
   'SetBlocks',
@@ -123,7 +124,6 @@ export type DebugOpList = [
   'Jump',
   'Return',
   'ReturnTo',
-  'PushTryFrame',
   'PopTryFrame',
   'UnwindTypeFrame',
   null,
@@ -132,6 +132,8 @@ export type DebugOpList = [
   null,
   null,
   null,
+  null,
+  'PushTryFrame',
   'Helper',
   'SetNamedVariables',
   'SetBlocks',

packages/@glimmer/debug/lib/opcode-metadata.ts

@@ -23,7 +23,6 @@ const METADATA = MetadataBuilder.build(({ add, stack }) =>
     .add(`Jump as goto`, ['to:imm/u32'])
     .add(`Return as ret`)
     .add(`ReturnTo as setra`, ['offset:imm/pc'])
-    .add(`PushTryFrame as try`, ['catch:imm/pc'])
     .add(`PopTryFrame as finally`)
     .add(`UnwindTypeFrame as unwind`)
     .add(RESERVED)
@@ -32,7 +31,13 @@ const METADATA = MetadataBuilder.build(({ add, stack }) =>
     .add(RESERVED)
     .add(RESERVED)
     .add(RESERVED)
+    .add(RESERVED)
 
+    .add(
+      `PushTryFrame as try`,
+      ['catch:imm/pc'],
+      stack.params(['handler:reference/any']).returns([])
+    )
     .add(`Helper as ncall`, [`helper:handle`], stack.params(['args:args']).returns([]))
     .add(`SetNamedVariables as vsargs`, [`register:register`])
     .add(`SetBlocks as vbblocks`, [`register:register`])

packages/@glimmer/debug/lib/stack-check.ts

@@ -132,16 +132,16 @@ class InstanceofChecker<T> implements Checker<T> {
   }
 }
 
-class OptionChecker<T> implements Checker<Nullable<T>> {
-  declare type: Nullable<T>;
+class MaybeChecker<T, Empty extends null | undefined> implements Checker<T | Empty> {
+  declare type: T | Empty;
 
   constructor(
     private checker: Checker<T>,
-    private emptyValue: null | undefined
+    private emptyValue: Empty[]
   ) {}
 
-  validate(value: unknown): value is Nullable<T> {
-    if (value === this.emptyValue) return true;
+  validate(value: unknown): value is T | Empty {
+    if ((this.emptyValue as unknown[]).includes(value)) return true;
     return this.checker.validate(value);
   }
 
@@ -150,21 +150,6 @@ class OptionChecker<T> implements Checker<Nullable<T>> {
   }
 }
 
-class MaybeChecker<T> implements Checker<Maybe<T>> {
-  declare type: Maybe<T>;
-
-  constructor(private checker: Checker<T>) {}
-
-  validate(value: unknown): value is Maybe<T> {
-    if (value === null || value === undefined) return true;
-    return this.checker.validate(value);
-  }
-
-  expected(): string {
-    return `${this.checker.expected()} or null or undefined`;
-  }
-}
-
 class OrChecker<T, U> implements Checker<T | U> {
   declare type: T | U;
 
@@ -312,12 +297,16 @@ export function CheckInstanceof<T>(Class: Constructor<T>): Checker<T> {
   return new InstanceofChecker<T>(Class);
 }
 
-export function CheckOption<T>(checker: Checker<T>): Checker<Nullable<T>> {
-  return new OptionChecker(checker, null);
+export function CheckNullable<T>(checker: Checker<T>): Checker<Nullable<T>> {
+  return new MaybeChecker(checker, [null]);
+}
+
+export function CheckOptional<T>(checker: Checker<T>): Checker<Maybe<T>> {
+  return new MaybeChecker(checker, [undefined]);
 }
 
 export function CheckMaybe<T>(checker: Checker<T>): Checker<Maybe<T>> {
-  return new MaybeChecker(checker);
+  return new MaybeChecker(checker, [null, undefined]);
 }
 
 export function CheckInterface<