Merge pull request #908 from ckeditor/t/906

Fix: Removed invalid promise catches from `dev-utils.DeltaReplayer`. Closes #906.

src/dev-utils/deltareplayer.js

@@ -7,7 +7,7 @@
  * @module engine/dev-utils/deltareplayer
  */
 
-/* global setTimeout, console */
+/* global setTimeout */
 
 import DeltaFactory from '../model/delta/deltafactory';
 
@@ -16,7 +16,7 @@ import DeltaFactory from '../model/delta/deltafactory';
  */
 export default class DeltaReplayer {
 	/**
-	 * @param {module:engine/model/document~Document} document Document to reply deltas on.
+	 * @param {module:engine/model/document~Document} document Document to replay deltas on.
 	 * @param {String} logSeparator Separator between deltas.
 	 * @param {String} stringifiedDeltas Deltas to replay.
 	 */
@@ -27,7 +27,7 @@ export default class DeltaReplayer {
 	}
 
 	/**
-	 * Parses given string containing stringified deltas and sets parsed deltas as deltas to reply.
+	 * Parses given string containing stringified deltas and sets parsed deltas as deltas to replay.
 	 *
 	 * @param {String} stringifiedDeltas Stringified deltas to replay.
 	 */
@@ -44,7 +44,7 @@ export default class DeltaReplayer {
 	}
 
 	/**
-	 * Returns deltas to reply.
+	 * Returns deltas to replay.
 	 *
 	 * @returns {Array.<module:engine/model/delta/delta~Delta>}
 	 */
@@ -61,17 +61,19 @@ export default class DeltaReplayer {
 	play( timeInterval = 1000 ) {
 		const deltaReplayer = this;
 
-		return new Promise( ( res ) => {
+		return new Promise( ( res, rej ) => {
 			play();
 
 			function play() {
-				if ( deltaReplayer._deltasToReplay.length === 0 ) {
-					return res();
-				}
+				deltaReplayer.applyNextDelta().then( ( isFinished ) => {
+					if ( isFinished ) {
+						return res();
+					}
 
-				deltaReplayer.applyNextDelta().then( () => {
 					setTimeout( play, timeInterval );
-				}, res );
+				} ).catch( ( err ) => {
+					rej( err );
+				} );
 			}
 		} );
 	}
@@ -88,8 +90,11 @@ export default class DeltaReplayer {
 		}
 
 		return this.applyNextDelta()
-			.then( () => this.applyDeltas( numberOfDeltas - 1 ) )
-			.catch( err => console.warn( err ) );
+			.then( ( isFinished ) => {
+				if ( !isFinished ) {
+					return this.applyDeltas( numberOfDeltas - 1 );
+				}
+			} );
 	}
 
 	/**
@@ -99,24 +104,28 @@ export default class DeltaReplayer {
 	 */
 	applyAllDeltas() {
 		return this.applyNextDelta()
-			.then( () => this.applyAllDeltas() )
-			.catch( () => {} );
+			.then( ( isFinished ) => {
+				if ( !isFinished ) {
+					return this.applyAllDeltas();
+				}
+			} );
 	}
 
 	/**
-	 * Applies the next delta to replay.
+	 * Applies the next delta to replay. Returns promise with `isFinished` parameter that is `true` if the last
+	 * delta in replayer has been applied, `false` otherwise.
 	 *
-	 * @returns {Promise}
+	 * @returns {Promise.<Boolean>}
 	 */
 	applyNextDelta() {
 		const document = this._document;
 
-		return new Promise( ( res, rej ) => {
+		return new Promise( ( res ) => {
 			document.enqueueChanges( () => {
 				const jsonDelta = this._deltasToReplay.shift();
 
 				if ( !jsonDelta ) {
-					return rej( new Error( 'No deltas to replay' ) );
+					return res( true );
 				}
 
 				const delta = DeltaFactory.fromJSON( jsonDelta, this._document );
@@ -128,7 +137,7 @@ export default class DeltaReplayer {
 					document.applyOperation( operation );
 				}
 
-				res();
+				res( false );
 			} );
 		} );
 	}

tests/dev-utils/deltareplayer.js

@@ -3,25 +3,10 @@
  * For licensing, see LICENSE.md.
  */
 
-/* global console */
-
 import DeltaReplayer from '../../src/dev-utils/deltareplayer';
 import Document from '../../src/model/document';
 
 describe( 'DeltaReplayer', () => {
-	const sandbox = sinon.sandbox.create();
-	let stubs;
-
-	beforeEach( () => {
-		stubs = {
-			consoleWarn: sandbox.stub( console, 'warn' ),
-		};
-	} );
-
-	afterEach( () => {
-		sandbox.restore();
-	} );
-
 	describe( 'constructor()', () => {
 		it( 'should be able to initialize replayer without deltas', () => {
 			const doc = getDocument();
@@ -48,8 +33,9 @@ describe( 'DeltaReplayer', () => {
 
 			const deltaReplayer = new DeltaReplayer( doc, '---', JSON.stringify( delta ) );
 
-			return deltaReplayer.applyNextDelta().then( () => {
+			return deltaReplayer.applyNextDelta().then( ( isFinished ) => {
 				expect( deltaReplayer.getDeltasToReplay() ).to.deep.equal( [] );
+				expect( isFinished ).to.equal( false );
 			} );
 		} );
 
@@ -64,15 +50,12 @@ describe( 'DeltaReplayer', () => {
 			} );
 		} );
 
-		it( 'should throw an error if 0 deltas are provided', () => {
+		it( 'should resolve with true if 0 deltas are provided', () => {
 			const doc = getDocument();
 			const deltaReplayer = new DeltaReplayer( doc, '---', '' );
 
-			return deltaReplayer.applyNextDelta().then( () => {
-				throw new Error( 'This should throw an error' );
-			}, ( err ) => {
-				expect( err instanceof Error ).to.equal( true );
-				expect( err.message ).to.equal( 'No deltas to replay' );
+			return deltaReplayer.applyNextDelta().then( ( isFinished ) => {
+				expect( isFinished ).to.equal( true );
 			} );
 		} );
 	} );
@@ -122,7 +105,6 @@ describe( 'DeltaReplayer', () => {
 			return deltaReplayer.applyDeltas( 3 ).then( () => {
 				expect( Array.from( doc.getRoot().getChildren() ).length ).to.equal( 2 );
 				expect( deltaReplayer.getDeltasToReplay().length ).to.equal( 0 );
-				sinon.assert.calledWithExactly( stubs.consoleWarn, new Error( 'No deltas to replay' ) );
 			} );
 		} );
 	} );
@@ -149,6 +131,26 @@ describe( 'DeltaReplayer', () => {
 
 			return deltaReplayer.play();
 		} );
+
+		it( 'should correctly handle errors coming from the engine', () => {
+			const doc = getDocument();
+
+			const invalidDelta = getSecondDelta();
+			invalidDelta.operations[ 0 ].baseVersion = 3;
+
+			const stringifiedDeltas = [ getFirstDelta(), invalidDelta ]
+				.map( d => JSON.stringify( d ) )
+				.join( '---' );
+
+			const deltaReplayer = new DeltaReplayer( doc, '---', stringifiedDeltas );
+
+			return deltaReplayer.play( 1 )
+				.then( () => {
+					throw new Error( 'It should throw an error' );
+				}, ( err ) => {
+					expect( err.message ).to.match( /^model-document-applyOperation-wrong-version:/ );
+				} );
+		} );
 	} );
 } );