From b2822cb720073357f6f565f3fa12f0ac608ddbc5 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Sun, 22 Oct 2023 21:26:32 -0700 Subject: [PATCH] refactor(core): report `@defer` errors using `ErrorHandler` This commit updates the code to report errors via `ErrorHandler` instance. For dependency loading problems, errors are reported only when `@error` block is not provided. --- goldens/public-api/core/errors.md | 2 + packages/core/src/defer/instructions.ts | 20 ++- packages/core/src/errors.ts | 3 + packages/core/test/acceptance/defer_spec.ts | 152 +++++++++++++++++++- 4 files changed, 175 insertions(+), 2 deletions(-) diff --git a/goldens/public-api/core/errors.md b/goldens/public-api/core/errors.md index 4aeefda8d79409..9d36522e527f42 100644 --- a/goldens/public-api/core/errors.md +++ b/goldens/public-api/core/errors.md @@ -29,6 +29,8 @@ export const enum RuntimeErrorCode { // (undocumented) CYCLIC_DI_DEPENDENCY = -200, // (undocumented) + DEFER_LOADING_FAILED = 750, + // (undocumented) DUPLICATE_DIRECTITVE = 309, // (undocumented) EXPORT_NOT_FOUND = -301, diff --git a/packages/core/src/defer/instructions.ts b/packages/core/src/defer/instructions.ts index f6d4d27564fee7..47a0a06b550eeb 100644 --- a/packages/core/src/defer/instructions.ts +++ b/packages/core/src/defer/instructions.ts @@ -7,12 +7,15 @@ */ import {InjectionToken, Injector} from '../di'; +import {RuntimeError, RuntimeErrorCode} from '../errors'; import {findMatchingDehydratedView} from '../hydration/views'; import {populateDehydratedViewsInLContainer} from '../linker/view_container_ref'; import {assertLContainer, assertTNodeForLView} from '../render3/assert'; import {bindingUpdated} from '../render3/bindings'; import {getComponentDef, getDirectiveDef, getPipeDef} from '../render3/definition'; +import {getTemplateLocationDetails} from '../render3/instructions/element_validation'; import {markViewDirty} from '../render3/instructions/mark_view_dirty'; +import {handleError} from '../render3/instructions/shared'; import {ɵɵtemplate} from '../render3/instructions/template'; import {LContainer} from '../render3/interfaces/container'; import {DirectiveDefList, PipeDefList} from '../render3/interfaces/definition'; @@ -463,7 +466,11 @@ export function renderDeferBlockState( const applyStateFn = needsScheduling ? applyDeferBlockStateWithSchedulingImpl! : applyDeferBlockState; - applyStateFn(newState, lDetails, lContainer, tNode, hostLView); + try { + applyStateFn(newState, lDetails, lContainer, tNode, hostLView); + } catch (error: unknown) { + handleError(hostLView, error); + } } } @@ -665,6 +672,17 @@ export function triggerResourceLoading(tDetails: TDeferBlockDetails, lView: LVie if (failed) { tDetails.loadingState = DeferDependenciesLoadingState.FAILED; + + if (tDetails.errorTmplIndex === null) { + const templateLocation = getTemplateLocationDetails(lView); + const error = new RuntimeError( + RuntimeErrorCode.DEFER_LOADING_FAILED, + ngDevMode && + 'Loading dependencies for `@defer` block failed, ' + + `but no \`@error\` block was configured${templateLocation}. ` + + 'Consider using the `@error` block to render an error state.'); + handleError(lView, error); + } } else { tDetails.loadingState = DeferDependenciesLoadingState.COMPLETE; diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts index 331f765de694d2..d220794f37d710 100644 --- a/packages/core/src/errors.ts +++ b/packages/core/src/errors.ts @@ -93,6 +93,9 @@ export const enum RuntimeErrorCode { INVALID_I18N_STRUCTURE = 700, MISSING_LOCALE_DATA = 701, + // Defer errors (750-799 range) + DEFER_LOADING_FAILED = 750, + // standalone errors IMPORT_PROVIDERS_FROM_STANDALONE = 800, diff --git a/packages/core/test/acceptance/defer_spec.ts b/packages/core/test/acceptance/defer_spec.ts index 0cea738eec30a1..cf7e2602fe0ae5 100644 --- a/packages/core/test/acceptance/defer_spec.ts +++ b/packages/core/test/acceptance/defer_spec.ts @@ -7,7 +7,7 @@ */ import {ɵPLATFORM_BROWSER_ID as PLATFORM_BROWSER_ID} from '@angular/common'; -import {Attribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, Directive, inject, Input, NgZone, Pipe, PipeTransform, PLATFORM_ID, QueryList, Type, ViewChildren, ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR} from '@angular/core'; +import {Attribute, ChangeDetectionStrategy, ChangeDetectorRef, Component, Directive, ErrorHandler, inject, Input, NgZone, Pipe, PipeTransform, PLATFORM_ID, QueryList, Type, ViewChildren, ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR} from '@angular/core'; import {getComponentDef} from '@angular/core/src/render3/definition'; import {ComponentFixture, DeferBlockBehavior, fakeAsync, flush, TestBed, tick} from '@angular/core/testing'; @@ -863,6 +863,156 @@ describe('@defer', () => { expect(fixture.componentInstance.cmps.length).toBe(1); expect(fixture.componentInstance.cmps.get(0)?.block).toBe('error'); }); + + it('should report an error to the ErrorHandler if no `@error` block is defined', async () => { + @Component({ + selector: 'nested-cmp', + standalone: true, + template: 'NestedCmp', + }) + class NestedCmp { + } + + @Component({ + standalone: true, + selector: 'simple-app', + imports: [NestedCmp], + template: ` + @defer (when isVisible) { + + } @loading { + Loading... + } @placeholder { + Placeholder + } + ` + }) + class MyCmp { + isVisible = false; + } + + const deferDepsInterceptor = { + intercept() { + return () => [failedDynamicImport()]; + } + }; + + const reportedErrors: Error[] = []; + TestBed.configureTestingModule({ + providers: [ + { + provide: ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR, + useValue: deferDepsInterceptor, + }, + { + provide: ErrorHandler, useClass: class extends ErrorHandler{ + override handleError(error: Error) { + reportedErrors.push(error); + } + }, + }, + ], + deferBlockBehavior: DeferBlockBehavior.Playthrough, + }); + + const fixture = TestBed.createComponent(MyCmp); + fixture.detectChanges(); + + expect(fixture.nativeElement.outerHTML).toContain('Placeholder'); + + fixture.componentInstance.isVisible = true; + fixture.detectChanges(); + + expect(fixture.nativeElement.outerHTML).toContain('Loading'); + + // Wait for dependencies to load. + await allPendingDynamicImports(); + fixture.detectChanges(); + + // Verify that there was an error reported to the `ErrorHandler`. + expect(reportedErrors.length).toBe(1); + expect(reportedErrors[0].message).toContain('NG0750'); + expect(reportedErrors[0].message).toContain(`(used in the 'MyCmp' component template)`); + }); + + it('should not render `@error` block if loaded component has errors', async () => { + @Component({ + selector: 'cmp-with-error', + standalone: true, + template: 'CmpWithError', + }) + class CmpWithError { + constructor() { + throw new Error('CmpWithError produced an error'); + } + } + + @Component({ + standalone: true, + selector: 'simple-app', + imports: [CmpWithError], + template: ` + @defer (when isVisible) { + + } @loading { + Loading... + } @error { + Error + } @placeholder { + Placeholder + } + ` + }) + class MyCmp { + isVisible = false; + } + + const deferDepsInterceptor = { + intercept() { + return () => [dynamicImportOf(CmpWithError)]; + } + }; + + const reportedErrors: Error[] = []; + TestBed.configureTestingModule({ + providers: [ + { + provide: ɵDEFER_BLOCK_DEPENDENCY_INTERCEPTOR, + useValue: deferDepsInterceptor, + }, + { + provide: ErrorHandler, useClass: class extends ErrorHandler{ + override handleError(error: Error) { + reportedErrors.push(error); + } + }, + }, + ], + deferBlockBehavior: DeferBlockBehavior.Playthrough, + }); + + const fixture = TestBed.createComponent(MyCmp); + fixture.detectChanges(); + + expect(fixture.nativeElement.outerHTML).toContain('Placeholder'); + + fixture.componentInstance.isVisible = true; + fixture.detectChanges(); + + expect(fixture.nativeElement.outerHTML).toContain('Loading'); + + // Wait for dependencies to load. + await allPendingDynamicImports(); + fixture.detectChanges(); + + // Expect an error to be reported to the `ErrorHandler`. + expect(reportedErrors.length).toBe(1); + expect(reportedErrors[0].message).toBe('CmpWithError produced an error'); + + // Expect that the `@loading` UI is removed, but the `@error` is *not* rendered, + // because it was a component initialization error, not resource loading issue. + expect(fixture.nativeElement.textContent).toBe(''); + }); }); describe('queries', () => {