Merge pull request #308 from ckeditor/t/ckeditor5/1304

Other: Introduced the `CKEditorError.rethrowUnexpectedError()` helper. Added custom error handling for the `Emitter#fire()` function. Part of ckeditor/ckeditor5#1304.

src/ckeditorerror.js

@@ -91,6 +91,38 @@ export default class CKEditorError extends Error {
 	is( type ) {
 		return type === 'CKEditorError';
 	}
+
+	/**
+	 * A utility that ensures the the thrown error is a {@link utils/ckeditorerror~CKEditorError} one.
+	 * It is uesful when combined with the {@link watchdog/watchdog~Watchdog} feature, which can restart the editor in case
+	 * of a {@link utils/ckeditorerror~CKEditorError} error.
+	 *
+	 * @param {Error} err An error.
+	 * @param {Object} context An object conected through properties with the editor instance. This context will be used
+	 * by the watchdog to verify which editor should be restarted.
+	 */
+	static rethrowUnexpectedError( err, context ) {
+		if ( err.is && err.is( 'CKEditorError' ) ) {
+			throw err;
+		}
+
+		/**
+		 * An unexpected error occurred inside the CKEditor 5 codebase. The `error.data.originalError` property
+		 * shows the original error properties.
+		 *
+		 * This error is only useful when the editor is initialized using the {@link watchdog/watchdog~Watchdog} feature.
+		 * In case of such error (or any {@link utils/ckeditorerror~CKEditorError} error) the wathcdog should restart the editor.
+		 *
+		 * @error unexpected-error
+		 */
+		throw new CKEditorError( 'unexpected-error', context, {
+			originalError: {
+				message: err.message,
+				stack: err.stack,
+				name: err.name
+			}
+		} );
+	}
 }
 
 /**

src/emittermixin.js

@@ -13,6 +13,7 @@ import priorities from './priorities';
 
 // To check if component is loaded more than once.
 import './version';
+import CKEditorError from './ckeditorerror';
 
 const _listeningTo = Symbol( 'listeningTo' );
 const _emitterId = Symbol( 'emitterId' );
@@ -184,58 +185,62 @@ const EmitterMixin = {
 	 * @inheritDoc
 	 */
 	fire( eventOrInfo, ...args ) {
-		const eventInfo = eventOrInfo instanceof EventInfo ? eventOrInfo : new EventInfo( this, eventOrInfo );
-		const event = eventInfo.name;
-		let callbacks = getCallbacksForEvent( this, event );
-
-		// Record that the event passed this emitter on its path.
-		eventInfo.path.push( this );
-
-		// Handle event listener callbacks first.
-		if ( callbacks ) {
-			// Arguments passed to each callback.
-			const callbackArgs = [ eventInfo, ...args ];
-
-			// Copying callbacks array is the easiest and most secure way of preventing infinite loops, when event callbacks
-			// are added while processing other callbacks. Previous solution involved adding counters (unique ids) but
-			// failed if callbacks were added to the queue before currently processed callback.
-			// If this proves to be too inefficient, another method is to change `.on()` so callbacks are stored if same
-			// event is currently processed. Then, `.fire()` at the end, would have to add all stored events.
-			callbacks = Array.from( callbacks );
+		try {
+			const eventInfo = eventOrInfo instanceof EventInfo ? eventOrInfo : new EventInfo( this, eventOrInfo );
+			const event = eventInfo.name;
+			let callbacks = getCallbacksForEvent( this, event );
+
+			// Record that the event passed this emitter on its path.
+			eventInfo.path.push( this );
+
+			// Handle event listener callbacks first.
+			if ( callbacks ) {
+				// Arguments passed to each callback.
+				const callbackArgs = [ eventInfo, ...args ];
+
+				// Copying callbacks array is the easiest and most secure way of preventing infinite loops, when event callbacks
+				// are added while processing other callbacks. Previous solution involved adding counters (unique ids) but
+				// failed if callbacks were added to the queue before currently processed callback.
+				// If this proves to be too inefficient, another method is to change `.on()` so callbacks are stored if same
+				// event is currently processed. Then, `.fire()` at the end, would have to add all stored events.
+				callbacks = Array.from( callbacks );
+
+				for ( let i = 0; i < callbacks.length; i++ ) {
+					callbacks[ i ].callback.apply( this, callbackArgs );
+
+					// Remove the callback from future requests if off() has been called.
+					if ( eventInfo.off.called ) {
+						// Remove the called mark for the next calls.
+						delete eventInfo.off.called;
+
+						removeCallback( this, event, callbacks[ i ].callback );
+					}
 
-			for ( let i = 0; i < callbacks.length; i++ ) {
-				callbacks[ i ].callback.apply( this, callbackArgs );
+					// Do not execute next callbacks if stop() was called.
+					if ( eventInfo.stop.called ) {
+						break;
+					}
+				}
+			}
 
-				// Remove the callback from future requests if off() has been called.
-				if ( eventInfo.off.called ) {
-					// Remove the called mark for the next calls.
-					delete eventInfo.off.called;
+			// Delegate event to other emitters if needed.
+			if ( this._delegations ) {
+				const destinations = this._delegations.get( event );
+				const passAllDestinations = this._delegations.get( '*' );
 
-					removeCallback( this, event, callbacks[ i ].callback );
+				if ( destinations ) {
+					fireDelegatedEvents( destinations, eventInfo, args );
 				}
 
-				// Do not execute next callbacks if stop() was called.
-				if ( eventInfo.stop.called ) {
-					break;
+				if ( passAllDestinations ) {
+					fireDelegatedEvents( passAllDestinations, eventInfo, args );
 				}
 			}
-		}
-
-		// Delegate event to other emitters if needed.
-		if ( this._delegations ) {
-			const destinations = this._delegations.get( event );
-			const passAllDestinations = this._delegations.get( '*' );
-
-			if ( destinations ) {
-				fireDelegatedEvents( destinations, eventInfo, args );
-			}
 
-			if ( passAllDestinations ) {
-				fireDelegatedEvents( passAllDestinations, eventInfo, args );
-			}
+			return eventInfo.return;
+		} catch ( err ) {
+			CKEditorError.rethrowUnexpectedError( err, this );
 		}
-
-		return eventInfo.return;
 	},
 
 	/**

tests/ckeditorerror.js

@@ -4,6 +4,7 @@
  */
 
 import { default as CKEditorError, DOCUMENTATION_URL } from '../src/ckeditorerror';
+import { expectToThrowCKEditorError } from './_utils/utils';
 
 describe( 'CKEditorError', () => {
 	it( 'inherits from Error', () => {
@@ -88,4 +89,30 @@ describe( 'CKEditorError', () => {
 			expect( ( !!regularError.is && regularError.is( 'CKEditorError' ) ) ).to.be.false;
 		} );
 	} );
+
+	describe( 'static rethrowUnexpectedError()', () => {
+		it( 'should rethrow the original CKEditorError as it is', () => {
+			const ckeditorError = new CKEditorError( 'foo', null );
+
+			expectToThrowCKEditorError( () => {
+				CKEditorError.rethrowUnexpectedError( ckeditorError, {} );
+			}, /foo/, null );
+		} );
+
+		it( 'should rethrow an unexpected error wrapped in CKEditorError', () => {
+			const error = new Error( 'foo' );
+			error.stack = 'bar';
+			const context = {};
+
+			expectToThrowCKEditorError( () => {
+				CKEditorError.rethrowUnexpectedError( error, context );
+			}, /unexpected-error/, context, {
+				originalError: {
+					message: 'foo',
+					stack: 'bar',
+					name: 'Error'
+				}
+			} );
+		} );
+	} );
 } );

tests/emittermixin.js

@@ -5,6 +5,8 @@
 
 import { default as EmitterMixin, _getEmitterListenedTo, _getEmitterId, _setEmitterId } from '../src/emittermixin';
 import EventInfo from '../src/eventinfo';
+import { expectToThrowCKEditorError } from './_utils/utils';
+import CKEditorError from '../src/ckeditorerror';
 
 describe( 'EmitterMixin', () => {
 	let emitter, listener;
@@ -154,6 +156,35 @@ describe( 'EmitterMixin', () => {
 			sinon.assert.calledThrice( spyFoo2 );
 		} );
 
+		it( 'should rethrow the CKEditorError error', () => {
+			emitter.on( 'test', () => {
+				throw new CKEditorError( 'Foo', null );
+			} );
+
+			expectToThrowCKEditorError( () => {
+				emitter.fire( 'test' );
+			}, /Foo/, null );
+		} );
+
+		it( 'should wrap an error into the CKEditorError if a native error was thrown', () => {
+			const error = new Error( 'foo' );
+			error.stack = 'bar';
+
+			emitter.on( 'test', () => {
+				throw error;
+			} );
+
+			expectToThrowCKEditorError( () => {
+				emitter.fire( 'test' );
+			}, /unexpected-error/, emitter, {
+				originalError: {
+					message: 'foo',
+					stack: 'bar',
+					name: 'Error'
+				}
+			} );
+		} );
+
 		describe( 'return value', () => {
 			it( 'is undefined by default', () => {
 				expect( emitter.fire( 'foo' ) ).to.be.undefined;