Bladeren bron

Enhanced the error handling mechanism.

Maciej Bukowski 5 jaren geleden
bovenliggende
commit
70ff979831

+ 50 - 16
packages/ckeditor5-watchdog/src/contextwatchdog.js

@@ -35,9 +35,9 @@ export default class ContextWatchdog extends Watchdog {
 
 		/**
 		 * @private
-		 * @type {Set.<*>|undefined}
+		 * @type {Set.<*>}
 		 */
-		this._contextProps;
+		this._contextProps = new Set();
 
 		/**
 		 * @private
@@ -66,36 +66,46 @@ export default class ContextWatchdog extends Watchdog {
 		return this._context;
 	}
 
+	/**
+	 *
+	 * @param {String} name The watchdog name (the key under which the watchdog config was passed to the add() method).
+	 */
+	getWatchdog( name ) {
+		return this._watchdogs.get( name );
+	}
+
 	async add( items ) {
 		await this._actionQueue.enqueue( async () => {
 			if ( this.state === 'destroyed' ) {
 				throw new Error( 'Cannot add items do destroyed watchdog.' );
 			}
 
-			await Promise.all( Object.entries( items ).map( async ( [ itemName, itemConfig ] ) => {
+			if ( !this._context ) {
+				throw new Error( 'Context was not created yet. You should call the `ContextWatchdog#create()` method first.' );
+			}
+
+			await Promise.all( Object.entries( items ).map( async ( [ itemName, item ] ) => {
 				let watchdog;
 
 				if ( this._watchdogs.has( itemName ) ) {
 					throw new Error( `Watchdog with the given name is already added: '${ itemName }'.` );
 				}
 
-				if ( itemConfig.type === 'editor' ) {
+				if ( item.type === 'editor' ) {
+					// TODO - await EditorWatchdog.createFrom( item, context ) ?
 					watchdog = new EditorWatchdog();
-					watchdog.setCreator( itemConfig.creator );
+					watchdog.setCreator( item.creator );
+					watchdog._setExcludedProperties( this._contextProps );
 
-					if ( itemConfig.destructor ) {
-						watchdog.setDestructor( itemConfig.destructor );
+					if ( item.destructor ) {
+						watchdog.setDestructor( item.destructor );
 					}
 
 					this._watchdogs.set( itemName, watchdog );
 
-					const editorConfig = watchdog._cloneConfig( itemConfig.config );
-
-					editorConfig.context = this._context;
-
-					await watchdog.create( itemConfig.sourceElementOrData, editorConfig );
+					await watchdog.create( item.sourceElementOrData, item.config, this._context );
 				} else {
-					throw new Error( `Not supported item type: '${ itemConfig.type }'.` );
+					throw new Error( `Not supported item type: '${ item.type }'.` );
 				}
 			} ) );
 
@@ -125,12 +135,25 @@ export default class ContextWatchdog extends Watchdog {
 	}
 
 	/**
-	 * TODO
+	 * Waits for all previous actions.
 	 */
 	async waitForReady() {
 		await this._actionQueue.enqueue( () => { } );
 	}
 
+	/**
+	 * Creates the Context watchdog and all internal watchdogs.
+	 */
+	async create() {
+		await this._actionQueue.enqueue( async () => {
+			this._create( true );
+		} );
+	}
+
+	/**
+	 * Destroys the `ContextWatchdog` and all internal watchdogs. This method can't be undone.
+	 * Once the `ContextWatchdog` is destroyed new items can not be added.
+	 */
 	async destroy() {
 		await this._actionQueue.enqueue( async () => {
 			this.state = 'destroyed';
@@ -171,8 +194,8 @@ export default class ContextWatchdog extends Watchdog {
 			await Promise.all(
 				Array.from( this._watchdogs.values() )
 					.map( async watchdog => {
-						watchdog.updateContext( this._context );
-						await watchdog.create();
+						watchdog._setExcludedProperties( this._contextProps );
+						await watchdog.create( undefined, undefined, this._context );
 					} )
 			);
 
@@ -200,6 +223,12 @@ export default class ContextWatchdog extends Watchdog {
 	}
 
 	_isErrorComingFromThisInstance( error ) {
+		for ( const watchdog of this._watchdogs.values() ) {
+			if ( watchdog._isErrorComingFromThisInstance( error ) ) {
+				return false;
+			}
+		}
+
 		// Return true only if the error comes directly from the context.
 		// Ignore cases when the error comes from editors.
 		return areConnectedThroughProperties( this._contextProps, error.context );
@@ -264,6 +293,11 @@ class ActionQueue {
 		}
 	}
 
+	/**
+	 * @protected
+	 *
+	 * Clears all queued actions (e.g. in case of an error).
+	 */
 	clear() {
 		this._queuedActions = [];
 	}

+ 16 - 15
packages/ckeditor5-watchdog/src/editorwatchdog.js

@@ -120,14 +120,6 @@ export default class EditorWatchdog extends Watchdog {
 	 */
 
 	/**
-	 * @protected
-	 * @param {Context}
-	 */
-	updateContext( context ) {
-		this._config.context = context;
-	}
-
-	/**
 	 * Restarts the editor instance. This method is called whenever an editor error occurs. It fires the `restart` event and changes
 	 * the state to `initializing`.
 	 *
@@ -161,12 +153,13 @@ export default class EditorWatchdog extends Watchdog {
 	 * Creates a watched editor instance using the creator passed to the {@link #setCreator `setCreator()`} method or
 	 * the {@link module:watchdog/watchdog~Watchdog.for `Watchdog.for()`} helper.
 	 *
-	 * @param {HTMLElement|String|Object.<String|String>} elementOrData
+	 * @param {HTMLElement|String|Object.<String|String>} [elementOrData]
 	 * @param {module:core/editor/editorconfig~EditorConfig} [config]
+	 * @param {Object} [context]
 	 *
 	 * @returns {Promise}
 	 */
-	async create( elementOrData = this._elementOrData, config = this._config ) {
+	async create( elementOrData = this._elementOrData, config = this._config, context ) {
 		if ( !this._creator ) {
 			/**
 			 * The watchdog's editor creator is not defined. Define it by using
@@ -187,7 +180,9 @@ export default class EditorWatchdog extends Watchdog {
 
 		// Clone configuration because it might be shared within multiple watchdog instances. Otherwise,
 		// when an error occurs in one of these editors, the watchdog will restart all of them.
-		this._config = this._cloneConfig( config );
+		this._config = this._cloneConfig( config ) || {};
+
+		this._config.context = context;
 
 		const editor = await this._creator( elementOrData, this._config );
 
@@ -254,6 +249,14 @@ export default class EditorWatchdog extends Watchdog {
 	}
 
 	/**
+	 * @protected
+	 * @param {Set} props
+	 */
+	_setExcludedProperties( props ) {
+		this._excludedProps = props;
+	}
+
+	/**
 	 * Returns the editor data.
 	 *
 	 * @private
@@ -273,13 +276,11 @@ export default class EditorWatchdog extends Watchdog {
 	 * Traverses both structures to find out whether the error context is connected
 	 * with the current editor.
 	 *
-	 * @private
+	 * @protected
 	 * @param {module:utils/ckeditorerror~CKEditorError} error
 	 */
 	_isErrorComingFromThisInstance( error ) {
-		// TODO - remove context from the path.
-
-		return areConnectedThroughProperties( this._editor, error.context );
+		return areConnectedThroughProperties( this._editor, error.context, this._excludedProps );
 	}
 
 	_cloneConfig( config ) {

+ 3 - 3
packages/ckeditor5-watchdog/src/utils/areconnectedThroughProperties.js

@@ -15,13 +15,13 @@ import getSubNodes from './getsubnodes';
  * @param {Object|Array} obj1
  * @param {Object|Array} obj2
  */
-export default function areConnectedThroughProperties( obj1, obj2 ) {
+export default function areConnectedThroughProperties( obj1, obj2, excludedProperties = new Set() ) {
 	if ( obj1 === obj2 && isObject( obj1 ) ) {
 		return true;
 	}
 
-	const subNodes1 = getSubNodes( obj1 );
-	const subNodes2 = getSubNodes( obj2 );
+	const subNodes1 = getSubNodes( obj1, excludedProperties );
+	const subNodes2 = getSubNodes( obj2, excludedProperties );
 
 	for ( const node of subNodes1 ) {
 		if ( subNodes2.has( node ) ) {

+ 2 - 2
packages/ckeditor5-watchdog/src/utils/getsubnodes.js

@@ -9,7 +9,7 @@
 
 /* globals EventTarget, Event */
 
-export default function getSubNodes( head ) {
+export default function getSubNodes( head, excludedProperties = new Set() ) {
 	const nodes = [ head ];
 
 	// Nodes are stored to prevent infinite looping.
@@ -18,7 +18,7 @@ export default function getSubNodes( head ) {
 	while ( nodes.length > 0 ) {
 		const node = nodes.shift();
 
-		if ( subNodes.has( node ) || shouldNodeBeSkipped( node ) ) {
+		if ( subNodes.has( node ) || shouldNodeBeSkipped( node ) || excludedProperties.has( node ) ) {
 			continue;
 		}
 

+ 89 - 18
packages/ckeditor5-watchdog/tests/contextwatchdog.js

@@ -15,6 +15,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 describe( 'ContextWatchdog', () => {
 	let element1, element2;
 	let mainWatchdog;
+	let originalErrorHandler;
 
 	beforeEach( () => {
 		element1 = document.createElement( 'div' );
@@ -22,9 +23,14 @@ describe( 'ContextWatchdog', () => {
 
 		document.body.appendChild( element1 );
 		document.body.appendChild( element2 );
+
+		originalErrorHandler = window.onerror;
+		window.onerror = sinon.spy();
 	} );
 
 	afterEach( () => {
+		window.onerror = originalErrorHandler;
+
 		element1.remove();
 		element2.remove();
 
@@ -122,9 +128,6 @@ describe( 'ContextWatchdog', () => {
 
 				const oldContext = mainWatchdog.context;
 
-				const originalErrorHandler = window.onerror;
-				window.onerror = sinon.spy();
-
 				mainWatchdog.on( 'restart', errorSpy );
 
 				setTimeout( () => throwCKEditorError( 'foo', mainWatchdog.context ) );
@@ -134,8 +137,6 @@ describe( 'ContextWatchdog', () => {
 				sinon.assert.calledOnce( errorSpy );
 
 				expect( mainWatchdog.context ).to.not.equal( oldContext );
-
-				window.onerror = originalErrorHandler;
 			} );
 		} );
 	} );
@@ -308,6 +309,25 @@ describe( 'ContextWatchdog', () => {
 			await mainWatchdog.destroy();
 		} );
 
+		it( 'should not change the input items', async () => {
+			mainWatchdog = ContextWatchdog.for( Context, {} );
+
+			mainWatchdog.add( Object.freeze( {
+				editor1: {
+					type: 'editor',
+					creator: ( el, config ) => ClassicTestEditor.create( el, config ),
+					sourceElementOrData: element1,
+					config: {}
+				}
+			} ) );
+
+			mainWatchdog._restart();
+
+			await mainWatchdog.waitForReady();
+
+			await mainWatchdog.destroy();
+		} );
+
 		describe( 'in case of error handling', () => {
 			it( 'should restart the whole structure of editors if an error happens inside the `Context`', async () => {
 				mainWatchdog = ContextWatchdog.for( Context, {} );
@@ -324,9 +344,6 @@ describe( 'ContextWatchdog', () => {
 
 				const oldContext = mainWatchdog.context;
 				const restartSpy = sinon.spy();
-				const originalErrorHandler = window.onerror;
-
-				window.onerror = sinon.spy();
 
 				mainWatchdog.on( 'restart', restartSpy );
 
@@ -339,12 +356,10 @@ describe( 'ContextWatchdog', () => {
 				expect( mainWatchdog._watchdogs.get( 'editor1' ).state ).to.equal( 'ready' );
 				expect( mainWatchdog.context ).to.not.equal( oldContext );
 
-				window.onerror = originalErrorHandler;
-
 				await mainWatchdog.destroy();
 			} );
 
-			it( 'should restart the editor if an error happens inside the editor', async () => {
+			it( 'should restart only the editor if an error happens inside the editor', async () => {
 				mainWatchdog = ContextWatchdog.for( Context, {} );
 				mainWatchdog.add( {
 					editor1: {
@@ -359,22 +374,78 @@ describe( 'ContextWatchdog', () => {
 
 				const oldContext = mainWatchdog.context;
 				const restartSpy = sinon.spy();
-				const originalErrorHandler = window.onerror;
 
-				window.onerror = sinon.spy();
+				const oldEditor = mainWatchdog.getWatchdog( 'editor1' ).editor;
 
 				mainWatchdog.on( 'restart', restartSpy );
 
-				setTimeout( () => throwCKEditorError( 'foo', mainWatchdog.context ) );
+				setTimeout( () => throwCKEditorError( 'foo', oldEditor ) );
 
 				await waitCycle();
 
-				window.onerror = originalErrorHandler;
+				sinon.assert.notCalled( restartSpy );
 
-				sinon.assert.calledOnce( restartSpy );
+				expect( mainWatchdog.context ).to.equal( oldContext );
 
-				expect( mainWatchdog._watchdogs.get( 'editor1' ).state ).to.equal( 'ready' );
-				expect( mainWatchdog.context ).to.not.equal( oldContext );
+				expect( mainWatchdog.getWatchdog( 'editor1' ).editor ).to.not.equal( oldEditor );
+				expect( mainWatchdog.getWatchdog( 'editor1' ).state ).to.equal( 'ready' );
+
+				await mainWatchdog.destroy();
+			} );
+
+			it( 'should restart only the editor if an error happens inside one of the editors', async () => {
+				mainWatchdog = ContextWatchdog.for( Context, {} );
+
+				mainWatchdog.add( {
+					editor1: {
+						type: 'editor',
+						creator: ( el, config ) => ClassicTestEditor.create( el, config ),
+						sourceElementOrData: element1,
+						config: {}
+					},
+					editor2: {
+						type: 'editor',
+						creator: ( el, config ) => ClassicTestEditor.create( el, config ),
+						sourceElementOrData: element1,
+						config: {}
+					}
+				} );
+
+				await mainWatchdog.waitForReady();
+
+				const oldContext = mainWatchdog.context;
+
+				const editorWatchdog1 = mainWatchdog.getWatchdog( 'editor1' );
+				const editorWatchdog2 = mainWatchdog.getWatchdog( 'editor2' );
+
+				const oldEditor1 = editorWatchdog1.editor;
+				const oldEditor2 = editorWatchdog2.editor;
+
+				const mainWatchdogRestartSpy = sinon.spy();
+				const editorWatchdog1RestartSpy = sinon.spy();
+				const editorWatchdog2RestartSpy = sinon.spy();
+
+				mainWatchdog.on( 'restart', mainWatchdogRestartSpy );
+				editorWatchdog1.on( 'restart', editorWatchdog1RestartSpy );
+				editorWatchdog2.on( 'restart', editorWatchdog2RestartSpy );
+
+				setTimeout( () => throwCKEditorError( 'foo', editorWatchdog1.editor ) );
+
+				await waitCycle();
+
+				sinon.assert.calledOnce( editorWatchdog1RestartSpy );
+
+				sinon.assert.notCalled( mainWatchdogRestartSpy );
+				sinon.assert.notCalled( editorWatchdog2RestartSpy );
+
+				expect( editorWatchdog1.state ).to.equal( 'ready' );
+				expect( editorWatchdog2.state ).to.equal( 'ready' );
+				expect( mainWatchdog.state ).to.equal( 'ready' );
+
+				expect( oldEditor1 ).to.not.equal( editorWatchdog1.editor );
+				expect( oldEditor2 ).to.equal( editorWatchdog2.editor );
+
+				expect( mainWatchdog.context ).to.equal( oldContext );
 
 				await mainWatchdog.destroy();
 			} );