fix(core): attempt to recover from user errors during creation (#36381)

If there's an error during the first creation pass of a `TView`, the data structure may be corrupted which will cause framework assertion failures downstream which can mask the user's error. These changes add a new flag to the `TView` that indicates whether the first creation pass was successful, and if it wasn't we try re-create the `TView`.

Fixes #31221.

PR Close #36381
This commit is contained in:
crisbeto 2020-04-20 20:49:08 +02:00 committed by Andrew Kushnir
parent 89c589085d
commit 3d82aa781d
5 changed files with 285 additions and 16 deletions

View File

@ -30,7 +30,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 137320,
"main-es2015": 137897,
"polyfills-es2015": 37334
}
}
@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 226728,
"main-es2015": 227258,
"polyfills-es2015": 36657,
"5-es2015": 779
}

View File

@ -143,6 +143,7 @@ export const TViewConstructor = class TView implements ITView {
public firstChild: ITNode|null, //
public schemas: SchemaMetadata[]|null, //
public consts: TConstants|null, //
public incompleteFirstPass: boolean //
) {}
get template_(): string {

View File

@ -374,6 +374,14 @@ export function renderView<T>(tView: TView, lView: LView, context: T): void {
renderChildComponents(lView, components);
}
} catch (error) {
// If we didn't manage to get past the first template pass due to
// an error, mark the view as corrupted so we can try to recover.
if (tView.firstCreatePass) {
tView.incompleteFirstPass = true;
}
throw error;
} finally {
lView[FLAGS] &= ~LViewFlags.CreationMode;
leaveView();
@ -598,10 +606,17 @@ export function saveResolvedLocalsInData(
* @returns TView
*/
export function getOrCreateTComponentView(def: ComponentDef<any>): TView {
return def.tView ||
(def.tView = createTView(
const tView = def.tView;
// Create a TView if there isn't one, or recreate it if the first create pass didn't
// complete successfuly since we can't know for sure whether it's in a usable shape.
if (tView === null || tView.incompleteFirstPass) {
return def.tView = createTView(
TViewType.Component, -1, def.template, def.decls, def.vars, def.directiveDefs,
def.pipeDefs, def.viewQuery, def.schemas, def.consts));
def.pipeDefs, def.viewQuery, def.schemas, def.consts);
}
return tView;
}
@ -662,7 +677,9 @@ export function createTView(
typeof pipes === 'function' ? pipes() : pipes, // pipeRegistry: PipeDefList|null,
null, // firstChild: TNode|null,
schemas, // schemas: SchemaMetadata[]|null,
consts) : // consts: TConstants|null
consts, // consts: TConstants|null
false // incompleteFirstPass: boolean
) :
{
type: type,
id: viewIndex,
@ -694,6 +711,7 @@ export function createTView(
firstChild: null,
schemas: schemas,
consts: consts,
incompleteFirstPass: false
};
}

View File

@ -684,6 +684,12 @@ export interface TView {
* Used for directive matching, attribute bindings, local definitions and more.
*/
consts: TConstants|null;
/**
* Indicates that there was an error before we managed to complete the first create pass of the
* view. This means that the view is likely corrupted and we should try to recover it.
*/
incompleteFirstPass: boolean;
}
export const enum RootContextFlags {

View File

@ -7,7 +7,7 @@
*/
import {CommonModule} from '@angular/common';
import {ChangeDetectorRef, Component, ComponentFactoryResolver, Directive, EmbeddedViewRef, Injector, NgModule, TemplateRef, ViewChild, ViewContainerRef, ViewRef} from '@angular/core';
import {ChangeDetectorRef, Component, ComponentFactoryResolver, Directive, EmbeddedViewRef, Injector, Input, NgModule, TemplateRef, ViewChild, ViewContainerRef, ViewRef} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {onlyInIvy} from '@angular/private/testing';
@ -398,11 +398,11 @@ describe('view insertion', () => {
const fixture = TestBed.createComponent(AppComponent);
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.textContent).toBe('start|test|end');
expect(fixture.nativeElement.textContent).toBe('start|test|end');
fixture.componentInstance.insertTpl = true;
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.textContent).toBe('start|testtest|end');
expect(fixture.nativeElement.textContent).toBe('start|testtest|end');
});
});
@ -643,4 +643,248 @@ describe('view insertion', () => {
expect(fixture.nativeElement.textContent.trim()).toContain('2 1');
});
});
describe('create mode error handling', () => {
it('should consistently report errors raised a directive constructor', () => {
@Directive({
selector: '[failInConstructorAlways]',
})
class FailInConstructorAlways {
constructor() {
throw new Error('Error in a constructor');
}
}
@Component({
template: `<div failInConstructorAlways></div>`,
})
class TestCmpt {
}
TestBed.configureTestingModule({
declarations: [TestCmpt, FailInConstructorAlways],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in a constructor');
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in a constructor');
});
it('should render even if a directive constructor throws in the first create pass', () => {
let firstRun = true;
@Directive({
selector: '[failInConstructorOnce]',
})
class FailInConstructorOnce {
constructor() {
if (firstRun) {
firstRun = false;
throw new Error('Error in a constructor');
}
}
}
@Component({
template: `<div failInConstructorOnce>OK</div>`,
})
class TestCmpt {
}
TestBed.configureTestingModule({
declarations: [TestCmpt, FailInConstructorOnce],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in a constructor');
const fixture = TestBed.createComponent(TestCmpt);
expect(fixture.nativeElement.textContent).toContain('OK');
});
onlyInIvy('Test depends on static inputs being set during creation')
.it('should consistently report errors raised a directive input setter', () => {
@Directive({
selector: '[failInInputAlways]',
})
class FailInInputAlways {
@Input()
set failInInputAlways(_: string) {
throw new Error('Error in an input');
}
}
@Component({
template: `<div failInInputAlways="static"></div>`,
})
class TestCmpt {
}
TestBed.configureTestingModule({
declarations: [TestCmpt, FailInInputAlways],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in an input');
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in an input');
});
it('should consistently report errors raised a static query setter', () => {
@Directive({
selector: '[someDir]',
})
class SomeDirective {
}
@Component({
template: `<div someDir></div>`,
})
class TestCmpt {
@ViewChild(SomeDirective, {static: true})
set directive(_: SomeDirective) {
throw new Error('Error in static query setter');
}
}
TestBed.configureTestingModule({
declarations: [TestCmpt, SomeDirective],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in static query setter');
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in static query setter');
});
it('should match a static query, even if its setter throws in the first create pass', () => {
let hasThrown = false;
@Directive({
selector: '[someDir]',
})
class SomeDirective {
}
@Component({
template: `<div someDir></div>`,
})
class TestCmpt {
@ViewChild(SomeDirective, {static: true})
get directive() {
return this._directive;
}
set directive(directiveInstance: SomeDirective) {
if (!hasThrown) {
hasThrown = true;
throw new Error('Error in static query setter');
}
this._directive = directiveInstance;
}
private _directive!: SomeDirective;
}
TestBed.configureTestingModule({
declarations: [TestCmpt, SomeDirective],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in static query setter');
const fixture = TestBed.createComponent(TestCmpt);
expect(fixture.componentInstance.directive).toBeInstanceOf(SomeDirective);
});
it('should render a recursive component if it throws during the first creation pass', () => {
let hasThrown = false;
@Component({
selector: 'test',
template: `<ng-content></ng-content>OK`,
})
class TestCmpt {
constructor() {
if (!hasThrown) {
hasThrown = true;
throw new Error('Error in a constructor');
}
}
}
@Component({
template: `<test><test><test></test></test></test>`,
})
class App {
}
TestBed.configureTestingModule({
declarations: [App, TestCmpt],
});
expect(() => {
TestBed.createComponent(App);
}).toThrowError('Error in a constructor');
const fixture = TestBed.createComponent(App);
expect(fixture.nativeElement.textContent).toContain('OKOKOK');
});
it('should continue detecting changes if a directive throws in its constructor', () => {
let firstRun = true;
@Directive({
selector: '[failInConstructorOnce]',
})
class FailInConstructorOnce {
constructor() {
if (firstRun) {
firstRun = false;
throw new Error('Error in a constructor');
}
}
}
@Component({
template: `<div failInConstructorOnce>{{value}}</div>`,
})
class TestCmpt {
value = 0;
}
TestBed.configureTestingModule({
declarations: [TestCmpt, FailInConstructorOnce],
});
expect(() => {
TestBed.createComponent(TestCmpt);
}).toThrowError('Error in a constructor');
const fixture = TestBed.createComponent(TestCmpt);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('0');
fixture.componentInstance.value = 1;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('1');
fixture.componentInstance.value = 2;
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toContain('2');
});
});
});