Skip to content

Commit 468d3fb

Browse files
atscottAndrewKushnir
authored andcommitted
fix(core): rethrow errors during ApplicationRef.tick in TestBed (angular#57200)
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 angular#57200
1 parent 24cd1c8 commit 468d3fb

File tree

5 files changed

+42
-39
lines changed

5 files changed

+42
-39
lines changed

goldens/public-api/core/testing/index.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ export interface TestModuleMetadata {
216216
imports?: any[];
217217
// (undocumented)
218218
providers?: any[];
219+
rethrowApplicationErrors?: boolean;
219220
// (undocumented)
220221
schemas?: Array<SchemaMetadata | any[]>;
221222
// (undocumented)

packages/core/test/component_fixture_spec.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -367,32 +367,25 @@ describe('ComponentFixture', () => {
367367
})
368368
class Blank {}
369369

370-
// note: this test only verifies existing behavior was not broken by a change to the zoneless fixture.
371-
// We probably do want the whenStable promise to be rejected. The current zone-based fixture is bad
372-
// and confusing for two reason:
373-
// 1. with autoDetect, errors in the fixture _cannot be handled_ with whenStable because
374-
// they're just thrown inside the rxjs subcription (and then goes to setTimeout(() => throw e))
375-
// 2. errors from other views attached to ApplicationRef just go to the ErrorHandler, which by default
376-
// only logs to console, allowing the test to pass
377-
it('resolves whenStable promise when errors happen during appRef.tick', async () => {
370+
it('rejects whenStable promise when errors happen during appRef.tick', async () => {
378371
const fixture = TestBed.createComponent(Blank);
379372
const throwingThing = createComponent(ThrowingThing, {
380373
environmentInjector: TestBed.inject(EnvironmentInjector),
381374
});
382375

383376
TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
384-
await expectAsync(fixture.whenStable()).toBeResolved();
377+
await expectAsync(fixture.whenStable()).toBeRejected();
385378
});
386379

387-
it('can opt-in to rethrowing application errors and rejecting whenStable promises', async () => {
388-
TestBed.configureTestingModule({_rethrowApplicationTickErrors: true} as any);
380+
it('can opt-out of rethrowing application errors and rejecting whenStable promises', async () => {
381+
TestBed.configureTestingModule({rethrowApplicationErrors: false});
389382
const fixture = TestBed.createComponent(Blank);
390383
const throwingThing = createComponent(ThrowingThing, {
391384
environmentInjector: TestBed.inject(EnvironmentInjector),
392385
});
393386

394387
TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
395-
await expectAsync(fixture.whenStable()).toBeRejected();
388+
await expectAsync(fixture.whenStable()).toBeResolved();
396389
});
397390
});
398391

packages/core/testing/src/application_error_handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ErrorHandler, inject, NgZone, Injectable, InjectionToken} from '@angular/core';
9+
import {ErrorHandler, inject, NgZone, Injectable} from '@angular/core';
1010

11-
export const RETHROW_APPLICATION_ERRORS = new InjectionToken<boolean>('rethrow application errors');
11+
export const RETHROW_APPLICATION_ERRORS_DEFAULT = true;
1212

1313
@Injectable()
1414
export class TestBedApplicationErrorHandler {

packages/core/testing/src/test_bed_common.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,25 @@ export interface TestModuleMetadata {
6868
*/
6969
errorOnUnknownProperties?: boolean;
7070

71+
/**
72+
* Whether errors that happen during application change detection should be rethrown.
73+
*
74+
* When `true`, errors that are caught during application change detection will
75+
* be reported to the `ErrorHandler` and rethrown to prevent them from going
76+
* unnoticed in tests.
77+
*
78+
* When `false`, errors are only forwarded to the `ErrorHandler`, which by default
79+
* simply logs them to the console.
80+
*
81+
* Defaults to `true`.
82+
*/
83+
rethrowApplicationErrors?: boolean;
84+
7185
/**
7286
* Whether defer blocks should behave with manual triggering or play through normally.
7387
* Defaults to `manual`.
7488
*/
7589
deferBlockBehavior?: DeferBlockBehavior;
76-
77-
/** @internal */
78-
_rethrowApplicationTickErrors?: boolean;
7990
}
8091

8192
/**

packages/core/testing/src/test_bed_compiler.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import {
2222
LOCALE_ID,
2323
ModuleWithComponentFactories,
2424
ModuleWithProviders,
25-
ɵZONELESS_ENABLED as ZONELESS_ENABLED,
2625
NgModule,
2726
NgModuleFactory,
2827
Pipe,
@@ -52,6 +51,7 @@ import {
5251
ɵNG_INJ_DEF as NG_INJ_DEF,
5352
ɵNG_MOD_DEF as NG_MOD_DEF,
5453
ɵNG_PIPE_DEF as NG_PIPE_DEF,
54+
ɵZONELESS_ENABLED as ZONELESS_ENABLED,
5555
ɵNgModuleFactory as R3NgModuleFactory,
5656
ɵNgModuleTransitiveScopes as NgModuleTransitiveScopes,
5757
ɵNgModuleType as NgModuleType,
@@ -80,7 +80,7 @@ import {
8080
} from './resolvers';
8181
import {DEFER_BLOCK_DEFAULT_BEHAVIOR, TestModuleMetadata} from './test_bed_common';
8282
import {
83-
RETHROW_APPLICATION_ERRORS,
83+
RETHROW_APPLICATION_ERRORS_DEFAULT,
8484
TestBedApplicationErrorHandler,
8585
} from './application_error_handler';
8686

@@ -190,6 +190,7 @@ export class TestBedCompiler {
190190
private testModuleRef: NgModuleRef<any> | null = null;
191191

192192
private deferBlockBehavior = DEFER_BLOCK_DEFAULT_BEHAVIOR;
193+
private rethrowApplicationTickErrors = RETHROW_APPLICATION_ERRORS_DEFAULT;
193194

194195
constructor(
195196
private platform: PlatformRef,
@@ -226,16 +227,14 @@ export class TestBedCompiler {
226227
if (moduleDef.providers !== undefined) {
227228
this.providers.push(...moduleDef.providers);
228229
}
229-
this.providers.push({
230-
provide: RETHROW_APPLICATION_ERRORS,
231-
useValue: moduleDef._rethrowApplicationTickErrors ?? false,
232-
});
233230

234231
if (moduleDef.schemas !== undefined) {
235232
this.schemas.push(...moduleDef.schemas);
236233
}
237234

238235
this.deferBlockBehavior = moduleDef.deferBlockBehavior ?? DEFER_BLOCK_DEFAULT_BEHAVIOR;
236+
this.rethrowApplicationTickErrors =
237+
moduleDef.rethrowApplicationErrors ?? RETHROW_APPLICATION_ERRORS_DEFAULT;
239238
}
240239

241240
overrideModule(ngModule: Type<any>, override: MetadataOverride<NgModule>): void {
@@ -944,29 +943,28 @@ export class TestBedCompiler {
944943
...this.rootProviderOverrides,
945944
internalProvideZoneChangeDetection({}),
946945
TestBedApplicationErrorHandler,
947-
{
948-
provide: INTERNAL_APPLICATION_ERROR_HANDLER,
949-
useFactory: () => {
950-
if (inject(ZONELESS_ENABLED) || inject(RETHROW_APPLICATION_ERRORS, {optional: true})) {
951-
const handler = inject(TestBedApplicationErrorHandler);
952-
return (e: unknown) => {
953-
handler.handleError(e);
954-
};
955-
} else {
956-
const userErrorHandler = inject(ErrorHandler);
957-
const ngZone = inject(NgZone);
958-
return (e: unknown) =>
959-
ngZone.runOutsideAngular(() => userErrorHandler.handleError(e));
960-
}
961-
},
962-
},
963946
{provide: ChangeDetectionScheduler, useExisting: ChangeDetectionSchedulerImpl},
964947
],
965948
});
966949

967950
const providers = [
968951
{provide: Compiler, useFactory: () => new R3TestCompiler(this)},
969952
{provide: DEFER_BLOCK_CONFIG, useValue: {behavior: this.deferBlockBehavior}},
953+
{
954+
provide: INTERNAL_APPLICATION_ERROR_HANDLER,
955+
useFactory: () => {
956+
if (this.rethrowApplicationTickErrors) {
957+
const handler = inject(TestBedApplicationErrorHandler);
958+
return (e: unknown) => {
959+
handler.handleError(e);
960+
};
961+
} else {
962+
const userErrorHandler = inject(ErrorHandler);
963+
const ngZone = inject(NgZone);
964+
return (e: unknown) => ngZone.runOutsideAngular(() => userErrorHandler.handleError(e));
965+
}
966+
},
967+
},
970968
...this.providers,
971969
...this.providerOverrides,
972970
];

0 commit comments

Comments
 (0)