angular · atscott · Jul 14, 2024
diff --git a/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts b/packages/core/src/change_detection/scheduling/ng_zone_scheduling.ts
@@ -18,7 +18,6 @@ import {
   makeEnvironmentProviders,
   StaticProvider,
 } from '../../di';
-import {ErrorHandler, INTERNAL_APPLICATION_ERROR_HANDLER} from '../../error_handler';
 import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {PendingTasks} from '../../pending_tasks';
 import {performanceMarkFeature} from '../../util/performance';
@@ -113,19 +112,12 @@ export function internalProvideZoneChangeDetection({
         };
       },
     },
-    {provide: INTERNAL_APPLICATION_ERROR_HANDLER, useFactory: ngZoneApplicationErrorHandlerFactory},
     // Always disable scheduler whenever explicitly disabled, even if another place called
     // `provideZoneChangeDetection` without the 'ignore' option.
     ignoreChangesOutsideZone === true ? {provide: ZONELESS_SCHEDULER_DISABLED, useValue: true} : [],
   ];
 }
 
-export function ngZoneApplicationErrorHandlerFactory() {
-  const zone = inject(NgZone);
-  const userErrorHandler = inject(ErrorHandler);
-  return (e: unknown) => zone.runOutsideAngular(() => userErrorHandler.handleError(e));
-}
-
 /**
  * Provides `NgZone`-based change detection for the application bootstrapped using
  * `bootstrapApplication`.

diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts
@@ -11,6 +11,7 @@ export {
   detectChangesInViewIfRequired as ɵdetectChangesInViewIfRequired,
   whenStable as ɵwhenStable,
 } from './application/application_ref';
+export {INTERNAL_APPLICATION_ERROR_HANDLER as ɵINTERNAL_APPLICATION_ERROR_HANDLER} from './error_handler';
 export {
   IMAGE_CONFIG as ɵIMAGE_CONFIG,
   IMAGE_CONFIG_DEFAULTS as ɵIMAGE_CONFIG_DEFAULTS,

diff --git a/packages/core/src/error_handler.ts b/packages/core/src/error_handler.ts
@@ -8,6 +8,7 @@
 
 import {inject, InjectionToken} from './di';
 import {getOriginalError} from './util/errors';
+import {NgZone} from './zone';
 
 /**
  * Provides a hook for centralized exception handling.
@@ -71,8 +72,9 @@ export const INTERNAL_APPLICATION_ERROR_HANDLER = new InjectionToken<(e: any) =>
   {
     providedIn: 'root',
     factory: () => {
+      const zone = inject(NgZone);
       const userErrorHandler = inject(ErrorHandler);
-      return userErrorHandler.handleError.bind(this);
+      return (e: unknown) => zone.runOutsideAngular(() => userErrorHandler.handleError(e));
     },
   },
 );
diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json
@@ -1256,9 +1256,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/animations/bundle.golden_symbols.json b/packages/core/test/bundling/animations/bundle.golden_symbols.json
@@ -1319,9 +1319,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noSideEffects"
   },

diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
@@ -1061,9 +1061,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noSideEffects"
   },

diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json
@@ -2282,9 +2282,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json
@@ -1583,9 +1583,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noSideEffects"
   },

diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json
@@ -1550,9 +1550,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noSideEffects"
   },

diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json
@@ -836,9 +836,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noop"
   },

diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json
@@ -1178,9 +1178,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json
@@ -1778,9 +1778,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noMatch"
   },

diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
@@ -941,9 +941,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "nonNull"
   },

diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json
@@ -1277,9 +1277,6 @@
   {
     "name": "ngOnChangesSetInput"
   },
-  {
-    "name": "ngZoneApplicationErrorHandlerFactory"
-  },
   {
     "name": "noSideEffects"
   },

diff --git a/packages/core/test/component_fixture_spec.ts b/packages/core/test/component_fixture_spec.ts
@@ -7,10 +7,14 @@
  */
 
 import {
+  ApplicationRef,
   Component,
+  EnvironmentInjector,
   ErrorHandler,
   Injectable,
   Input,
+  NgZone,
+  createComponent,
   provideExperimentalZonelessChangeDetection,
   signal,
 } from '@angular/core';
@@ -333,6 +337,53 @@ describe('ComponentFixture', () => {
     });
   }));
 
+  it('throws errors that happen during detectChanges', () => {
+    @Component({
+      template: '',
+      standalone: true,
+    })
+    class App {
+      ngOnInit() {
+        throw new Error();
+      }
+    }
+
+    const fixture = TestBed.createComponent(App);
+    expect(() => fixture.detectChanges()).toThrow();
+  });
+
+  // note: this test only verifies existing behavior was not broken by a change to the zoneless fixture.
+  // We probably do want the whenStable promise to be rejected. The current zone-based fixture is bad
+  // and confusing for two reason:
+  //  1. with autoDetect, errors in the fixture _cannot be handled_ with whenStable because
+  //  they're just thrown inside the rxjs subcription (and then goes to setTimeout(() => throw e))
+  //  2. errors from other views attached to ApplicationRef just go to the ErrorHandler, which by default
+  //  only logs to console, allowing the test to pass
+  it('resolves whenStable promise when errors happen during appRef.tick', async () => {
+    @Component({
+      template: '',
+      standalone: true,
+    })
+    class ThrowingThing {
+      ngOnInit() {
+        throw new Error();
+      }
+    }
+    @Component({
+      template: '',
+      standalone: true,
+    })
+    class Blank {}
+
+    const fixture = TestBed.createComponent(Blank);
+    const throwingThing = createComponent(ThrowingThing, {
+      environmentInjector: TestBed.inject(EnvironmentInjector),
+    });
+
+    TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
+    await expectAsync(fixture.whenStable()).toBeResolved();
+  });
+
   describe('defer', () => {
     it('should return all defer blocks in the component', async () => {
       @Component({
@@ -388,17 +439,35 @@ describe('ComponentFixture', () => {
       componentFixture.detectChanges();
       expect(componentFixture.nativeElement).toHaveText('MyIf(More)');
     }));
+
+    it('throws errors that happen during detectChanges', () => {
+      @Component({
+        template: '',
+        standalone: true,
+      })
+      class App {
+        ngOnInit() {
+          throw new Error();
+        }
+      }
+
+      const fixture = TestBed.createComponent(App);
+      expect(() => fixture.detectChanges()).toThrow();
+    });
   });
 });
 
 describe('ComponentFixture with zoneless', () => {
-  it('will not refresh CheckAlways views when detectChanges is called if not marked dirty', () => {
+  beforeEach(() => {
     TestBed.configureTestingModule({
       providers: [
         provideExperimentalZonelessChangeDetection(),
         {provide: ErrorHandler, useValue: {handleError: () => {}}},
       ],
     });
+  });
+
+  it('will not refresh CheckAlways views when detectChanges is called if not marked dirty', () => {
     @Component({standalone: true, template: '{{signalThing()}}|{{regularThing}}'})
     class CheckAlwaysCmp {
       regularThing = 'initial';
@@ -410,10 +479,41 @@ describe('ComponentFixture with zoneless', () => {
     fixture.detectChanges();
     expect(fixture.nativeElement.innerText).toEqual('initial|initial');
     fixture.componentInstance.regularThing = 'new';
-    fixture.detectChanges();
+    // Expression changed after checked
+    expect(() => fixture.detectChanges()).toThrow();
     expect(fixture.nativeElement.innerText).toEqual('initial|initial');
     fixture.componentInstance.signalThing.set('new');
     fixture.detectChanges();
     expect(fixture.nativeElement.innerText).toEqual('new|new');
   });
+
+  it('throws errors that happen during detectChanges', () => {
+    @Component({
+      template: '',
+      standalone: true,
+    })
+    class App {
+      ngOnInit() {
+        throw new Error();
+      }
+    }
+
+    const fixture = TestBed.createComponent(App);
+    expect(() => fixture.detectChanges()).toThrow();
+  });
+
+  it('rejects whenStable promise when errors happen during detectChanges', async () => {
+    @Component({
+      template: '',
+      standalone: true,
+    })
+    class App {
+      ngOnInit() {
+        throw new Error();
+      }
+    }
+
+    const fixture = TestBed.createComponent(App);
+    await expectAsync(fixture.whenStable()).toBeRejected();
+  });
 });
diff --git a/packages/core/testing/src/application_error_handler.ts b/packages/core/testing/src/application_error_handler.ts
@@ -0,0 +1,52 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {
+  ErrorHandler,
+  inject,
+  NgZone,
+  Injectable,
+  ɵZONELESS_ENABLED as ZONELESS_ENABLED,
+} from '@angular/core';
+
+@Injectable()
+export class TestBedApplicationErrorHandler {
+  private readonly zone = inject(NgZone);
+  private readonly userErrorHandler = inject(ErrorHandler);
+  private readonly zoneless = inject(ZONELESS_ENABLED);
+  readonly whenStableRejectFunctions: Set<(e: unknown) => void> = new Set();
+
+  handleError(e: unknown) {
+    // TODO(atscott): Investigate if we can align the behaviors of zone and zoneless
+    if (this.zoneless) {
+      this.zonelessHandleError(e);
+    } else {
+      this.zone.runOutsideAngular(() => this.userErrorHandler.handleError(e));
+    }
+  }
+
+  private zonelessHandleError(e: unknown) {
+    try {
+      this.zone.runOutsideAngular(() => this.userErrorHandler.handleError(e));
+    } catch (userError: unknown) {
+      e = userError;
+    }
+
+    // Instead of throwing the error when there are outstanding `fixture.whenStable` promises,
+    // reject those promises with the error. This allows developers to write
+    // expectAsync(fix.whenStable()).toBeRejected();
+    if (this.whenStableRejectFunctions.size > 0) {
+      for (const fn of this.whenStableRejectFunctions.values()) {
+        fn(e);
+      }
+      this.whenStableRejectFunctions.clear();
+    } else {
+      throw e;
+    }
+  }
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -1256,9 +1256,6 @@ @@
       {
         "name": "ngOnChangesSetInput"
       },
-      {
-        "name": "ngZoneApplicationErrorHandlerFactory"
-      },
       {
         "name": "nonNull"
       },
@@ Expand Down @@