fix(core): rethrow errors during ApplicationRef.tick in TestBed (#57200)

atscott · AndrewKushnir · commit 468d3fb9b1c3 · 2024-08-12T11:23:46.000-07:00
Errors during change detection from `ApplicationRef.tick` are only reported to the `ErrorHandler`. By default, this only logs the error to console. As a result, these errors can be missed/ignored and allow tests to pass when they should not. This change ensures that the errors are surfaced. Note that this is already the behavior when zoneless is enabled. BREAKING CHANGE: Errors that are thrown during `ApplicationRef.tick` will now be rethrown when using `TestBed`. These errors should be resolved by ensuring the test environment is set up correctly to complete change detection successfully. There are two alternatives to catch the errors: * Instead of waiting for automatic change detection to happen, trigger it synchronously and expect the error. For example, a jasmine test could write `expect(() => TestBed.inject(ApplicationRef).tick()).toThrow()` * `TestBed` will reject any outstanding `ComponentFixture.whenStable` promises. A jasmine test, for example, could write `expectAsync(fixture.whenStable()).toBeRejected()`. As a last resort, you can configure errors to _not_ be rethrown by setting `rethrowApplicationErrors` to `false` in `TestBed.configureTestingModule`. PR Close #57200
diff --git a/goldens/public-api/core/testing/index.api.md b/goldens/public-api/core/testing/index.api.md
@@ -216,6 +216,7 @@ export interface TestModuleMetadata {
     imports?: any[];
     // (undocumented)
     providers?: any[];
+    rethrowApplicationErrors?: boolean;
     // (undocumented)
     schemas?: Array<SchemaMetadata | any[]>;
     // (undocumented)
diff --git a/packages/core/test/component_fixture_spec.ts b/packages/core/test/component_fixture_spec.ts
@@ -367,32 +367,25 @@ describe('ComponentFixture', () => {
     })
     class Blank {}
 
-    // 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 () => {
+    it('rejects whenStable promise when errors happen during appRef.tick', async () => {
       const fixture = TestBed.createComponent(Blank);
       const throwingThing = createComponent(ThrowingThing, {
         environmentInjector: TestBed.inject(EnvironmentInjector),
       });
 
       TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
-      await expectAsync(fixture.whenStable()).toBeResolved();
+      await expectAsync(fixture.whenStable()).toBeRejected();
     });
 
-    it('can opt-in to rethrowing application errors and rejecting whenStable promises', async () => {
-      TestBed.configureTestingModule({_rethrowApplicationTickErrors: true} as any);
+    it('can opt-out of rethrowing application errors and rejecting whenStable promises', async () => {
+      TestBed.configureTestingModule({rethrowApplicationErrors: false});
       const fixture = TestBed.createComponent(Blank);
       const throwingThing = createComponent(ThrowingThing, {
         environmentInjector: TestBed.inject(EnvironmentInjector),
       });
 
       TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
-      await expectAsync(fixture.whenStable()).toBeRejected();
+      await expectAsync(fixture.whenStable()).toBeResolved();
     });
   });
 
diff --git a/packages/core/testing/src/application_error_handler.ts b/packages/core/testing/src/application_error_handler.ts
@@ -6,9 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ErrorHandler, inject, NgZone, Injectable, InjectionToken} from '@angular/core';
+import {ErrorHandler, inject, NgZone, Injectable} from '@angular/core';
 
-export const RETHROW_APPLICATION_ERRORS = new InjectionToken<boolean>('rethrow application errors');
+export const RETHROW_APPLICATION_ERRORS_DEFAULT = true;
 
 @Injectable()
 export class TestBedApplicationErrorHandler {
diff --git a/packages/core/testing/src/test_bed_common.ts b/packages/core/testing/src/test_bed_common.ts
@@ -68,14 +68,25 @@ export interface TestModuleMetadata {
    */
   errorOnUnknownProperties?: boolean;
 
+  /**
+   * Whether errors that happen during application change detection should be rethrown.
+   *
+   * When `true`, errors that are caught during application change detection will
+   * be reported to the `ErrorHandler` and rethrown to prevent them from going
+   * unnoticed in tests.
+   *
+   * When `false`, errors are only forwarded to the `ErrorHandler`, which by default
+   * simply logs them to the console.
+   *
+   * Defaults to `true`.
+   */
+  rethrowApplicationErrors?: boolean;
+
   /**
    * Whether defer blocks should behave with manual triggering or play through normally.
    * Defaults to `manual`.
    */
   deferBlockBehavior?: DeferBlockBehavior;
-
-  /** @internal */
-  _rethrowApplicationTickErrors?: boolean;
 }
 
 /**
diff --git a/packages/core/testing/src/test_bed_compiler.ts b/packages/core/testing/src/test_bed_compiler.ts
@@ -22,7 +22,6 @@ import {
   LOCALE_ID,
   ModuleWithComponentFactories,
   ModuleWithProviders,
-  ɵZONELESS_ENABLED as ZONELESS_ENABLED,
   NgModule,
   NgModuleFactory,
   Pipe,
@@ -52,6 +51,7 @@ import {
   ɵNG_INJ_DEF as NG_INJ_DEF,
   ɵNG_MOD_DEF as NG_MOD_DEF,
   ɵNG_PIPE_DEF as NG_PIPE_DEF,
+  ɵZONELESS_ENABLED as ZONELESS_ENABLED,
   ɵNgModuleFactory as R3NgModuleFactory,
   ɵNgModuleTransitiveScopes as NgModuleTransitiveScopes,
   ɵNgModuleType as NgModuleType,
@@ -80,7 +80,7 @@ import {
 } from './resolvers';
 import {DEFER_BLOCK_DEFAULT_BEHAVIOR, TestModuleMetadata} from './test_bed_common';
 import {
-  RETHROW_APPLICATION_ERRORS,
+  RETHROW_APPLICATION_ERRORS_DEFAULT,
   TestBedApplicationErrorHandler,
 } from './application_error_handler';
 
@@ -190,6 +190,7 @@ export class TestBedCompiler {
   private testModuleRef: NgModuleRef<any> | null = null;
 
   private deferBlockBehavior = DEFER_BLOCK_DEFAULT_BEHAVIOR;
+  private rethrowApplicationTickErrors = RETHROW_APPLICATION_ERRORS_DEFAULT;
 
   constructor(
     private platform: PlatformRef,
@@ -226,16 +227,14 @@ export class TestBedCompiler {
     if (moduleDef.providers !== undefined) {
       this.providers.push(...moduleDef.providers);
     }
-    this.providers.push({
-      provide: RETHROW_APPLICATION_ERRORS,
-      useValue: moduleDef._rethrowApplicationTickErrors ?? false,
-    });
 
     if (moduleDef.schemas !== undefined) {
       this.schemas.push(...moduleDef.schemas);
     }
 
     this.deferBlockBehavior = moduleDef.deferBlockBehavior ?? DEFER_BLOCK_DEFAULT_BEHAVIOR;
+    this.rethrowApplicationTickErrors =
+      moduleDef.rethrowApplicationErrors ?? RETHROW_APPLICATION_ERRORS_DEFAULT;
   }
 
   overrideModule(ngModule: Type<any>, override: MetadataOverride<NgModule>): void {
@@ -944,29 +943,28 @@ export class TestBedCompiler {
         ...this.rootProviderOverrides,
         internalProvideZoneChangeDetection({}),
         TestBedApplicationErrorHandler,
-        {
-          provide: INTERNAL_APPLICATION_ERROR_HANDLER,
-          useFactory: () => {
-            if (inject(ZONELESS_ENABLED) || inject(RETHROW_APPLICATION_ERRORS, {optional: true})) {
-              const handler = inject(TestBedApplicationErrorHandler);
-              return (e: unknown) => {
-                handler.handleError(e);
-              };
-            } else {
-              const userErrorHandler = inject(ErrorHandler);
-              const ngZone = inject(NgZone);
-              return (e: unknown) =>
-                ngZone.runOutsideAngular(() => userErrorHandler.handleError(e));
-            }
-          },
-        },
         {provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl},
       ],
     });
 
     const providers = [
       {provide: Compiler, useFactory: () => new R3TestCompiler(this)},
       {provide: DEFER_BLOCK_CONFIG, useValue: {behavior: this.deferBlockBehavior}},
+      {
+        provide: INTERNAL_APPLICATION_ERROR_HANDLER,
+        useFactory: () => {
+          if (this.rethrowApplicationTickErrors) {
+            const handler = inject(TestBedApplicationErrorHandler);
+            return (e: unknown) => {
+              handler.handleError(e);
+            };
+          } else {
+            const userErrorHandler = inject(ErrorHandler);
+            const ngZone = inject(NgZone);
+            return (e: unknown) => ngZone.runOutsideAngular(() => userErrorHandler.handleError(e));
+          }
+        },
+      },
       ...this.providers,
       ...this.providerOverrides,
     ];
