fix(ivy): errors not being logged to ErrorHandler (#28447)

Fixes Ivy not passing thrown errors along to the `ErrorHandler`.

**Note:** the failing test had to be reworked a little bit, because it has some assertions that depend on an error context being logged, however Ivy doesn't keep track of the error context.

This PR resolves FW-840.

PR Close #28447
This commit is contained in:
Kristiyan Kostadinov 2019-01-30 15:23:23 +01:00 committed by Matias Niemelä
parent a98d66078d
commit fc88a79b32
3 changed files with 137 additions and 79 deletions

View File

@ -8,6 +8,7 @@
import {InjectFlags, InjectionToken, Injector} from '../di';
import {resolveForwardRef} from '../di/forward_ref';
import {ErrorHandler} from '../error_handler';
import {Type} from '../interface/type';
import {validateAttribute, validateProperty} from '../sanitization/sanitization';
import {Sanitizer} from '../sanitization/security';
@ -936,6 +937,7 @@ function listenerInternal(
const tView = lView[TVIEW];
const firstTemplatePass = tView.firstTemplatePass;
const tCleanup: false|any[] = firstTemplatePass && (tView.cleanup || (tView.cleanup = []));
ngDevMode && assertNodeOfPossibleTypes(
tNode, TNodeType.Element, TNodeType.Container, TNodeType.ElementContainer);
@ -2605,6 +2607,7 @@ function wrapListener(
markViewDirty(startView);
}
try {
const result = listenerFn(e);
if (wrapWithPreventDefault && result === false) {
e.preventDefault();
@ -2612,6 +2615,9 @@ function wrapListener(
e.returnValue = false;
}
return result;
} catch (error) {
handleError(lView, error);
}
};
}
/**
@ -2723,12 +2729,17 @@ export function detectChangesInternal<T>(view: LView, context: T) {
if (rendererFactory.begin) rendererFactory.begin();
try {
if (isCreationMode(view)) {
checkView(view, context); // creation mode pass
}
checkView(view, context); // update mode pass
} catch (error) {
handleError(view, error);
throw error;
} finally {
if (rendererFactory.end) rendererFactory.end();
}
}
/**
@ -3237,3 +3248,10 @@ function loadComponentRenderer(tNode: TNode, lView: LView): Renderer3 {
const componentLView = lView[tNode.index] as LView;
return componentLView[RENDERER];
}
/** Handles an error thrown in an LView. */
function handleError(lView: LView, error: any): void {
const injector = lView[INJECTOR];
const errorHandler = injector ? injector.get(ErrorHandler, null) : null;
errorHandler && errorHandler.handleError(error);
}

View File

@ -50,12 +50,24 @@
{
"name": "EMPTY_OBJ"
},
{
"name": "ERROR_DEBUG_CONTEXT"
},
{
"name": "ERROR_LOGGER"
},
{
"name": "ERROR_ORIGINAL_ERROR"
},
{
"name": "ElementRef"
},
{
"name": "EmptyErrorImpl"
},
{
"name": "ErrorHandler"
},
{
"name": "FLAGS"
},
@ -491,6 +503,9 @@
{
"name": "decreaseElementDepthCount"
},
{
"name": "defaultErrorLogger"
},
{
"name": "defaultScheduler"
},
@ -641,6 +656,9 @@
{
"name": "getCurrentView"
},
{
"name": "getDebugContext"
},
{
"name": "getDirectiveDef"
},
@ -656,6 +674,9 @@
{
"name": "getElementDepthCount"
},
{
"name": "getErrorLogger"
},
{
"name": "getHighestElementOrICUContainer"
},
@ -722,6 +743,9 @@
{
"name": "getOrCreateTView"
},
{
"name": "getOriginalError"
},
{
"name": "getParentInjectorIndex"
},
@ -803,6 +827,9 @@
{
"name": "getValue"
},
{
"name": "handleError"
},
{
"name": "hasClassInput"
},

View File

@ -450,8 +450,7 @@ function declareTestsUsingBootstrap() {
if (getDOM().supportsDOMEvents()) {
// This test needs a real DOM....
fixmeIvy('FW-840: Exceptions thrown in event handlers are not reported to ErrorHandler')
.it('should keep change detecting if there was an error', (done) => {
it('should keep change detecting if there was an error', (done) => {
@Component({
selector: COMP_SELECTOR,
template:
@ -501,23 +500,35 @@ function declareTestsUsingBootstrap() {
const nextAndThrowBtn = compEl.children[1];
const nextAndThrowDirBtn = compEl.children[2];
// Note: the amount of events sent to the logger will differ between ViewEngine
// and Ivy, because Ivy doesn't attach an error context. This means that the amount
// of logged errors increases by 1 for Ivy and 2 for ViewEngine after each event.
const errorDelta = ivyEnabled ? 1 : 2;
let currentErrorIndex = 0;
nextBtn.click();
assertValueAndErrors(compEl, 1, 0);
assertValueAndErrors(compEl, 1, currentErrorIndex);
currentErrorIndex += errorDelta;
nextBtn.click();
assertValueAndErrors(compEl, 2, 2);
assertValueAndErrors(compEl, 2, currentErrorIndex);
currentErrorIndex += errorDelta;
nextAndThrowBtn.click();
assertValueAndErrors(compEl, 3, 4);
assertValueAndErrors(compEl, 3, currentErrorIndex);
currentErrorIndex += errorDelta;
nextAndThrowBtn.click();
assertValueAndErrors(compEl, 4, 6);
assertValueAndErrors(compEl, 4, currentErrorIndex);
currentErrorIndex += errorDelta;
nextAndThrowDirBtn.click();
assertValueAndErrors(compEl, 5, 8);
assertValueAndErrors(compEl, 5, currentErrorIndex);
currentErrorIndex += errorDelta;
nextAndThrowDirBtn.click();
assertValueAndErrors(compEl, 6, 10);
assertValueAndErrors(compEl, 6, currentErrorIndex);
currentErrorIndex += errorDelta;
// Assert that there were no more errors
expect(logger.errors.length).toBe(12);
expect(logger.errors.length).toBe(currentErrorIndex);
done();
});
@ -525,7 +536,9 @@ function declareTestsUsingBootstrap() {
expect(compEl).toHaveText(`Value:${value}`);
expect(logger.errors[errorIndex][0]).toBe('ERROR');
expect(logger.errors[errorIndex][1].message).toBe(`Error: ${value}`);
expect(logger.errors[errorIndex + 1][0]).toBe('ERROR CONTEXT');
// Ivy doesn't attach an error context.
!ivyEnabled && expect(logger.errors[errorIndex + 1][0]).toBe('ERROR CONTEXT');
}
});
}