From 11f7c962776975cae2bf07cfcaa5512946366a35 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Wed, 31 Jul 2019 11:51:22 +0200 Subject: [PATCH 01/25] WIP - added the `restart` event and added configurable `minNonErrorTimePeriod` property. --- src/watchdog.js | 60 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index d02ef49..eeb3f3a 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -24,18 +24,23 @@ import areConnectedThroughProperties from '@ckeditor/ckeditor5-utils/src/areconn export default class Watchdog { /** * @param {Object} [config] The watchdog plugin configuration. - * @param {Number} [config.crashNumberLimit=3] A threshold specifying the number of crashes - * when the watchdog stops restarting the editor in case of errors. + * @param {Number} [config.crashNumberLimit=3] A threshold specifying the number of editor errors (defaults to `3`). + * After this limit is reached and the `minNonErrorTimePeriod` is also reached the editor is not restarted + * by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. + * @param {Number} [config.minNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. + * When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the editor is not + * restarted by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. * @param {Number} [config.waitingTime=5000] A minimum amount of milliseconds between saving editor data internally. */ - constructor( { crashNumberLimit, waitingTime } = {} ) { + constructor( { crashNumberLimit, minNonErrorTimePeriod, waitingTime } = {} ) { /** * An array of crashes saved as an object with the following properties: * * * `message`: `String`, * * `source`: `String`, * * `lineno`: `String`, - * * `colno`: `String` + * * `colno`: `String`, + * * `date`: `Number`, * * @public * @readonly @@ -44,14 +49,23 @@ export default class Watchdog { this.crashes = []; /** - * Crash number limit (defaults to `3`). After this limit is reached the editor is not restarted by the watchdog. - * This is to prevent an infinite crash loop. + * 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 + * the {@link #crash `crash` event}. This prevents an infinite restart loop. * * @private * @type {Number} */ this._crashNumberLimit = crashNumberLimit || 3; + /** + * Minumum non-error time period (defaults to `5000`). When the period of time between errors is lower than that, + * and the {@link #_crashNumberLimit} is also reached the editor is not restarted by the watchdog and the watchdog fires + * the {@link #crash `crash` event}. This prevents an infinite restart loop. + * + */ + this._minNonErrorTimePeriod = minNonErrorTimePeriod || 5000; + /** * Checks if the event error comes from the editor that is handled by the watchdog (by checking the error context) * and restarts the editor. @@ -290,17 +304,34 @@ export default class Watchdog { message: evt.error.message, source: evt.source, lineno: evt.lineno, - colno: evt.colno + colno: evt.colno, + date: Date.now() } ); - this.fire( 'error' ); + this.fire( 'error', { error: evt.error } ); - if ( this.crashes.length <= this._crashNumberLimit ) { - this._restart(); + if ( this._shouldRestartEditor() ) { + this.restart(); + } else { + this.fire( 'crash', { error: evt.error } ); } } } + /** + * Checks if the editor should be restared. + */ + _shouldRestartEditor() { + if ( this.crashes.length <= this._crashNumberLimit ) { + return true; + } + + return ( + this.crashes[ this.crashes.length - 1 ].date - + this.crashes[ this.crashes.length - 1 - this._crashNumberLimit ].date + ) / this._crashNumberLimit > this._minNonErrorTimePeriod; + } + /** * Restarts the editor instance. This method is called whenever an editor error occurs. It fires the `restart` event. * @@ -363,11 +394,18 @@ export default class Watchdog { } /** - * Fired when an error occurs and the watchdog will be restarting the editor. + * Fired when a new {@link module:utils/ckeditorerror~CKEditorError `CKEditorError`} error connected to the watchdog editor occurs. * * @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. * From 1d88340b9e96bdd73477895222f4dfc495827d37 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Wed, 31 Jul 2019 16:47:31 +0200 Subject: [PATCH 02/25] Introduced Watchdog#state. Fixed existing tests. --- src/watchdog.js | 76 +++++++++++++++++++++++++++++------------------ tests/watchdog.js | 46 +++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index eeb3f3a..3c4416d 100644 --- a/src/watchdog.js +++ b/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. * diff --git a/tests/watchdog.js b/tests/watchdog.js index f6731d1..91c6dec 100644 --- a/tests/watchdog.js +++ b/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; From 2e875fef2ed0718f8fbf6ef35f9e80be82195bc4 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 00:17:42 +0200 Subject: [PATCH 03/25] Made the `Watchdog#state` observable. --- src/watchdog.js | 12 +++---- tests/watchdog.js | 81 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index 3c4416d..061b96d 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -10,7 +10,7 @@ /* globals console, window */ import mix from '@ckeditor/ckeditor5-utils/src/mix'; -import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; +import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin'; import { throttle, cloneDeepWith, isElement } from 'lodash-es'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import areConnectedThroughProperties from '@ckeditor/ckeditor5-utils/src/areconnectedthroughproperties'; @@ -53,9 +53,9 @@ export default class Watchdog { * * @public * @observable - * @type {'initializing'|'ready'|'crashed'|'crashedPermanently'} + * @member {'initializing'|'ready'|'crashed'|'crashedPermanently'} #state */ - this.state = 'initializing'; + this.set( 'state', 'initializing' ); /** * Crash number limit (defaults to `3`). After this limit is reached and the {@link #_minNonErrorTimePeriod} @@ -307,9 +307,9 @@ export default class Watchdog { } ); this.fire( 'error', { error: evt.error } ); + this.state = 'crashed'; if ( this._shouldRestartEditor() ) { - this.state = 'crashed'; this._restart(); } else { this.state = 'crashedPermanently'; @@ -341,7 +341,7 @@ export default class Watchdog { } /** - * Checks if the editor should be restared. + * Checks if the editor should be restared or if it should be marked as crashed. */ _shouldRestartEditor() { if ( this.crashes.length <= this._crashNumberLimit ) { @@ -431,4 +431,4 @@ export default class Watchdog { */ } -mix( Watchdog, EmitterMixin ); +mix( Watchdog, ObservableMixin ); diff --git a/tests/watchdog.js b/tests/watchdog.js index 91c6dec..7ac407b 100644 --- a/tests/watchdog.js +++ b/tests/watchdog.js @@ -729,10 +729,7 @@ describe( 'Watchdog', () => { describe( 'crashes', () => { it( 'should be an array of caught errors by the watchdog', () => { - const watchdog = new Watchdog(); - - watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) ); - watchdog.setDestructor( editor => editor.destroy() ); + const watchdog = Watchdog.for( ClassicTestEditor ); // sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work. const originalErrorHandler = window.onerror; @@ -755,6 +752,82 @@ describe( 'Watchdog', () => { } ); } ); } ); + + describe( 'state', () => { + it( 'should reflect the state of the watchdog', () => { + const watchdog = Watchdog.for( ClassicTestEditor ); + + // sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work. + const originalErrorHandler = window.onerror; + window.onerror = undefined; + + expect( watchdog.state ).to.equal( 'initializing' ); + + return watchdog.create( element ).then( () => { + expect( watchdog.state ).to.equal( 'ready' ); + + return watchdog.create( element ).then( () => { + setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ) ); + setTimeout( () => throwCKEditorError( 'bar', watchdog.editor ) ); + + return new Promise( res => { + setTimeout( () => { + window.onerror = originalErrorHandler; + + expect( watchdog.state ).to.equal( 'ready' ); + + watchdog.destroy().then( res ); + } ); + } ); + } ); + } ); + } ); + + it( 'should be observable', () => { + const watchdog = Watchdog.for( ClassicTestEditor ); + const states = []; + + watchdog.on( 'change:state', ( evt, propName, newValue ) => { + states.push( newValue ); + } ); + + // sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work. + const originalErrorHandler = window.onerror; + window.onerror = undefined; + + return watchdog.create( element ).then( () => { + return watchdog.create( element ).then( () => { + setTimeout( () => throwCKEditorError( 'foo', watchdog.editor ) ); + setTimeout( () => throwCKEditorError( 'bar', watchdog.editor ) ); + setTimeout( () => throwCKEditorError( 'baz', watchdog.editor ) ); + setTimeout( () => throwCKEditorError( 'biz', watchdog.editor ) ); + + return new Promise( res => { + setTimeout( () => { + window.onerror = originalErrorHandler; + + expect( states ).to.deep.equal( [ + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'crashedPermanently' + ] ); + + watchdog.destroy().then( res ); + } ); + } ); + } ); + } ); + } ); + } ); } ); function throwCKEditorError( name, context ) { From f962d81158ba293b6d035c0b3e825e237ec8339d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 00:43:26 +0200 Subject: [PATCH 04/25] Changed `minNonErrorTimePeriod` to `minimumNonErrorTimePeriod`. --- src/watchdog.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index 061b96d..8faa781 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -25,14 +25,14 @@ export default class Watchdog { /** * @param {Object} [config] The watchdog plugin configuration. * @param {Number} [config.crashNumberLimit=3] A threshold specifying the number of editor errors (defaults to `3`). - * After this limit is reached and the `minNonErrorTimePeriod` is also reached the editor is not restarted + * After this limit is reached and the `minimumNonErrorTimePeriod` is also reached the editor is not restarted * by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. - * @param {Number} [config.minNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. + * @param {Number} [config.minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. * When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the editor is not * restarted by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. * @param {Number} [config.waitingTime=5000] A minimum amount of milliseconds between saving editor data internally. */ - constructor( { crashNumberLimit, minNonErrorTimePeriod, waitingTime } = {} ) { + constructor( { crashNumberLimit, minimumNonErrorTimePeriod, waitingTime } = {} ) { /** * An array of crashes saved as an object with the following properties: * @@ -58,7 +58,7 @@ export default class Watchdog { this.set( 'state', 'initializing' ); /** - * Crash number limit (defaults to `3`). After this limit is reached and the {@link #_minNonErrorTimePeriod} + * Crash number limit (defaults to `3`). After this limit is reached and the {@link #_miimumnNonErrorTimePeriod} * is also reached the editor is not restarted by the watchdog and the watchdog fires * the {@link #crash `crash` event}. This prevents an infinite restart loop. * @@ -73,7 +73,7 @@ export default class Watchdog { * the {@link #crash `crash` event}. This prevents an infinite restart loop. * */ - this._minNonErrorTimePeriod = typeof minNonErrorTimePeriod === 'number' ? minNonErrorTimePeriod : 5000; + this._minimumNonErrorTimePeriod = typeof minimumNonErrorTimePeriod === 'number' ? minimumNonErrorTimePeriod : 5000; /** * Checks if the event error comes from the editor that is handled by the watchdog (by checking the error context) @@ -348,10 +348,12 @@ export default class Watchdog { return true; } - return ( ( - this.crashes[ this.crashes.length - 1 ].date - - this.crashes[ this.crashes.length - 1 - this._crashNumberLimit ].date - ) / this._crashNumberLimit ) > this._minNonErrorTimePeriod; + const lastErrorTime = this.crashes[ this.crashes.length - 1 ].date; + const firstMeaningfulErrorTime = this.crashes[ this.crashes.length - 1 - this._crashNumberLimit ].date; + + const averageNonErrorTimePeriod = ( lastErrorTime - firstMeaningfulErrorTime ) / this._crashNumberLimit; + + return averageNonErrorTimePeriod > this._minimumNonErrorTimePeriod; } /** From 801407ac8640ff585cfc6d89d6487d08482a43c7 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 01:17:09 +0200 Subject: [PATCH 05/25] Fixed the changelog. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7550a91..4fb9e1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,4 +8,4 @@ Internal changes only (updated dependencies, documentation, etc.). ## [10.0.0](https://github.com/ckeditor/ckeditor5-watchdog/tree/v10.0.0) (2019-07-04) -The initial font feature implementation. +The initial watchdog feature implementation. From 719f52e09b01b3f7deb078c4162125b6fb261ec0 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 01:29:59 +0200 Subject: [PATCH 06/25] Fixed tests. --- tests/watchdog.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/watchdog.js b/tests/watchdog.js index 7ac407b..b62c255 100644 --- a/tests/watchdog.js +++ b/tests/watchdog.js @@ -369,7 +369,7 @@ describe( 'Watchdog', () => { } ); it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' + - ' and the average time between errors is lower than `minNonErrorTimePeriod` (default values)', () => { + ' and the average time between errors is lower than `minimumNonErrorTimePeriod` (default values)', () => { const watchdog = new Watchdog(); watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) ); @@ -406,8 +406,8 @@ describe( 'Watchdog', () => { } ); 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 } ); + ' and the average time between errors is lower than `minimumNonErrorTimePeriod` (custom values)', () => { + const watchdog = new Watchdog( { crashNumberLimit: 2, minimumNonErrorTimePeriod: 1000 } ); watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) ); watchdog.setDestructor( editor => editor.destroy() ); @@ -442,8 +442,8 @@ describe( 'Watchdog', () => { } ); } ); - it( 'Watchdog should not crash permantently when average time between errors is longer than `minNonErrorTimePeriod`', () => { - const watchdog = new Watchdog( { crashNumberLimit: 2, minNonErrorTimePeriod: 0 } ); + it( 'Watchdog should not crash permantently when average time between errors is longer than `minimumNonErrorTimePeriod`', () => { + const watchdog = new Watchdog( { crashNumberLimit: 2, minimumNonErrorTimePeriod: 0 } ); watchdog.setCreator( ( el, config ) => ClassicTestEditor.create( el, config ) ); watchdog.setDestructor( editor => editor.destroy() ); @@ -459,10 +459,10 @@ describe( 'Watchdog', () => { 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 ) ); + setTimeout( () => throwCKEditorError( 'foo1', watchdog.editor ), 5 ); + setTimeout( () => throwCKEditorError( 'foo2', watchdog.editor ), 10 ); + setTimeout( () => throwCKEditorError( 'foo3', watchdog.editor ), 15 ); + setTimeout( () => throwCKEditorError( 'foo4', watchdog.editor ), 20 ); return new Promise( res => { setTimeout( () => { @@ -473,7 +473,7 @@ describe( 'Watchdog', () => { window.onerror = originalErrorHandler; watchdog.destroy().then( res ); - } ); + }, 20 ); } ); } ); } ); From 3bf3166541641a8fd261ed7eab4a45995d3ac71d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 11:10:09 +0200 Subject: [PATCH 07/25] Improved manual tests. --- tests/manual/watchdog.html | 2 -- tests/manual/watchdog.js | 50 ++++++++++++++++---------------------- tests/manual/watchdog.md | 4 ++- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/tests/manual/watchdog.html b/tests/manual/watchdog.html index de178c3..896aff5 100644 --- a/tests/manual/watchdog.html +++ b/tests/manual/watchdog.html @@ -1,7 +1,5 @@ - -

Heading 1

Paragraph

diff --git a/tests/manual/watchdog.js b/tests/manual/watchdog.js index b13896e..94f5793 100644 --- a/tests/manual/watchdog.js +++ b/tests/manual/watchdog.js @@ -11,11 +11,6 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import Watchdog from '../../src/watchdog'; -const firstEditorElement = document.getElementById( 'editor-1' ); -const secondEditorElement = document.getElementById( 'editor-2' ); - -const randomErrorButton = document.getElementById( 'random-error' ); - class TypingError { constructor( editor ) { this.editor = editor; @@ -49,36 +44,33 @@ const editorConfig = { } }; -// Watchdog 1 - -const watchdog1 = Watchdog.for( ClassicEditor ); -window.watchdog1 = watchdog1; +const watchdog1 = createWatchdog( document.getElementById( 'editor-1' ), 'First' ); +const watchdog2 = createWatchdog( document.getElementById( 'editor-2' ), 'Second' ); -watchdog1.create( firstEditorElement, editorConfig ); +Object.assign( window, { watchdog1, watchdog2 } ); -watchdog1.on( 'error', () => { - console.log( 'First editor crashed!' ); +document.getElementById( 'random-error' ).addEventListener( 'click', () => { + throw new Error( 'foo' ); } ); -watchdog1.on( 'restart', () => { - console.log( 'First editor restarted.' ); -} ); +function createWatchdog( element, which ) { + const watchdog = Watchdog.for( ClassicEditor ); -// Watchdog 2 + watchdog.create( element, editorConfig ); -const watchdog2 = Watchdog.for( ClassicEditor ); -window.watchdog2 = watchdog2; + watchdog.on( 'error', () => { + console.log( `${ which } editor crashed!` ); + } ); -watchdog2.create( secondEditorElement, editorConfig ); + watchdog.on( 'restart', () => { + console.log( `${ which } editor restarted.` ); + } ); -watchdog2.on( 'error', () => { - console.log( 'Second editor crashed!' ); -} ); + watchdog.on( 'change:state', ( evt, name, currentValue, prevValue ) => { + console.log( `${ which } watchdog changed state from ${ prevValue } to ${ currentValue }` ); -watchdog2.on( 'restart', () => { - console.log( 'Second editor restarted.' ); -} ); - -randomErrorButton.addEventListener( 'click', () => { - throw new Error( 'foo' ); -} ); + if ( currentValue === 'crashedPermanently' ) { + watchdog.editor.isReadOnly = true; + } + } ); +} diff --git a/tests/manual/watchdog.md b/tests/manual/watchdog.md index 8ff5590..259d186 100644 --- a/tests/manual/watchdog.md +++ b/tests/manual/watchdog.md @@ -4,4 +4,6 @@ 1. Click `Simulate a random error` No editor should be restarted. -1. Refresh page and type `1` in the first editor 4 times. The last time the editor should not be restarted. +1. Refresh page and quickly type `1` in the first editor 4 times. After the last error the editor should be crashed and it should not restart. + +1. Refresh page and slowly (slower 1 per 5 seconds) type `1` in the first editor 4 times. After the last error the editor should be restarted and it should still work. From fe6f2a5b85f4413e6a07ece806573b8171bfbc83 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 11:27:42 +0200 Subject: [PATCH 08/25] Allowed `Watchdog.for` to take the watchdog config. --- src/watchdog.js | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index 8faa781..2121677 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -23,16 +23,9 @@ import areConnectedThroughProperties from '@ckeditor/ckeditor5-utils/src/areconn */ export default class Watchdog { /** - * @param {Object} [config] The watchdog plugin configuration. - * @param {Number} [config.crashNumberLimit=3] A threshold specifying the number of editor errors (defaults to `3`). - * After this limit is reached and the `minimumNonErrorTimePeriod` is also reached the editor is not restarted - * by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. - * @param {Number} [config.minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. - * When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the editor is not - * restarted by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. - * @param {Number} [config.waitingTime=5000] A minimum amount of milliseconds between saving editor data internally. + * @param {module:watchdog/watchdog~WatchdogConfig} [config] The watchdog plugin configuration. */ - constructor( { crashNumberLimit, minimumNonErrorTimePeriod, waitingTime } = {} ) { + constructor( config = {} ) { /** * An array of crashes saved as an object with the following properties: * @@ -65,7 +58,7 @@ export default class Watchdog { * @private * @type {Number} */ - this._crashNumberLimit = typeof crashNumberLimit === 'number' ? crashNumberLimit : 3; + this._crashNumberLimit = typeof config.crashNumberLimit === 'number' ? config.crashNumberLimit : 3; /** * Minumum non-error time period (defaults to `5000`). When the period of time between errors is lower than that, @@ -73,7 +66,7 @@ export default class Watchdog { * the {@link #crash `crash` event}. This prevents an infinite restart loop. * */ - this._minimumNonErrorTimePeriod = typeof minimumNonErrorTimePeriod === 'number' ? minimumNonErrorTimePeriod : 5000; + this._minimumNonErrorTimePeriod = typeof config.minimumNonErrorTimePeriod === 'number' ? config.minimumNonErrorTimePeriod : 5000; /** * Checks if the event error comes from the editor that is handled by the watchdog (by checking the error context) @@ -91,7 +84,7 @@ export default class Watchdog { * @private * @type {Function} */ - this._throttledSave = throttle( this._save.bind( this ), waitingTime || 5000 ); + this._throttledSave = throttle( this._save.bind( this ), config.waitingTime || 5000 ); /** * The current editor instance. @@ -409,9 +402,10 @@ export default class Watchdog { * watchdog.create( elementOrData, config ); * * @param {*} Editor The editor class. + * @param {module:watchdog/watchdog~WatchdogConfig} [watchdogConfig] The watchdog plugin configuration. */ - static for( Editor ) { - const watchdog = new Watchdog(); + static for( Editor, watchdogConfig ) { + const watchdog = new Watchdog( watchdogConfig ); watchdog.setCreator( ( elementOrData, config ) => Editor.create( elementOrData, config ) ); watchdog.setDestructor( editor => editor.destroy() ); @@ -434,3 +428,17 @@ export default class Watchdog { } mix( Watchdog, ObservableMixin ); + +/** + * The watchdog plugin configuration. + * + * @typedef {Object} WatchdogConfig + * + * @property {Number} [crashNumberLimit=3] A threshold specifying the number of editor errors (defaults to `3`). + * After this limit is reached and the `minimumNonErrorTimePeriod` is also reached the editor is not restarted + * by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. + * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. + * When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the editor is not + * restarted by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. + * @property {Number} [waitingTime=5000] A minimum amount of milliseconds between saving editor data internally. + */ From bbbb5a02889828974c33dbbb8d8fb31ac0628e36 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 12:21:05 +0200 Subject: [PATCH 09/25] Manual test improvments. --- tests/manual/watchdog.html | 26 ++++++++++++++++++++------ tests/manual/watchdog.js | 29 +++++++++++++++++++++-------- tests/manual/watchdog.md | 2 +- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/tests/manual/watchdog.html b/tests/manual/watchdog.html index 896aff5..3e8270a 100644 --- a/tests/manual/watchdog.html +++ b/tests/manual/watchdog.html @@ -1,11 +1,25 @@ -
-

Heading 1

-

Paragraph

+
+
First editor state:
+ +
+

Heading 1

+

Paragraph

+
-
-

Heading 1

-

Paragraph

+
+
Second editor state:
+ +
+

Heading 1

+

Paragraph

+
+ + diff --git a/tests/manual/watchdog.js b/tests/manual/watchdog.js index 94f5793..9a3b328 100644 --- a/tests/manual/watchdog.js +++ b/tests/manual/watchdog.js @@ -44,8 +44,17 @@ const editorConfig = { } }; -const watchdog1 = createWatchdog( document.getElementById( 'editor-1' ), 'First' ); -const watchdog2 = createWatchdog( document.getElementById( 'editor-2' ), 'Second' ); +const watchdog1 = createWatchdog( + document.getElementById( 'editor-1' ), + document.getElementById( 'editor-1-state' ), + 'First' +); + +const watchdog2 = createWatchdog( + document.getElementById( 'editor-2' ), + document.getElementById( 'editor-2-state' ), + 'Second' +); Object.assign( window, { watchdog1, watchdog2 } ); @@ -53,24 +62,28 @@ document.getElementById( 'random-error' ).addEventListener( 'click', () => { throw new Error( 'foo' ); } ); -function createWatchdog( element, which ) { +function createWatchdog( editorElement, stateElement, name ) { const watchdog = Watchdog.for( ClassicEditor ); - watchdog.create( element, editorConfig ); + watchdog.create( editorElement, editorConfig ); watchdog.on( 'error', () => { - console.log( `${ which } editor crashed!` ); + console.log( `${ name } editor crashed!` ); } ); watchdog.on( 'restart', () => { - console.log( `${ which } editor restarted.` ); + console.log( `${ name } editor restarted.` ); } ); - watchdog.on( 'change:state', ( evt, name, currentValue, prevValue ) => { - console.log( `${ which } watchdog changed state from ${ prevValue } to ${ currentValue }` ); + watchdog.on( 'change:state', ( evt, paramName, currentValue, prevValue ) => { + console.log( `${ name } watchdog changed state from ${ prevValue } to ${ currentValue }` ); + + stateElement.innerText = currentValue; if ( currentValue === 'crashedPermanently' ) { watchdog.editor.isReadOnly = true; } } ); + + stateElement.innerText = watchdog.state; } diff --git a/tests/manual/watchdog.md b/tests/manual/watchdog.md index 259d186..24b395d 100644 --- a/tests/manual/watchdog.md +++ b/tests/manual/watchdog.md @@ -6,4 +6,4 @@ 1. Refresh page and quickly type `1` in the first editor 4 times. After the last error the editor should be crashed and it should not restart. -1. Refresh page and slowly (slower 1 per 5 seconds) type `1` in the first editor 4 times. After the last error the editor should be restarted and it should still work. +1. Refresh page and slowly (slower than 1 per 5 seconds) type `1` in the first editor 4 times. After the last error the editor should be restarted and it should still work. From d3cba5a7c904bd5ac1c6da97920f4f59c0887e4d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 12:21:16 +0200 Subject: [PATCH 10/25] Improved docs. --- docs/features/watchdog.md | 27 +++++++++++++++++++++++++-- src/watchdog.js | 12 ++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index e3dae58..813b13c 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -47,7 +47,7 @@ watchdog.create( document.querySelector( '#editor' ), { In other words, your goal is to create a watchdog instance and make the watchdog create an instance of the editor you want to use. Watchdog will then create a new editor and if it ever crashes restart it by creating a new editor. - A new editor instance is created every time the watchdog detects a crash. Thus, the editor instance should not be kept in your application's state. Use the {@link module:watchdog/watchdog~Watchdog#editor Watchdog#editor`} property instead. + A new editor instance is created every time the watchdog detects a crash. Thus, the editor instance should not be kept in your application's state. Use the {@link module:watchdog/watchdog~Watchdog#editor `Watchdog#editor`} property instead. It also means that any code that should be executed for any new editor instance should be either loaded as an editor plugin or executed in the callbacks defined by {@link module:watchdog/watchdog~Watchdog#setCreator `Watchdog#setCreator()`} and {@link module:watchdog/watchdog~Watchdog#setDestructor `Watchdog#setDestructor()`}. Read more about controlling editor creation/destruction in the next section. @@ -85,7 +85,7 @@ watchdog.create( elementOrData, editorConfig ); Other useful {@link module:watchdog/watchdog~Watchdog methods, properties and events}: - ```js +```js watchdog.on( 'error', () => { console.log( 'Editor crashed.' ) } ); watchdog.on( 'restart', () => { console.log( 'Editor was restarted.' ) } ); @@ -95,9 +95,32 @@ watchdog.destroy(); // The current editor instance. watchdog.editor; +// The current state of the editor. +// The editor might be in one of the following states: +// * `initializing` +// * `ready` +// * `crashed` +// * `crashedPermanently` +// This property is observable. +watchdog.state; + +// Listen to state changes. +watchdog.on( 'change:state' ( evt, name, currentState, prevState ) => { + console.log( `Editor changed from ${ currentState } to ${ prevState }` ); +} ); + +// An array of editor crashes info. watchdog.crashes.forEach( crashInfo => console.log( crashInfo ) ); ``` +### Configuration + +Both, the {@link module:watchdog/watchdog~Watchdog#constructor `Watchdog#constructor`} and the {@link module:watchdog/watchdog~Watchdog.for `Watchdog.for`} methods accept a {{@link module:watchdog/watchdog~WatchdogConfig configuration object} with the following optional properties: + +* `crashNumberLimit` - A threshold specifying the number of editor errors (defaults to `3`). After this limit is reached and the time between last errors is shorter than `minimumNonErrorTimePeriod` the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. +* `minimumNonErrorTimePeriod` - An average amount of milliseconds between last editor errors (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. +* `waitingTime` - A minimum number of milliseconds between saving editor data internally, (defaults to 5000). + ## Limitations The CKEditor 5 watchdog listens to uncaught errors which can be associated with the editor instance created by that watchdog. Currently, these errors are {@link module:utils/ckeditorerror~CKEditorError `CKEditorError` errors} so ones explicitly thrown by the editor (by its internal checks). This means that not every runtime error which crashed the editor can be caught which, in turn, means that not every crash can be detected. diff --git a/src/watchdog.js b/src/watchdog.js index 2121677..e49fedd 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -435,10 +435,10 @@ mix( Watchdog, ObservableMixin ); * @typedef {Object} WatchdogConfig * * @property {Number} [crashNumberLimit=3] A threshold specifying the number of editor errors (defaults to `3`). - * After this limit is reached and the `minimumNonErrorTimePeriod` is also reached the editor is not restarted - * by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. - * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors. - * When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the editor is not - * restarted by the watchdog and the watchdog fires the {@link #crash `crash` event}. This prevents an infinite restart loop. - * @property {Number} [waitingTime=5000] A minimum amount of milliseconds between saving editor data internally. + * After this limit is reached and the time between last errors is shorter than `minimumNonErrorTimePeriod` + * the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. + * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors + * (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached + * the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. + * @property {Number} [waitingTime=5000] A minimum number of milliseconds between saving editor data internally, (defaults to 5000). */ From da2713ef53e62a41f206246ebe6b9ac1010486e6 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 12:24:34 +0200 Subject: [PATCH 11/25] Improved API docs. --- src/watchdog.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index e49fedd..388cc16 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -51,20 +51,16 @@ export default class Watchdog { this.set( 'state', 'initializing' ); /** - * Crash number limit (defaults to `3`). After this limit is reached and the {@link #_miimumnNonErrorTimePeriod} - * is also reached the editor is not restarted by the watchdog and the watchdog fires - * the {@link #crash `crash` event}. This prevents an infinite restart loop. - * * @private * @type {Number} + * @see {module:watchdog/watchdog~WatchdogConfig} */ this._crashNumberLimit = typeof config.crashNumberLimit === 'number' ? config.crashNumberLimit : 3; /** - * Minumum non-error time period (defaults to `5000`). When the period of time between errors is lower than that, - * and the {@link #_crashNumberLimit} is also reached the editor is not restarted by the watchdog and the watchdog fires - * the {@link #crash `crash` event}. This prevents an infinite restart loop. - * + * @private + * @type {Number} + * @see {module:watchdog/watchdog~WatchdogConfig} */ this._minimumNonErrorTimePeriod = typeof config.minimumNonErrorTimePeriod === 'number' ? config.minimumNonErrorTimePeriod : 5000; From 870ad3db985aa4075b9fb0497ffcf07bf8d34a1d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 12:27:51 +0200 Subject: [PATCH 12/25] Improved API docs. --- docs/features/watchdog.md | 8 ++++++++ src/watchdog.js | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 813b13c..d8d9f99 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -121,6 +121,14 @@ Both, the {@link module:watchdog/watchdog~Watchdog#constructor `Watchdog#constru * `minimumNonErrorTimePeriod` - An average amount of milliseconds between last editor errors (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. * `waitingTime` - A minimum number of milliseconds between saving editor data internally, (defaults to 5000). +```js +const watchdog = new Watchdog( { + minimumNonErrorTimePeriod: 2000, + crashNumberLimit: 4, + waitingTime: 1000 +} ) +``` + ## Limitations The CKEditor 5 watchdog listens to uncaught errors which can be associated with the editor instance created by that watchdog. Currently, these errors are {@link module:utils/ckeditorerror~CKEditorError `CKEditorError` errors} so ones explicitly thrown by the editor (by its internal checks). This means that not every runtime error which crashed the editor can be caught which, in turn, means that not every crash can be detected. diff --git a/src/watchdog.js b/src/watchdog.js index 388cc16..b2e9f02 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -53,14 +53,14 @@ export default class Watchdog { /** * @private * @type {Number} - * @see {module:watchdog/watchdog~WatchdogConfig} + * @see module:watchdog/watchdog~WatchdogConfig */ this._crashNumberLimit = typeof config.crashNumberLimit === 'number' ? config.crashNumberLimit : 3; /** * @private * @type {Number} - * @see {module:watchdog/watchdog~WatchdogConfig} + * @see module:watchdog/watchdog~WatchdogConfig */ this._minimumNonErrorTimePeriod = typeof config.minimumNonErrorTimePeriod === 'number' ? config.minimumNonErrorTimePeriod : 5000; From 401de6a54747c83c7fb7722cec5b15416a517f7b Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 12:36:27 +0200 Subject: [PATCH 13/25] Wording. --- src/watchdog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/watchdog.js b/src/watchdog.js index b2e9f02..d4aca33 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -307,7 +307,7 @@ export default class Watchdog { } /** - * Checks whether the evt should be handled. + * Checks whether the error event should be handled. * * @private * @param {Event} evt Error event. From 2c7cabea859b1881ebc96afc30e9944765425402 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 16:57:45 +0200 Subject: [PATCH 14/25] Added a useful example to docs. [skip ci] --- docs/features/watchdog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index d8d9f99..267eb24 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -107,6 +107,10 @@ watchdog.state; // Listen to state changes. watchdog.on( 'change:state' ( evt, name, currentState, prevState ) => { console.log( `Editor changed from ${ currentState } to ${ prevState }` ); + + if ( currentState === 'crashedPermanently' ) { + watchdog.editor.isReadOnly = true; + } } ); // An array of editor crashes info. From f343afa819733a468f1c7b2f362fb675f0591863 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 18:10:50 +0200 Subject: [PATCH 15/25] Updated docs for the `Watchdog#state`. --- src/watchdog.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/watchdog.js b/src/watchdog.js index d4aca33..deeda1d 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -42,7 +42,13 @@ export default class Watchdog { this.crashes = []; /** - * Specifies the watchdog state. + * Specifies the state of the editor handled by the watchdog. The state can be one of the following values: + * + * * `initializing` - before the first initialization, and after crashes, before the editor is ready. + * * `ready` - a state when a user can interact with the editor + * * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPernamently` + * depending on how many and how frequency errors have been caught recently. + * * `crashedPernamently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. * * @public * @observable From d4abb287b91085a648c54e384887c2987463da7d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 18:15:38 +0200 Subject: [PATCH 16/25] Changed `waitingTime` to `saveInterval`. --- docs/features/watchdog.md | 4 ++-- src/watchdog.js | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 267eb24..894276b 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -123,13 +123,13 @@ Both, the {@link module:watchdog/watchdog~Watchdog#constructor `Watchdog#constru * `crashNumberLimit` - A threshold specifying the number of editor errors (defaults to `3`). After this limit is reached and the time between last errors is shorter than `minimumNonErrorTimePeriod` the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. * `minimumNonErrorTimePeriod` - An average amount of milliseconds between last editor errors (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. -* `waitingTime` - A minimum number of milliseconds between saving editor data internally, (defaults to 5000). +* `saveInterval` - A minimum number of milliseconds between saving editor data internally, (defaults to 5000). Note that for large documents this might have an impact on the editor performance. ```js const watchdog = new Watchdog( { minimumNonErrorTimePeriod: 2000, crashNumberLimit: 4, - waitingTime: 1000 + saveInterval: 1000 } ) ``` diff --git a/src/watchdog.js b/src/watchdog.js index deeda1d..f416f32 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -80,13 +80,13 @@ export default class Watchdog { this._boundErrorHandler = this._handleGlobalErrorEvent.bind( this ); /** - * Throttled save method. The `save()` method is called the specified `waitingTime` after `throttledSave()` is called, + * Throttled save method. The `save()` method is called the specified `saveInterval` after `throttledSave()` is called, * unless a new action happens in the meantime. * * @private * @type {Function} */ - this._throttledSave = throttle( this._save.bind( this ), config.waitingTime || 5000 ); + this._throttledSave = throttle( this._save.bind( this ), config.saveInterval || 5000 ); /** * The current editor instance. @@ -442,5 +442,6 @@ mix( Watchdog, ObservableMixin ); * @property {Number} [minimumNonErrorTimePeriod=5000] An average amount of milliseconds between last editor errors * (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached * the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. - * @property {Number} [waitingTime=5000] A minimum number of milliseconds between saving editor data internally, (defaults to 5000). + * @property {Number} [saveInterval=5000] A minimum number of milliseconds between saving editor data internally, (defaults to 5000). + * Note that for large documents this might have an impact on the editor performance. */ From 9f90939fddbc5dc5f58ae5ce9614c2fe31db3048 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 18:17:38 +0200 Subject: [PATCH 17/25] Update docs [skip ci] Co-Authored-By: Piotr Jasiun --- docs/features/watchdog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 894276b..3d14999 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -106,7 +106,7 @@ watchdog.state; // Listen to state changes. watchdog.on( 'change:state' ( evt, name, currentState, prevState ) => { - console.log( `Editor changed from ${ currentState } to ${ prevState }` ); + console.log( `State changed from ${ currentState } to ${ prevState }` ); if ( currentState === 'crashedPermanently' ) { watchdog.editor.isReadOnly = true; From 331cadebf0b84930b0de352fcb1823759a2e06ad Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 1 Aug 2019 18:20:23 +0200 Subject: [PATCH 18/25] Updated API docs. [skip ci] --- docs/features/watchdog.md | 8 ++++---- src/watchdog.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 3d14999..1953dcf 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -97,10 +97,10 @@ watchdog.editor; // The current state of the editor. // The editor might be in one of the following states: -// * `initializing` -// * `ready` -// * `crashed` -// * `crashedPermanently` +// * `initializing` - before the first initialization, and after crashes, before the editor is ready. +// * `ready` - a state when a user can interact with the editor +// * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` depending on how many and how frequency errors have been caught recently. +// * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. // This property is observable. watchdog.state; diff --git a/src/watchdog.js b/src/watchdog.js index f416f32..ba8a0d0 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -46,9 +46,9 @@ export default class Watchdog { * * * `initializing` - before the first initialization, and after crashes, before the editor is ready. * * `ready` - a state when a user can interact with the editor - * * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPernamently` + * * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` * depending on how many and how frequency errors have been caught recently. - * * `crashedPernamently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. + * * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. * * @public * @observable From ded836acbb303b2301dff0829dde0f4112bbd7b4 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 2 Aug 2019 10:01:21 +0200 Subject: [PATCH 19/25] Minor docs fixes. Co-Authored-By: Piotr Jasiun --- docs/features/watchdog.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 1953dcf..893ee58 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -97,9 +97,9 @@ watchdog.editor; // The current state of the editor. // The editor might be in one of the following states: -// * `initializing` - before the first initialization, and after crashes, before the editor is ready. -// * `ready` - a state when a user can interact with the editor -// * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` depending on how many and how frequency errors have been caught recently. +// * `initializing` - before the first initialization, and after crashes, before the editor is ready, +// * `ready` - a state when a user can interact with the editor, +// * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` depending on how many and how frequency errors have been caught recently, // * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. // This property is observable. watchdog.state; From b26d86616f04e23366819fcaf6684975ece626ce Mon Sep 17 00:00:00 2001 From: Piotr Jasiun Date: Fri, 2 Aug 2019 14:27:21 +0200 Subject: [PATCH 20/25] Update docs/features/watchdog.md --- docs/features/watchdog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index 893ee58..d69e7b5 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -123,7 +123,7 @@ Both, the {@link module:watchdog/watchdog~Watchdog#constructor `Watchdog#constru * `crashNumberLimit` - A threshold specifying the number of editor errors (defaults to `3`). After this limit is reached and the time between last errors is shorter than `minimumNonErrorTimePeriod` the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. * `minimumNonErrorTimePeriod` - An average amount of milliseconds between last editor errors (defaults to 5000). When the period of time between errors is lower than that and the `crashNumberLimit` is also reached the watchdog changes its state to `crashedPermanently` and it stops restarting the editor. This prevents an infinite restart loop. -* `saveInterval` - A minimum number of milliseconds between saving editor data internally, (defaults to 5000). Note that for large documents this might have an impact on the editor performance. +* `saveInterval` - A minimum number of milliseconds between saving editor data internally (defaults to 5000). Note that for large documents this might have an impact on the editor performance. ```js const watchdog = new Watchdog( { From e22a1fe0fd9c447dbe8ea820a5044c2d90ff03fe Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 2 Aug 2019 15:57:41 +0200 Subject: [PATCH 21/25] Added the `destroyed` state. --- docs/features/watchdog.md | 3 ++- src/watchdog.js | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/features/watchdog.md b/docs/features/watchdog.md index d69e7b5..3d4116b 100644 --- a/docs/features/watchdog.md +++ b/docs/features/watchdog.md @@ -100,7 +100,8 @@ watchdog.editor; // * `initializing` - before the first initialization, and after crashes, before the editor is ready, // * `ready` - a state when a user can interact with the editor, // * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` depending on how many and how frequency errors have been caught recently, -// * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. +// * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed, +// * `destroyed` - a state when the editor is manually destroyed by the user after calling `watchdog.destroy()`. // This property is observable. watchdog.state; diff --git a/src/watchdog.js b/src/watchdog.js index ba8a0d0..968c70a 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -44,11 +44,12 @@ export default class Watchdog { /** * Specifies the state of the editor handled by the watchdog. The state can be one of the following values: * - * * `initializing` - before the first initialization, and after crashes, before the editor is ready. - * * `ready` - a state when a user can interact with the editor + * * `initializing` - before the first initialization, and after crashes, before the editor is ready, + * * `ready` - a state when a user can interact with the editor, * * `crashed` - a state when an error occurs - it quickly changes to `initializing` or `crashedPermanently` - * depending on how many and how frequency errors have been caught recently. - * * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed. + * depending on how many and how frequency errors have been caught recently, + * * `crashedPermanently` - a state when the watchdog stops reacting to errors and keeps the editor crashed, + * * `destroyed` - a state when the editor is manually destroyed by the user after calling `watchdog.destroy()` * * @public * @observable @@ -242,6 +243,12 @@ export default class Watchdog { * @returns {Promise} */ destroy() { + return this._destroy().then( () => { + this.state = 'destroyed'; + } ); + } + + _destroy() { window.removeEventListener( 'error', this._boundErrorHandler ); this.stopListening( this._editor.model.document, 'change:data', this._throttledSave ); @@ -364,7 +371,7 @@ export default class Watchdog { this._throttledSave.flush(); return Promise.resolve() - .then( () => this.destroy() ) + .then( () => this._destroy() ) .catch( err => console.error( 'An error happened during the editor destructing.', err ) ) .then( () => { if ( typeof this._elementOrData === 'string' ) { From 2400a9ce4dd6e9afab8298573456b450a91e454f Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 2 Aug 2019 16:04:33 +0200 Subject: [PATCH 22/25] Updated tests and docs to the `destroyed` state. --- src/watchdog.js | 8 +++++++- tests/watchdog.js | 41 ++++++++++++++++++++++++----------------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index 968c70a..23bef41 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -238,7 +238,8 @@ export default class Watchdog { } /** - * Destroys the current editor instance by using the destructor passed to the {@link #setDestructor `setDestructor()`} method. + * Destroys the current editor instance by using the destructor passed to the {@link #setDestructor `setDestructor()`} method + * and set state to `destroyed`. * * @returns {Promise} */ @@ -248,6 +249,11 @@ export default class Watchdog { } ); } + /** + * Destroys the current editor instance by using the destructor passed to the {@link #setDestructor `setDestructor()`} method. + * + * @private + */ _destroy() { window.removeEventListener( 'error', this._boundErrorHandler ); this.stopListening( this._editor.model.document, 'change:data', this._throttledSave ); diff --git a/tests/watchdog.js b/tests/watchdog.js index b62c255..c5d80d5 100644 --- a/tests/watchdog.js +++ b/tests/watchdog.js @@ -776,7 +776,11 @@ describe( 'Watchdog', () => { expect( watchdog.state ).to.equal( 'ready' ); - watchdog.destroy().then( res ); + watchdog.destroy().then( () => { + expect( watchdog.state ).to.equal( 'destroyed' ); + + res(); + } ); } ); } ); } ); @@ -806,22 +810,25 @@ describe( 'Watchdog', () => { setTimeout( () => { window.onerror = originalErrorHandler; - expect( states ).to.deep.equal( [ - 'ready', - 'crashed', - 'initializing', - 'ready', - 'crashed', - 'initializing', - 'ready', - 'crashed', - 'initializing', - 'ready', - 'crashed', - 'crashedPermanently' - ] ); - - watchdog.destroy().then( res ); + watchdog.destroy().then( () => { + expect( states ).to.deep.equal( [ + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'initializing', + 'ready', + 'crashed', + 'crashedPermanently', + 'destroyed' + ] ); + + res(); + } ); } ); } ); } ); From dca79c8050bc81ce4a90c81a597b8f7dcf171dc3 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 2 Aug 2019 16:29:08 +0200 Subject: [PATCH 23/25] Updated API docs. Co-Authored-By: Piotr Jasiun --- src/watchdog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/watchdog.js b/src/watchdog.js index 23bef41..9488a2d 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -53,7 +53,7 @@ export default class Watchdog { * * @public * @observable - * @member {'initializing'|'ready'|'crashed'|'crashedPermanently'} #state + * @member {'initializing'|'ready'|'crashed'|'crashedPermanently'|'destroyed'} #state */ this.set( 'state', 'initializing' ); From 08d6125e4288c18c29a7a8657db824690152a075 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 2 Aug 2019 16:31:46 +0200 Subject: [PATCH 24/25] Made the `destroyed` state before destructing. --- src/watchdog.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/watchdog.js b/src/watchdog.js index 9488a2d..f0983c7 100644 --- a/src/watchdog.js +++ b/src/watchdog.js @@ -239,14 +239,14 @@ export default class Watchdog { /** * Destroys the current editor instance by using the destructor passed to the {@link #setDestructor `setDestructor()`} method - * and set state to `destroyed`. + * and sets state to `destroyed`. * * @returns {Promise} */ destroy() { - return this._destroy().then( () => { - this.state = 'destroyed'; - } ); + this.state = 'destroyed'; + + return this._destroy(); } /** From e85adca627e0759042eecdc4609f8211b42969a2 Mon Sep 17 00:00:00 2001 From: Piotr Jasiun Date: Fri, 2 Aug 2019 16:54:40 +0200 Subject: [PATCH 25/25] Tests: fixed manual test. --- tests/manual/watchdog.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/manual/watchdog.js b/tests/manual/watchdog.js index 9a3b328..95b17d4 100644 --- a/tests/manual/watchdog.js +++ b/tests/manual/watchdog.js @@ -86,4 +86,6 @@ function createWatchdog( editorElement, stateElement, name ) { } ); stateElement.innerText = watchdog.state; + + return watchdog; }