From 493a826e446a3d5915e847910fed58f27290c5bf Mon Sep 17 00:00:00 2001
From: Darren Ethier <darren@roughsmootheng.in>
Date: Mon, 20 Apr 2020 22:33:16 -0400
Subject: [PATCH] modify emitters so it handles non object or invalid object
 responses correctly.
 (https://github.com/woocommerce/woocommerce-blocks/pull/2249)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With these changes:

- If an observer returns an object wthout a type property an error is thrown and the emitter is aborted with an error type response.
- For anything else returned from an observer, it’s discarded.

Tests are updated for the new expectations
---
 .../cart-checkout/event-emit/emitters.js      | 17 ++++-
 .../cart-checkout/event-emit/test/emitters.js | 70 ++++++++++++++-----
 2 files changed, 65 insertions(+), 22 deletions(-)

diff --git a/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/emitters.js b/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/emitters.js
index 1a588f30760..67a418a9ab2 100644
--- a/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/emitters.js
+++ b/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/emitters.js
@@ -58,18 +58,29 @@ export const emitEvent = async ( observers, eventType, data ) => {
  */
 export const emitEventWithAbort = async ( observers, eventType, data ) => {
 	const observersByType = getObserversByPriority( observers, eventType );
+	let emitterResponse = true;
 	for ( const observer of observersByType ) {
 		try {
 			const response = await Promise.resolve( observer.callback( data ) );
-			if ( response !== true ) {
-				return response;
+			if (
+				typeof response === 'object' &&
+				typeof response.type === 'undefined'
+			) {
+				throw new Error(
+					'If you want to abort event emitter processing, your observer must return an object with a type property'
+				);
+			}
+			emitterResponse = typeof response === 'object' ? response : true;
+			if ( emitterResponse !== true ) {
+				return emitterResponse;
 			}
 		} catch ( e ) {
 			// we don't handle thrown errors but just console.log for
 			// troubleshooting
 			// eslint-disable-next-line no-console
 			console.error( e );
+			return { type: 'error' };
 		}
 	}
-	return true;
+	return emitterResponse;
 };
diff --git a/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/test/emitters.js b/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/test/emitters.js
index c3ebdfe33c6..9739b80ed98 100644
--- a/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/test/emitters.js
+++ b/plugins/woocommerce-blocks/assets/js/base/context/cart-checkout/event-emit/test/emitters.js
@@ -43,29 +43,61 @@ describe( 'Testing emitters', () => {
 		} );
 	} );
 	describe( 'Testing emitEventWithAbort()', () => {
-		it(
-			'aborts on non truthy value and does not invoke remaining ' +
-				'observers',
-			async () => {
-				const observers = { test: observerMocks };
-				const response = await emitEventWithAbort(
-					observers,
-					'test',
-					'foo'
-				);
-				expect( console ).not.toHaveErrored();
-				expect( observerB ).toHaveBeenCalledTimes( 1 );
-				expect(
-					observerPromiseWithResolvedValue
-				).not.toHaveBeenCalled();
-				expect( response ).toBe( 10 );
-			}
-		);
+		it( 'does not abort on any return value other than an object with a type property', async () => {
+			observerMocks.delete( 'observerPromiseWithReject' );
+			const observers = { test: observerMocks };
+			const response = await emitEventWithAbort(
+				observers,
+				'test',
+				'foo'
+			);
+			expect( console ).not.toHaveErrored();
+			expect( observerB ).toHaveBeenCalledTimes( 1 );
+			expect( observerPromiseWithResolvedValue ).toHaveBeenCalled();
+			expect( response ).toBe( true );
+		} );
+		it( 'Aborts on a return value with an object that has a type property', async () => {
+			const validObjectResponse = jest
+				.fn()
+				.mockReturnValue( { type: 'success' } );
+			observerMocks.set( 'observerValidObject', {
+				priority: 5,
+				callback: validObjectResponse,
+			} );
+			observerMocks.delete( 'observerPromiseWithReject' );
+			const observers = { test: observerMocks };
+			const response = await emitEventWithAbort(
+				observers,
+				'test',
+				'foo'
+			);
+			expect( console ).not.toHaveErrored();
+			expect( validObjectResponse ).toHaveBeenCalledTimes( 1 );
+			expect( observerPromiseWithResolvedValue ).not.toHaveBeenCalled();
+			expect( response ).toEqual( { type: 'success' } );
+		} );
+		it( 'throws an error on an object returned from observer without a type property', async () => {
+			const failingObjectResponse = jest.fn().mockReturnValue( {} );
+			observerMocks.set( 'observerInvalidObject', {
+				priority: 5,
+				callback: failingObjectResponse,
+			} );
+			const observers = { test: observerMocks };
+			const response = await emitEventWithAbort(
+				observers,
+				'test',
+				'foo'
+			);
+			expect( console ).toHaveErrored();
+			expect( failingObjectResponse ).toHaveBeenCalledTimes( 1 );
+			expect( observerPromiseWithResolvedValue ).not.toHaveBeenCalled();
+			expect( response ).toEqual( { type: 'error' } );
+		} );
 	} );
 	describe( 'Test Priority', () => {
 		it( 'executes observers in expected order by priority', async () => {
 			const a = jest.fn();
-			const b = jest.fn().mockReturnValue( false );
+			const b = jest.fn().mockReturnValue( { type: 'error' } );
 			const observers = {
 				test: new Map( [
 					[ 'observerA', { priority: 200, callback: a } ],