Skip to content

refactor(core): Private option to rethrow ApplicationRef.tick errors in tests #57153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
atscott wants to merge 1 commit into from
Closed

refactor(core): Private option to rethrow ApplicationRef.tick errors in tests #57153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 26 additions & 14 deletions packages/core/test/component_fixture_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
ErrorHandler,
Injectable,
Input,
NgZone,
createComponent,
provideExperimentalZonelessChangeDetection,
signal,
Expand Down Expand Up @@ -352,14 +351,7 @@ describe('ComponentFixture', () => {
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 () => {
describe('errors during ApplicationRef.tick', () => {
@Component({
template: '',
standalone: true,
Expand All @@ -375,13 +367,33 @@ describe('ComponentFixture', () => {
})
class Blank {}

const fixture = TestBed.createComponent(Blank);
const throwingThing = createComponent(ThrowingThing, {
environmentInjector: TestBed.inject(EnvironmentInjector),
// 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 () => {
const fixture = TestBed.createComponent(Blank);
const throwingThing = createComponent(ThrowingThing, {
environmentInjector: TestBed.inject(EnvironmentInjector),
});

TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
await expectAsync(fixture.whenStable()).toBeResolved();
});

TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
await expectAsync(fixture.whenStable()).toBeResolved();
it('can opt-in to rethrowing application errors and rejecting whenStable promises', async () => {
TestBed.configureTestingModule({_rethrowApplicationTickErrors: true} as any);
const fixture = TestBed.createComponent(Blank);
const throwingThing = createComponent(ThrowingThing, {
environmentInjector: TestBed.inject(EnvironmentInjector),
});

TestBed.inject(ApplicationRef).attachView(throwingThing.hostView);
await expectAsync(fixture.whenStable()).toBeRejected();
});
});

describe('defer', () => {
Expand Down
20 changes: 3 additions & 17 deletions packages/core/testing/src/application_error_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,17 @@
* found in the LICENSE file at https://angular.io/license
*/

import {
ErrorHandler,
inject,
NgZone,
Injectable,
ɵZONELESS_ENABLED as ZONELESS_ENABLED,
} from '@angular/core';
import {ErrorHandler, inject, NgZone, Injectable, InjectionToken} from '@angular/core';

export const RETHROW_APPLICATION_ERRORS = new InjectionToken<boolean>('rethrow application errors');

@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) {
Expand Down
31 changes: 11 additions & 20 deletions packages/core/testing/src/component_fixture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,7 @@ export abstract class ComponentFixture<T> {
/** @internal */
protected readonly _testAppRef = this._appRef as unknown as TestAppRef;
private readonly pendingTasks = inject(PendingTasks);
/** @internal */
protected readonly _appErrorHandler = inject(TestBedApplicationErrorHandler);
/** @internal */
protected _rejectWhenStablePromiseOnAppError = true;
private readonly appErrorHandler = inject(TestBedApplicationErrorHandler);

// TODO(atscott): Remove this from public API
ngZone = this._noZoneOptionIsSet ? null : this._ngZone;
Expand Down Expand Up @@ -137,7 +134,16 @@ export abstract class ComponentFixture<T> {
return Promise.resolve(false);
}

return this._appRef.isStable.pipe(first((stable) => stable)).toPromise();
return new Promise((resolve, reject) => {
this.appErrorHandler.whenStableRejectFunctions.add(reject);
this._appRef.isStable
.pipe(first((stable) => stable))
.toPromise()
.then((v) => {
this.appErrorHandler.whenStableRejectFunctions.delete(reject);
resolve(v);
});
});
}

/**
Expand Down Expand Up @@ -200,16 +206,6 @@ export class ScheduledComponentFixture<T> extends ComponentFixture<T> {
}
}

override whenStable(): Promise<any> {
return new Promise((resolve, reject) => {
this._appErrorHandler.whenStableRejectFunctions.add(reject);
super.whenStable().then((v) => {
this._appErrorHandler.whenStableRejectFunctions.delete(reject);
resolve(v);
});
});
}

override detectChanges(checkNoChanges = true): void {
if (!checkNoChanges) {
throw new Error(
Expand Down Expand Up @@ -252,11 +248,6 @@ export class PseudoApplicationComponentFixture<T> extends ComponentFixture<T> {
private beforeRenderSubscription: Subscription | undefined = undefined;

initialize(): void {
// TODO(atscott): Determine whether we can align this behavior with the zoneless fixture.
// This exists to keep the previous zone-based fixture behavior consistent with how it was before.
// However, we currently feel that the zoneless fixture is doing the more correct thing.
this._rejectWhenStablePromiseOnAppError = false;

if (this._autoDetect) {
this.subscribeToAppRefEvents();
}
Expand Down
3 changes: 3 additions & 0 deletions packages/core/testing/src/test_bed_common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ export interface TestModuleMetadata {
* Defaults to `manual`.
*/
deferBlockBehavior?: DeferBlockBehavior;

/** @internal */
_rethrowApplicationTickErrors?: boolean;
}

/**
Expand Down
27 changes: 20 additions & 7 deletions packages/core/testing/src/test_bed_compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
Compiler,
COMPILER_OPTIONS,
Component,
ErrorHandler,
Directive,
Injector,
inject,
Expand All @@ -39,7 +38,6 @@ import {
ɵcompilePipe as compilePipe,
ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID,
ɵDEFER_BLOCK_CONFIG as DEFER_BLOCK_CONFIG,
ɵDeferBlockBehavior as DeferBlockBehavior,
ɵdepsTracker as depsTracker,
ɵDirectiveDef as DirectiveDef,
ɵgenerateStandaloneInDeclarationsError,
Expand Down Expand Up @@ -67,6 +65,7 @@ import {
ɵUSE_RUNTIME_DEPS_TRACKER_FOR_JIT as USE_RUNTIME_DEPS_TRACKER_FOR_JIT,
ɵɵInjectableDeclaration as InjectableDeclaration,
NgZone,
ErrorHandler,
} from '@angular/core';

import {ComponentDef, ComponentType} from '../../src/render3';
Expand All @@ -80,7 +79,10 @@ import {
Resolver,
} from './resolvers';
import {DEFER_BLOCK_DEFAULT_BEHAVIOR, TestModuleMetadata} from './test_bed_common';
import {TestBedApplicationErrorHandler} from './application_error_handler';
import {
RETHROW_APPLICATION_ERRORS,
TestBedApplicationErrorHandler,
} from './application_error_handler';

enum TestingModuleOverride {
DECLARATION,
Expand Down Expand Up @@ -224,6 +226,10 @@ 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);
Expand Down Expand Up @@ -941,10 +947,17 @@ export class TestBedCompiler {
{
provide: INTERNAL_APPLICATION_ERROR_HANDLER,
useFactory: () => {
const handler = inject(TestBedApplicationErrorHandler);
return (e: unknown) => {
handler.handleError(e);
};
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},
Expand Down