From a793bb586d3c1ce8ad468d18d85c8970d2da45a9 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:02:05 +0100 Subject: [PATCH 01/10] Watchdog no longer restarts other editors that might share some configuration or plugins but were not affected by the error. --- .changelog/20251125135803_ck_19407.md | 10 ++ .../ckeditor5-watchdog/src/editorwatchdog.ts | 65 ++++++++++- .../tests/editorwatchdog.js | 107 ++++++++++++++++++ 3 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 .changelog/20251125135803_ck_19407.md diff --git a/.changelog/20251125135803_ck_19407.md b/.changelog/20251125135803_ck_19407.md new file mode 100644 index 00000000000..2851f5e90dc --- /dev/null +++ b/.changelog/20251125135803_ck_19407.md @@ -0,0 +1,10 @@ +--- +type: Fix +scope: + - ckeditor5-watchdog +closes: + - https://github.com/ckeditor/ckeditor5/issues/19407 +--- + +Improved the error detection mechanism in `EditorWatchdog`. It now better identifies which editor instance is responsible for an error, preventing unnecessary restarts of other editors that might share some configuration or plugins but were not affected by the error. + diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index 8979d2fee89..ea38b46330d 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -9,6 +9,7 @@ import { throttle, cloneDeepWith, isElement, type DebouncedFunc } from 'es-toolkit/compat'; import { areConnectedThroughProperties } from './utils/areconnectedthroughproperties.js'; +import { getSubNodes } from './utils/getsubnodes.js'; import { Watchdog, type WatchdogConfig } from './watchdog.js'; import type { CKEditorError } from '@ckeditor/ckeditor5-utils'; import type { ModelNode, ModelText, ModelElement, ModelWriter } from '@ckeditor/ckeditor5-engine'; @@ -432,7 +433,53 @@ export class EditorWatchdog extends Watchdog { * @internal */ public _isErrorComingFromThisItem( error: CKEditorError ): boolean { - return areConnectedThroughProperties( this._editor, error.context, this._excludedProps ); + // If here are no matching properties, the error is definitely not coming from this editor (or any other editor at all). + if ( !areConnectedThroughProperties( this._editor, error.context, this._excludedProps ) ) { + return false; + } + + // ... however, even if there are some matching properties, we still cannot be 100% sure that the error is coming from this editor. + // + // For example, two different editors may share some plugins or other global objects in configuration. While these objects + // are connected through properties, the error may still come from a different editor instance. + // + // To be more certain, we can probe subnodes of the error context and see if they contain any editor-specific objects such as + // the model document, UI view, or editing view. If we find any of these objects, we can be more confident that the error is indeed + // coming from this editor, however, this is still not a 100% guarantee. The exception might happen within context plugins + // that have access to multiple editors. We can group these objects, and if only one of them is found, we can be more certain. + const subNodes = getSubNodes( error.context, this._excludedProps ); + const editorSpecificEntries = getEditorSpecificEntries( this._editor! ); + + let isAssociatedWithThisEditor = false; + let isAssociatedWithOtherEditor = false; + + for ( const node of subNodes ) { + for ( const instance of editorSpecificEntries ) { + const Constructor = instance.constructor; + + if ( node && typeof node === 'object' && node instanceof Constructor ) { + if ( node === instance ) { + isAssociatedWithThisEditor = true; + } else { + isAssociatedWithOtherEditor = true; + } + } + } + } + + if ( isAssociatedWithThisEditor ) { + return true; + } + + if ( isAssociatedWithOtherEditor ) { + return false; + } + + // If no editor-specific objects were found, we cannot be sure whether the error is coming from this editor or not. + // The problem is, that some matching properties was found in the first step, so there is some connection + // between the editor and the error context. If we cannot disprove that the error is coming from this editor, + // we restart all editors that have matching properties. + return true; } /** @@ -628,3 +675,19 @@ export type EditorWatchdogCreatorFunction = ( elementOrData: HTMLElement | string | Record | Record, config: EditorConfig ) => Promise; + +/** + * Returns editor-specific entries that can be used to identify whether an error context is connected to the editor. + */ +function getEditorSpecificEntries( editor: Editor ) { + return [ + editor.conversion, + editor.commands, + editor.plugins, + editor.model.document, + editor.model, + editor.model.schema, + editor.ui?.view, + editor.editing?.view + ].filter( Boolean ); +}; diff --git a/packages/ckeditor5-watchdog/tests/editorwatchdog.js b/packages/ckeditor5-watchdog/tests/editorwatchdog.js index 7822ba8a950..61538f4dd95 100644 --- a/packages/ckeditor5-watchdog/tests/editorwatchdog.js +++ b/packages/ckeditor5-watchdog/tests/editorwatchdog.js @@ -898,6 +898,113 @@ describe( 'EditorWatchdog', () => { } ); } ); + it( 'Watchdog should restart only the relevant editor when the error context contains both shared and editor-specific objects', + async () => { + const watchdog1 = new EditorWatchdog( ClassicTestEditor ); + const watchdog2 = new EditorWatchdog( ClassicTestEditor ); + const element2 = document.createElement( 'div' ); + document.body.appendChild( element2 ); + + // sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work. + const originalErrorHandler = window.onerror; + window.onerror = undefined; + + const sharedObject = { foo: 'bar' }; + + await Promise.all( [ + watchdog1.create( element ), + watchdog2.create( element2 ) + ] ); + + // Attach shared object to both editors so areConnectedThroughProperties returns true for both. + watchdog1.editor.shared = sharedObject; + watchdog2.editor.shared = sharedObject; + + const context = { + shared: sharedObject, + // This makes it specific to watchdog1's editor + specific: watchdog1.editor.model.document + }; + + const watchdog1RestartSpy = sinon.spy(); + const watchdog2RestartSpy = sinon.spy(); + + watchdog1.on( 'restart', watchdog1RestartSpy ); + watchdog2.on( 'restart', watchdog2RestartSpy ); + + setTimeout( () => throwCKEditorError( 'foo', context ) ); + + await new Promise( res => { + watchdog1.on( 'restart', () => { + // Wait a bit to ensure watchdog2 does not restart + setTimeout( () => { + window.onerror = originalErrorHandler; + + sinon.assert.calledOnce( watchdog1RestartSpy ); + sinon.assert.notCalled( watchdog2RestartSpy ); + + Promise.all( [ watchdog1.destroy(), watchdog2.destroy() ] ).then( () => { + element2.remove(); + res(); + } ); + }, 60 ); + } ); + } ); + } + ); + + it( 'Watchdog should restart all editors connected to the error context if no editor-specific object is found', async () => { + const watchdog1 = new EditorWatchdog( ClassicTestEditor ); + const watchdog2 = new EditorWatchdog( ClassicTestEditor ); + const element2 = document.createElement( 'div' ); + document.body.appendChild( element2 ); + + // sinon.stub( window, 'onerror' ).value( undefined ); and similar do not work. + const originalErrorHandler = window.onerror; + window.onerror = undefined; + + const sharedObject = { foo: 'bar' }; + + await Promise.all( [ + watchdog1.create( element ), + watchdog2.create( element2 ) + ] ); + + watchdog1.editor.shared = sharedObject; + watchdog2.editor.shared = sharedObject; + + const context = { + shared: sharedObject + }; + + const watchdog1RestartSpy = sinon.spy(); + const watchdog2RestartSpy = sinon.spy(); + + watchdog1.on( 'restart', watchdog1RestartSpy ); + watchdog2.on( 'restart', watchdog2RestartSpy ); + + setTimeout( () => throwCKEditorError( 'foo', context ) ); + + await new Promise( res => { + let restarts = 0; + const check = () => { + restarts++; + if ( restarts === 2 ) { + window.onerror = originalErrorHandler; + sinon.assert.calledOnce( watchdog1RestartSpy ); + sinon.assert.calledOnce( watchdog2RestartSpy ); + Promise.all( [ watchdog1.destroy(), watchdog2.destroy() ] ).then( () => { + element2.remove(); + res(); + } ); + } + }; + + watchdog1.on( 'restart', check ); + watchdog2.on( 'restart', check ); + } ); + } ); + it( 'Watchdog should crash permanently if the `crashNumberLimit` is reached' + ' and the average time between errors is lower than `minimumNonErrorTimePeriod` (default values)', async () => { const watchdog = new EditorWatchdog( ClassicTestEditor ); From b9617cfb7610aec8b0b3b62212f2e64a54ba94a8 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:02:26 +0100 Subject: [PATCH 02/10] Add context watchdog manual test. --- .../tests/manual/contextwatchdog.html | 24 ++++ .../tests/manual/contextwatchdog.js | 124 ++++++++++++++++++ .../tests/manual/contextwatchdog.md | 7 + 3 files changed, 155 insertions(+) create mode 100644 packages/ckeditor5-watchdog/tests/manual/contextwatchdog.html create mode 100644 packages/ckeditor5-watchdog/tests/manual/contextwatchdog.js create mode 100644 packages/ckeditor5-watchdog/tests/manual/contextwatchdog.md diff --git a/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.html b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.html new file mode 100644 index 00000000000..9686e31ee34 --- /dev/null +++ b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.html @@ -0,0 +1,24 @@ + + + +
Context state:
+ +
+
+

Heading 1

+

Paragraph

+
+
+ +
+
+

Heading 1

+

Paragraph

+
+
+ + diff --git a/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.js b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.js new file mode 100644 index 00000000000..7359e360440 --- /dev/null +++ b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.js @@ -0,0 +1,124 @@ +/** + * @license Copyright (c) 2003-2025, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-licensing-options + */ + +import { Context, ContextPlugin } from '@ckeditor/ckeditor5-core'; +import { ClassicEditor } from '@ckeditor/ckeditor5-editor-classic'; +import { ArticlePluginSet } from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset.js'; +import { CKEditorError } from '@ckeditor/ckeditor5-utils'; + +import { ContextWatchdog } from '../../src/contextwatchdog.js'; + +const stateElement = document.getElementById( 'context-state' ); + +class ContextErrorPlugin extends ContextPlugin { + init() { + window.contextErrorPlugin = this; + } + + throwError() { + setTimeout( () => { + throw new CKEditorError( 'context-error', this.context ); + } ); + } +} + +class TypingError { + constructor( editor ) { + this.editor = editor; + } + + init() { + const inputCommand = this.editor.commands.get( 'input' ); + + inputCommand.on( 'execute', ( evt, data ) => { + const commandArgs = data[ 0 ]; + + if ( commandArgs.text === '1' ) { + // Simulate error. + this.editor.foo.bar = 'bom'; + } + } ); + } +} + +const watchdog = new ContextWatchdog( Context ); + +window.watchdog = watchdog; + +const contextConfig = { + plugins: [ ContextErrorPlugin ] +}; + +watchdog.create( contextConfig ) + .then( () => { + return Promise.all( [ + watchdog.add( { + id: 'editor1', + type: 'editor', + creator: async ( element, config ) => { + const editor1 = await ClassicEditor.create( element, config ); + + window.editor1 = editor1; + console.log( 'Created editor1!', editor1 ); + + return editor1; + }, + sourceElementOrData: document.getElementById( 'editor-1' ), + config: { + plugins: [ ArticlePluginSet, TypingError ], + image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, + toolbar: [ 'heading', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ] + } + } ), + watchdog.add( { + id: 'editor2', + type: 'editor', + creator: async ( element, config ) => { + const editor2 = await ClassicEditor.create( element, config ); + + window.editor2 = editor2; + console.log( 'Created editor2!', editor2 ); + + return editor2; + }, + sourceElementOrData: document.getElementById( 'editor-2' ), + config: { + plugins: [ ArticlePluginSet, TypingError ], + image: { toolbar: [ 'toggleImageCaption', 'imageTextAlternative' ] }, + toolbar: [ 'heading', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ] + } + } ) + ] ); + } ); + +watchdog.on( 'itemError', ( evt, { itemId, error } ) => { + console.log( `Context watchdog item (${ itemId }) error:`, error ); +} ); + +watchdog.on( 'error', ( evt, { error } ) => { + console.log( 'Context watchdog error:', error ); +} ); + +watchdog.on( 'restart', () => { + console.log( 'Context watchdog restarted.' ); +} ); + +watchdog.on( 'stateChange', () => { + console.log( `Context watchdog state changed to '${ watchdog.state }'` ); + + stateElement.innerText = watchdog.state; +} ); + +stateElement.innerText = watchdog.state; + +document.getElementById( 'context-error' ).addEventListener( 'click', () => { + if ( window.contextErrorPlugin ) { + window.contextErrorPlugin.throwError(); + } +} ); + +document.getElementById( 'random-error' ).addEventListener( 'click', () => { + throw new Error( 'foo' ); +} ); diff --git a/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.md b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.md new file mode 100644 index 00000000000..75abe29e50f --- /dev/null +++ b/packages/ckeditor5-watchdog/tests/manual/contextwatchdog.md @@ -0,0 +1,7 @@ +# ContextWatchdog manual test + +1. Click `Simulate Error in Context Plugin`. The context and both editors should crash and be restarted. The error should be logged in the console. + +2. Click `Simulate a random error`. No editor should be restarted. + +3. Refresh page and quickly click `Simulate Error in Context Plugin` 4 times. After the last error the watchdog should be crashed permanently and it should not restart. From 7ce5497260e910b1cc7089554f058aa865c588d2 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:03:51 +0100 Subject: [PATCH 03/10] Fix EOF --- .changelog/20251125135803_ck_19407.md | 1 - 1 file changed, 1 deletion(-) diff --git a/.changelog/20251125135803_ck_19407.md b/.changelog/20251125135803_ck_19407.md index 2851f5e90dc..15e5d9949f7 100644 --- a/.changelog/20251125135803_ck_19407.md +++ b/.changelog/20251125135803_ck_19407.md @@ -7,4 +7,3 @@ closes: --- Improved the error detection mechanism in `EditorWatchdog`. It now better identifies which editor instance is responsible for an error, preventing unnecessary restarts of other editors that might share some configuration or plugins but were not affected by the error. - From 797edfbaa8c5653c063dae2dc995a4419346213b Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:05:26 +0100 Subject: [PATCH 04/10] Adjust comment. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index ea38b46330d..30fa91363a9 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -446,7 +446,7 @@ export class EditorWatchdog extends Watchdog { // To be more certain, we can probe subnodes of the error context and see if they contain any editor-specific objects such as // the model document, UI view, or editing view. If we find any of these objects, we can be more confident that the error is indeed // coming from this editor, however, this is still not a 100% guarantee. The exception might happen within context plugins - // that have access to multiple editors. We can group these objects, and if only one of them is found, we can be more certain. + // that have access to multiple editors. const subNodes = getSubNodes( error.context, this._excludedProps ); const editorSpecificEntries = getEditorSpecificEntries( this._editor! ); From 06fe4e12e332a9e0ea20acc85631322d63fc2ddc Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:06:28 +0100 Subject: [PATCH 05/10] Adjust comment. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index 30fa91363a9..0443e6574ec 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -476,9 +476,9 @@ export class EditorWatchdog extends Watchdog { } // If no editor-specific objects were found, we cannot be sure whether the error is coming from this editor or not. - // The problem is, that some matching properties was found in the first step, so there is some connection - // between the editor and the error context. If we cannot disprove that the error is coming from this editor, - // we restart all editors that have matching properties. + // The problem is, that some matching properties were found in the first step, so there is some connection + // between the editor and the error context. It's safer to restart all associated editors than risk not restarting + // an editor that is actually broken. return true; } From 5b8fb5f74103e67747ef8a1cdf91d0e62567ab93 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:19:26 +0100 Subject: [PATCH 06/10] Extend probe. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index 0443e6574ec..502a7c9323b 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -681,6 +681,9 @@ export type EditorWatchdogCreatorFunction = ( */ function getEditorSpecificEntries( editor: Editor ) { return [ + editor.data, + editor.plugins, + editor.config, editor.conversion, editor.commands, editor.plugins, From 67a9767d1d8c9ad7860fe32ad07d12f6c3729831 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:20:59 +0100 Subject: [PATCH 07/10] Adjust asseration. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index 502a7c9323b..c716a0df47c 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -434,7 +434,7 @@ export class EditorWatchdog extends Watchdog { */ public _isErrorComingFromThisItem( error: CKEditorError ): boolean { // If here are no matching properties, the error is definitely not coming from this editor (or any other editor at all). - if ( !areConnectedThroughProperties( this._editor, error.context, this._excludedProps ) ) { + if ( !this._editor || !areConnectedThroughProperties( this._editor, error.context, this._excludedProps ) ) { return false; } @@ -448,7 +448,7 @@ export class EditorWatchdog extends Watchdog { // coming from this editor, however, this is still not a 100% guarantee. The exception might happen within context plugins // that have access to multiple editors. const subNodes = getSubNodes( error.context, this._excludedProps ); - const editorSpecificEntries = getEditorSpecificEntries( this._editor! ); + const editorSpecificEntries = getEditorSpecificEntries( this._editor ); let isAssociatedWithThisEditor = false; let isAssociatedWithOtherEditor = false; From a684de25b3594c1c8a4451c07e748425b6c69cb1 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 25 Nov 2025 14:29:46 +0100 Subject: [PATCH 08/10] Simplify expresion. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index c716a0df47c..81d817e15e6 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -455,9 +455,7 @@ export class EditorWatchdog extends Watchdog { for ( const node of subNodes ) { for ( const instance of editorSpecificEntries ) { - const Constructor = instance.constructor; - - if ( node && typeof node === 'object' && node instanceof Constructor ) { + if ( node && typeof node === 'object' && node instanceof instance.constructor ) { if ( node === instance ) { isAssociatedWithThisEditor = true; } else { From 595d286f01e4779a0bb557375fbd660edcd0ce6a Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 1 Dec 2025 07:36:38 +0100 Subject: [PATCH 09/10] Extend editor specific variables array. --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index 81d817e15e6..e0241bdfe3e 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -685,10 +685,14 @@ function getEditorSpecificEntries( editor: Editor ) { editor.conversion, editor.commands, editor.plugins, - editor.model.document, + editor.keystrokes, editor.model, - editor.model.schema, + editor.model?.document, + editor.model?.schema, + editor.ui, editor.ui?.view, - editor.editing?.view + editor.editing, + editor.editing?.view, + editor.editing?.mapper ].filter( Boolean ); }; From 48ca81775fcf35e62bac13ac23b16398c6838df1 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Mon, 1 Dec 2025 07:48:57 +0100 Subject: [PATCH 10/10] Extend list --- packages/ckeditor5-watchdog/src/editorwatchdog.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/ckeditor5-watchdog/src/editorwatchdog.ts b/packages/ckeditor5-watchdog/src/editorwatchdog.ts index e0241bdfe3e..33ff37509f1 100644 --- a/packages/ckeditor5-watchdog/src/editorwatchdog.ts +++ b/packages/ckeditor5-watchdog/src/editorwatchdog.ts @@ -679,6 +679,8 @@ export type EditorWatchdogCreatorFunction = ( */ function getEditorSpecificEntries( editor: Editor ) { return [ + editor.locale, + editor.accessibility, editor.data, editor.plugins, editor.config,