Преглед на файлове

Introduced Watchdog#state. Fixed existing tests.

Maciej Bukowski преди 6 години
родител
ревизия
80127be7d9
променени са 2 файла, в които са добавени 89 реда и са изтрити 33 реда
  1. 47 29
      packages/ckeditor5-watchdog/src/watchdog.js
  2. 42 4
      packages/ckeditor5-watchdog/tests/watchdog.js

+ 47 - 29
packages/ckeditor5-watchdog/src/watchdog.js

@@ -48,6 +48,15 @@ export default class Watchdog {
 		 */
 		this.crashes = [];
 
+		/**
+		 * Specifies the watchdog state.
+		 *
+		 * @public
+		 * @observable
+		 * @type {'initializing'|'ready'|'crashed'|'crashedPermanently'}
+		 */
+		this.state = 'initializing';
+
 		/**
 		 * Crash number limit (defaults to `3`). After this limit is reached and the {@link #_minNonErrorTimePeriod}
 		 * is also reached the editor is not restarted by the watchdog and the watchdog fires
@@ -56,7 +65,7 @@ export default class Watchdog {
 		 * @private
 		 * @type {Number}
 		 */
-		this._crashNumberLimit = crashNumberLimit || 3;
+		this._crashNumberLimit = typeof crashNumberLimit === 'number' ? crashNumberLimit : 3;
 
 		/**
 		 * Minumum non-error time period (defaults to `5000`). When the period of time between errors is lower than that,
@@ -64,7 +73,7 @@ export default class Watchdog {
 		 * the {@link #crash `crash` event}. This prevents an infinite restart loop.
 		 *
 		 */
-		this._minNonErrorTimePeriod = minNonErrorTimePeriod || 5000;
+		this._minNonErrorTimePeriod = typeof minNonErrorTimePeriod === 'number' ? minNonErrorTimePeriod : 5000;
 
 		/**
 		 * Checks if the event error comes from the editor that is handled by the watchdog (by checking the error context)
@@ -228,6 +237,7 @@ export default class Watchdog {
 
 				this._lastDocumentVersion = editor.model.document.version;
 				this._data = editor.data.get();
+				this.state = 'ready';
 			} );
 	}
 
@@ -283,23 +293,11 @@ export default class Watchdog {
 	 * @param {Event} evt Error event.
 	 */
 	_handleGlobalErrorEvent( evt ) {
-		if ( !evt.error.is || !evt.error.is( 'CKEditorError' ) ) {
-			return;
-		}
-
-		if ( evt.error.context === undefined ) {
+		if ( evt.error.is && evt.error.is( 'CKEditorError' ) && evt.error.context === undefined ) {
 			console.error( 'The error is missing its context and Watchdog cannot restart the proper editor.' );
-
-			return;
 		}
 
-		// In some cases the editor should not be restarted - e.g. in case of the editor initialization.
-		// That's why the `null` was introduced as a correct error context which does cause restarting.
-		if ( evt.error.context === null ) {
-			return;
-		}
-
-		if ( this._isErrorComingFromThisEditor( evt.error ) ) {
+		if ( this._shouldReactToErrorEvent( evt ) ) {
 			this.crashes.push( {
 				message: evt.error.message,
 				source: evt.source,
@@ -311,13 +309,37 @@ export default class Watchdog {
 			this.fire( 'error', { error: evt.error } );
 
 			if ( this._shouldRestartEditor() ) {
-				this.restart();
+				this.state = 'crashed';
+				this._restart();
 			} else {
-				this.fire( 'crash', { error: evt.error } );
+				this.state = 'crashedPermanently';
 			}
 		}
 	}
 
+	/**
+	 * Checks whether the evt should be handled.
+	 *
+	 * @private
+	 * @param {Event} evt Error event.
+	 */
+	_shouldReactToErrorEvent( evt ) {
+		return (
+			evt.error.is &&
+			evt.error.is( 'CKEditorError' ) &&
+			evt.error.context !== undefined &&
+
+			// In some cases the editor should not be restarted - e.g. in case of the editor initialization.
+			// That's why the `null` was introduced as a correct error context which does cause restarting.
+			evt.error.context !== null &&
+
+			// Do not react to errors if the watchdog is in states other than `ready`.
+			this.state === 'ready' &&
+
+			this._isErrorComingFromThisEditor( evt.error )
+		);
+	}
+
 	/**
 	 * Checks if the editor should be restared.
 	 */
@@ -326,20 +348,22 @@ export default class Watchdog {
 			return true;
 		}
 
-		return (
+		return ( (
 			this.crashes[ this.crashes.length - 1 ].date -
 			this.crashes[ this.crashes.length - 1 - this._crashNumberLimit ].date
-		) / this._crashNumberLimit > this._minNonErrorTimePeriod;
+		) / this._crashNumberLimit ) > this._minNonErrorTimePeriod;
 	}
 
 	/**
-	 * Restarts the editor instance. This method is called whenever an editor error occurs. It fires the `restart` event.
+	 * Restarts the editor instance. This method is called whenever an editor error occurs. It fires the `restart` event and changes
+	 * the state to `initializing`.
 	 *
 	 * @private
 	 * @fires restart
 	 * @returns {Promise}
 	 */
 	_restart() {
+		this.state = 'initializing';
 		this._throttledSave.flush();
 
 		return Promise.resolve()
@@ -394,18 +418,12 @@ export default class Watchdog {
 	}
 
 	/**
-	 * Fired when a new {@link module:utils/ckeditorerror~CKEditorError `CKEditorError`} error connected to the watchdog editor occurs.
+	 * Fired when a new {@link module:utils/ckeditorerror~CKEditorError `CKEditorError`} error connected to the watchdog editor occurs
+	 * and the watchdog will react to it.
 	 *
 	 * @event error
 	 */
 
-	/**
-	 * Fired when the error stack is greater than crash number limit and the error frequency threshold was exceeded.
-	 *
-	 * @event crash
-	 * @see #constructor
-	 */
-
 	/**
 	 * Fired after the watchdog restarts the error in case of a crash.
 	 *

+ 42 - 4
packages/ckeditor5-watchdog/tests/watchdog.js

@@ -368,7 +368,8 @@ describe( 'Watchdog', () => {
 			} );
 		} );
 
-		it( 'editor should be restarted up to 3 times by default', () => {
+		it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' +
+		' and the average time between errors is lower than `minNonErrorTimePeriod` (default values)', () => {
 			const watchdog = new Watchdog();
 
 			watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) );
@@ -404,8 +405,45 @@ describe( 'Watchdog', () => {
 			} );
 		} );
 
-		it( 'editor should be restarted up to `crashNumberLimit` times if the option is set', () => {
-			const watchdog = new Watchdog( { crashNumberLimit: 2 } );
+		it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' +
+		' and the average time between errors is lower than `minNonErrorTimePeriod` (custom values)', () => {
+			const watchdog = new Watchdog( { crashNumberLimit: 2, minNonErrorTimePeriod: 1000 } );
+
+			watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) );
+			watchdog.setDestructor( editor => editor.destroy() );
+
+			const errorSpy = sinon.spy();
+			watchdog.on( 'error', errorSpy );
+
+			const restartSpy = sinon.spy();
+			watchdog.on( 'restart', restartSpy );
+
+			// sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work.
+			const originalErrorHandler = window.onerror;
+			window.onerror = undefined;
+
+			return watchdog.create( element ).then( () => {
+				setTimeout( () => throwCKEditorError( 'foo1', watchdog.editor ) );
+				setTimeout( () => throwCKEditorError( 'foo2', watchdog.editor ) );
+				setTimeout( () => throwCKEditorError( 'foo3', watchdog.editor ) );
+				setTimeout( () => throwCKEditorError( 'foo4', watchdog.editor ) );
+
+				return new Promise( res => {
+					setTimeout( () => {
+						expect( errorSpy.callCount ).to.equal( 3 );
+						expect( watchdog.crashes.length ).to.equal( 3 );
+						expect( restartSpy.callCount ).to.equal( 2 );
+
+						window.onerror = originalErrorHandler;
+
+						watchdog.destroy().then( res );
+					} );
+				} );
+			} );
+		} );
+
+		it( 'Watchdog should not crash permantently when average time between errors is longer than `minNonErrorTimePeriod`', () => {
+			const watchdog = new Watchdog( { crashNumberLimit: 2, minNonErrorTimePeriod: 0 } );
 
 			watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) );
 			watchdog.setDestructor( editor => editor.destroy() );
@@ -430,7 +468,7 @@ describe( 'Watchdog', () => {
 					setTimeout( () => {
 						expect( errorSpy.callCount ).to.equal( 4 );
 						expect( watchdog.crashes.length ).to.equal( 4 );
-						expect( restartSpy.callCount ).to.equal( 2 );
+						expect( restartSpy.callCount ).to.equal( 4 );
 
 						window.onerror = originalErrorHandler;