Add stricter observer type checks in payments thunk to improve resilience to bad observer responses (https://github.com/woocommerce/woocommerce-blocks/pull/8319)

* Allow observers to set billingAddress by returning billingData

This is required since we didn't correctly deprecate billingData when we changed the name to billingAddress

* Add tests for shippingAddress and paymentMethodData

* Add mocked __internalSetPaymentMethodData to correct object

It was in registry, but should be in dispatch as the action is on the same store as the thunk. Registry is used for actions on other stores.

* Re-add FieldValidationStatus type

* Add FieldValidationStatus back

* Remove empty file

* Import FieldValidationStatus from correct place

* Remove import of deleted types file

* Add isObserverResponse type guard

* Use error constant instead of magic string in event emitter

* Remove composite project tsconfig

* Add ObserverResponse type

* Add types to emitEventWithAbort

* Check if paymentmethod data is an object before dispatching

* Set types on observer responses

* Add validationErrors type guards

* Add tests for validation typeguards

* Add validation errors as option on observer response

* Add more granular observer response types

* Check observer response has correct types before dispatching actions

* Force type on deprecated billingData and shippingData

* Remove unnecessary comment

plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/address-form/address-form.tsx

@@ -22,10 +22,8 @@ import {
 	ShippingAddress,
 } from '@woocommerce/settings';
 import { useSelect, useDispatch } from '@wordpress/data';
-import {
-	VALIDATION_STORE_KEY,
-	FieldValidationStatus,
-} from '@woocommerce/block-data';
+import { VALIDATION_STORE_KEY } from '@woocommerce/block-data';
+import { FieldValidationStatus } from '@woocommerce/types';
 
 /**
  * Internal dependencies

plugins/woocommerce-blocks/assets/js/base/context/event-emit/emitters.ts

@@ -5,8 +5,11 @@ import {
 	getObserversByPriority,
 	isErrorResponse,
 	isFailResponse,
+	ObserverResponse,
+	responseTypes,
 } from './utils';
 import type { EventObserversType } from './types';
+import { isObserverResponse } from '../../../types/type-guards/observers';
 
 /**
  * Emits events on registered observers for the provided type and passes along
@@ -64,13 +67,13 @@ export const emitEventWithAbort = async (
 	observers: EventObserversType,
 	eventType: string,
 	data: unknown
-): Promise< Array< unknown > > => {
-	const observerResponses = [];
+): Promise< ObserverResponse[] > => {
+	const observerResponses: ObserverResponse[] = [];
 	const observersByType = getObserversByPriority( observers, eventType );
 	for ( const observer of observersByType ) {
 		try {
 			const response = await Promise.resolve( observer.callback( data ) );
-			if ( typeof response !== 'object' || response === null ) {
+			if ( ! isObserverResponse( response ) ) {
 				continue;
 			}
 			if ( ! response.hasOwnProperty( 'type' ) ) {
@@ -90,7 +93,7 @@ export const emitEventWithAbort = async (
 			// We don't handle thrown errors but just console.log for troubleshooting.
 			// eslint-disable-next-line no-console
 			console.error( e );
-			observerResponses.push( { type: 'error' } );
+			observerResponses.push( { type: responseTypes.ERROR } );
 			return observerResponses;
 		}
 	}

plugins/woocommerce-blocks/assets/js/base/context/event-emit/utils.ts

@@ -1,7 +1,7 @@
 /**
  * External dependencies
  */
-import { isObject } from '@woocommerce/types';
+import { FieldValidationStatus, isObject } from '@woocommerce/types';
 
 /**
  * Internal dependencies
@@ -42,6 +42,16 @@ export interface ResponseType extends Record< string, unknown > {
 	retry?: boolean;
 }
 
+/**
+ * Observers of checkout/cart events can return a response object to indicate success/error/failure. They may also
+ * optionally pass metadata.
+ */
+export interface ObserverResponse {
+	type: responseTypes;
+	meta?: Record< string, unknown > | undefined;
+	validationErrors?: Record< string, FieldValidationStatus > | undefined;
+}
+
 const isResponseOf = (
 	response: unknown,
 	type: string
@@ -51,19 +61,27 @@ const isResponseOf = (
 
 export const isSuccessResponse = (
 	response: unknown
-): response is ResponseType => {
+): response is ObserverFailResponse => {
 	return isResponseOf( response, responseTypes.SUCCESS );
 };
-
+interface ObserverSuccessResponse extends ObserverResponse {
+	type: responseTypes.SUCCESS;
+}
 export const isErrorResponse = (
 	response: unknown
-): response is ResponseType => {
+): response is ObserverSuccessResponse => {
 	return isResponseOf( response, responseTypes.ERROR );
 };
+interface ObserverErrorResponse extends ObserverResponse {
+	type: responseTypes.ERROR;
+}
 
+interface ObserverFailResponse extends ObserverResponse {
+	type: responseTypes.FAIL;
+}
 export const isFailResponse = (
 	response: unknown
-): response is ResponseType => {
+): response is ObserverErrorResponse => {
 	return isResponseOf( response, responseTypes.FAIL );
 };
 

plugins/woocommerce-blocks/assets/js/base/context/tsconfig.json

@@ -1,19 +0,0 @@
-{
-	"extends": "../../../../tsconfig.base.json",
-	"compilerOptions": {},
-	"include": [
-		".",
-		"../../blocks-registry/index.js",
-		"../../settings/shared/index.ts",
-		"../../settings/blocks/index.ts",
-		"../../base/hooks/index.js",
-		"../../base/utils/",
-		"../../utils",
-		"../../data/",
-		"../../types/",
-		"../components",
-		"../../blocks/cart-checkout-shared/payment-methods",
-		"../../settings/shared/default-address-fields.ts"
-	],
-	"exclude": [ "**/test/**" ]
-}

plugins/woocommerce-blocks/assets/js/data/checkout/types.ts

@@ -3,6 +3,7 @@
  */
 import type { Notice } from '@wordpress/notices/';
 import { DataRegistry } from '@wordpress/data';
+import { FieldValidationStatus } from '@woocommerce/types';
 
 /**
  * Internal dependencies
@@ -13,7 +14,6 @@ import type { PaymentState } from '../payment/default-state';
 import type { DispatchFromMap, SelectFromMap } from '../mapped-types';
 import * as selectors from './selectors';
 import * as actions from './actions';
-import { FieldValidationStatus } from '../types';
 
 export type CheckoutAfterProcessingWithErrorEventData = {
 	redirectUrl: CheckoutState[ 'redirectUrl' ];

plugins/woocommerce-blocks/assets/js/data/index.ts

@@ -15,5 +15,4 @@ export { VALIDATION_STORE_KEY } from './validation';
 export { QUERY_STATE_STORE_KEY } from './query-state';
 export { STORE_NOTICES_STORE_KEY } from './store-notices';
 export * from './constants';
-export * from './types';
 export * from './utils';

plugins/woocommerce-blocks/assets/js/data/payment/thunks.ts

@@ -4,6 +4,7 @@
 import { store as noticesStore } from '@wordpress/notices';
 import deprecated from '@wordpress/deprecated';
 import type { BillingAddress, ShippingAddress } from '@woocommerce/settings';
+import { isObject, isString, objectHasProp } from '@woocommerce/types';
 
 /**
  * Internal dependencies
@@ -14,6 +15,7 @@ import {
 	isFailResponse,
 	isSuccessResponse,
 	noticeContexts,
+	ObserverResponse,
 } from '../../base/context/event-emit';
 import { EMIT_TYPES } from '../../base/context/providers/cart-checkout/payment-events/event-emit';
 import type { emitProcessingEventType } from './types';
@@ -22,6 +24,8 @@ import {
 	isBillingAddress,
 	isShippingAddress,
 } from '../../types/type-guards/address';
+import { isObserverResponse } from '../../types/type-guards/observers';
+import { isValidValidationErrorsObject } from '../../types/type-guards/validation';
 
 export const __internalSetExpressPaymentError = ( message?: string ) => {
 	return ( { registry } ) => {
@@ -57,8 +61,8 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 			EMIT_TYPES.PAYMENT_PROCESSING,
 			{}
 		).then( ( observerResponses ) => {
-			let successResponse,
-				errorResponse,
+			let successResponse: ObserverResponse | undefined,
+				errorResponse: ObserverResponse | undefined,
 				billingAddress: BillingAddress | undefined,
 				shippingAddress: ShippingAddress | undefined;
 			observerResponses.forEach( ( response ) => {
@@ -86,12 +90,13 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 					shippingData: shippingDataFromResponse,
 				} = response?.meta || {};
 
-				billingAddress = billingAddressFromResponse;
-				shippingAddress = shippingAddressFromResponse;
+				billingAddress = billingAddressFromResponse as BillingAddress;
+				shippingAddress =
+					shippingAddressFromResponse as ShippingAddress;
 
 				if ( billingDataFromResponse ) {
 					// Set this here so that old extensions still using billingData can set the billingAddress.
-					billingAddress = billingDataFromResponse;
+					billingAddress = billingDataFromResponse as BillingAddress;
 					deprecated(
 						'returning billingData from an onPaymentProcessing observer in WooCommerce Blocks',
 						{
@@ -104,7 +109,8 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 
 				if ( shippingDataFromResponse ) {
 					// Set this here so that old extensions still using shippingData can set the shippingAddress.
-					shippingAddress = shippingDataFromResponse;
+					shippingAddress =
+						shippingDataFromResponse as ShippingAddress;
 					deprecated(
 						'returning shippingData from an onPaymentProcessing observer in WooCommerce Blocks',
 						{
@@ -119,9 +125,12 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 			const { setBillingAddress, setShippingAddress } =
 				registry.dispatch( CART_STORE_KEY );
 
-			if ( successResponse && ! errorResponse ) {
+			if (
+				isObserverResponse( successResponse ) &&
+				successResponse &&
+				! errorResponse
+			) {
 				const { paymentMethodData } = successResponse?.meta || {};
-
 				if ( billingAddress && isBillingAddress( billingAddress ) ) {
 					setBillingAddress( billingAddress );
 				}
@@ -131,16 +140,29 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 				) {
 					setShippingAddress( shippingAddress );
 				}
-				dispatch.__internalSetPaymentMethodData( paymentMethodData );
+				const paymentDataToSet = isObject( paymentMethodData )
+					? paymentMethodData
+					: {};
+				dispatch.__internalSetPaymentMethodData( paymentDataToSet );
 				dispatch.__internalSetPaymentSuccess();
-			} else if ( errorResponse && isFailResponse( errorResponse ) ) {
-				if ( errorResponse.message && errorResponse.message.length ) {
+			} else if ( isFailResponse( errorResponse ) ) {
+				if (
+					objectHasProp( errorResponse, 'message' ) &&
+					isString( errorResponse.message ) &&
+					errorResponse.message.length
+				) {
+					let context: string = noticeContexts.PAYMENTS;
+					if (
+						objectHasProp( errorResponse, 'messageContext' ) &&
+						isString( errorResponse.messageContext ) &&
+						errorResponse.messageContext.length
+					) {
+						context = errorResponse.messageContext;
+					}
 					createErrorNotice( errorResponse.message, {
 						id: 'wc-payment-error',
 						isDismissible: false,
-						context:
-							errorResponse?.messageContext ||
-							noticeContexts.PAYMENTS,
+						context,
 					} );
 				}
 
@@ -149,20 +171,41 @@ export const __internalEmitPaymentProcessingEvent: emitProcessingEventType = (
 					setBillingAddress( billingAddress );
 				}
 				dispatch.__internalSetPaymentFailed();
-				dispatch.__internalSetPaymentMethodData( paymentMethodData );
-			} else if ( errorResponse ) {
-				if ( errorResponse.message && errorResponse.message.length ) {
+
+				const paymentDataToSet = isObject( paymentMethodData )
+					? paymentMethodData
+					: {};
+				dispatch.__internalSetPaymentMethodData( paymentDataToSet );
+			} else if ( isErrorResponse( errorResponse ) ) {
+				if (
+					objectHasProp( errorResponse, 'message' ) &&
+					isString( errorResponse.message ) &&
+					errorResponse.message.length
+				) {
+					let context: string = noticeContexts.PAYMENTS;
+					if (
+						objectHasProp( errorResponse, 'messageContext' ) &&
+						isString( errorResponse.messageContext ) &&
+						errorResponse.messageContext.length
+					) {
+						context = errorResponse.messageContext;
+					}
 					createErrorNotice( errorResponse.message, {
 						id: 'wc-payment-error',
 						isDismissible: false,
-						context:
-							errorResponse?.messageContext ||
-							noticeContexts.PAYMENTS,
+						context,
 					} );
 				}
 
 				dispatch.__internalSetPaymentError();
-				setValidationErrors( errorResponse?.validationErrors );
+
+				if (
+					isValidValidationErrorsObject(
+						errorResponse.validationErrors
+					)
+				) {
+					setValidationErrors( errorResponse.validationErrors );
+				}
 			} else {
 				// otherwise there are no payment methods doing anything so
 				// just consider success

plugins/woocommerce-blocks/assets/js/data/payment/types.ts

@@ -5,7 +5,11 @@ import {
 	PlainPaymentMethods,
 	PlainExpressPaymentMethods,
 } from '@woocommerce/types';
-import type { EmptyObjectType, ObjectType } from '@woocommerce/types';
+import type {
+	EmptyObjectType,
+	ObjectType,
+	FieldValidationStatus,
+} from '@woocommerce/types';
 import { DataRegistry } from '@wordpress/data';
 
 /**
@@ -14,7 +18,6 @@ import { DataRegistry } from '@wordpress/data';
 import type { EventObserversType } from '../../base/context/event-emit';
 import type { DispatchFromMap } from '../mapped-types';
 import * as actions from './actions';
-import { FieldValidationStatus } from '../types';
 
 export interface CustomerPaymentMethodConfiguration {
 	gateway: string;

plugins/woocommerce-blocks/assets/js/data/validation/actions.ts

@@ -2,13 +2,13 @@
  * External dependencies
  */
 import deprecated from '@wordpress/deprecated';
+import { FieldValidationStatus } from '@woocommerce/types';
 
 /**
  * Internal dependencies
  */
 import { ACTION_TYPES as types } from './action-types';
 import { ReturnOrGeneratorYieldUnion } from '../mapped-types';
-import { FieldValidationStatus } from '../types';
 
 export const setValidationErrors = (
 	errors: Record< string, FieldValidationStatus >

plugins/woocommerce-blocks/assets/js/data/validation/reducers.ts

@@ -4,14 +4,13 @@
 import type { Reducer } from 'redux';
 import { pickBy } from 'lodash';
 import isShallowEqual from '@wordpress/is-shallow-equal';
-import { isString } from '@woocommerce/types';
+import { isString, FieldValidationStatus } from '@woocommerce/types';
 
 /**
  * Internal dependencies
  */
 import { ValidationAction } from './actions';
 import { ACTION_TYPES as types } from './action-types';
-import { FieldValidationStatus } from '../types';
 
 const reducer: Reducer< Record< string, FieldValidationStatus > > = (
 	state: Record< string, FieldValidationStatus > = {},

plugins/woocommerce-blocks/assets/js/data/validation/test/reducers.ts

@@ -1,8 +1,12 @@
+/**
+ * External dependencies
+ */
+import { FieldValidationStatus } from '@woocommerce/types';
+
 /**
  * Internal dependencies
  */
 import reducer from '../reducers';
-import { FieldValidationStatus } from '../../types';
 import { ACTION_TYPES as types } from '.././action-types';
 import { ValidationAction } from '../actions';
 

plugins/woocommerce-blocks/assets/js/data/validation/test/selectors.ts

@@ -1,3 +1,8 @@
+/**
+ * External dependencies
+ */
+import { FieldValidationStatus } from '@woocommerce/types';
+
 /**
  * Internal dependencies
  */
@@ -6,7 +11,6 @@ import {
 	getValidationError,
 	hasValidationErrors,
 } from '../selectors';
-import { FieldValidationStatus } from '../../types';
 
 describe( 'Validation selectors', () => {
 	it( 'Gets the validation error', () => {

plugins/woocommerce-blocks/assets/js/types/type-defs/index.ts

@@ -17,3 +17,4 @@ export * from './utils';
 export * from './taxes';
 export * from './attributes';
 export * from './stock-status';
+export * from './validation';

plugins/woocommerce-blocks/assets/js/types/type-defs/validation.ts

@@ -0,0 +1,16 @@
+/**
+ * An interface to describe the validity of a Checkout field. This is what will be stored in the wc/store/validation
+ * data store.
+ */
+export interface FieldValidationStatus {
+	/**
+	 * The message to display to the user.
+	 */
+	message: string;
+	/**
+	 * Whether this validation error should be hidden. Note, hidden errors still prevent checkout. Adding a hidden error
+	 * allows required fields to be validated, but not show the error to the user until they interact with the input
+	 * element, or try to submit the form.
+	 */
+	hidden: boolean;
+}

plugins/woocommerce-blocks/assets/js/types/type-guards/observers.ts

@@ -0,0 +1,14 @@
+/**
+ * External dependencies
+ */
+import { ObserverResponse } from '@woocommerce/base-context';
+import { isObject, objectHasProp } from '@woocommerce/types';
+
+/**
+ * Whether the passed object is an ObserverResponse.
+ */
+export const isObserverResponse = (
+	response: unknown
+): response is ObserverResponse => {
+	return isObject( response ) && objectHasProp( response, 'type' );
+};

plugins/woocommerce-blocks/assets/js/types/type-guards/test/validation.ts

@@ -0,0 +1,57 @@
+/**
+ * Internal dependencies
+ */
+import {
+	isValidFieldValidationStatus,
+	isValidValidationErrorsObject,
+} from '../validation';
+
+describe( 'validation type guards', () => {
+	describe( 'isValidFieldValidationStatus', () => {
+		it( 'identifies valid objects', () => {
+			const valid = {
+				message: 'message',
+				hidden: false,
+			};
+			expect( isValidFieldValidationStatus( valid ) ).toBe( true );
+		} );
+		it( 'identifies invalid objects', () => {
+			const invalid = {
+				message: 'message',
+				hidden: 'string',
+			};
+			expect( isValidFieldValidationStatus( invalid ) ).toBe( false );
+			const noMessage = {
+				hidden: false,
+			};
+			expect( isValidFieldValidationStatus( noMessage ) ).toBe( false );
+		} );
+	} );
+
+	describe( 'isValidValidationErrorsObject', () => {
+		it( 'identifies valid objects', () => {
+			const valid = {
+				'billing.first-name': {
+					message: 'message',
+					hidden: false,
+				},
+			};
+			expect( isValidValidationErrorsObject( valid ) ).toBe( true );
+		} );
+		it( 'identifies invalid objects', () => {
+			const invalid = {
+				'billing.first-name': {
+					message: 'message',
+					hidden: 'string',
+				},
+			};
+			expect( isValidValidationErrorsObject( invalid ) ).toBe( false );
+			const noMessage = {
+				'billing.first-name': {
+					hidden: false,
+				},
+			};
+			expect( isValidValidationErrorsObject( noMessage ) ).toBe( false );
+		} );
+	} );
+} );

plugins/woocommerce-blocks/assets/js/types/type-guards/validation.ts

@@ -0,0 +1,41 @@
+/**
+ * External dependencies
+ */
+import {
+	FieldValidationStatus,
+	isBoolean,
+	isObject,
+	isString,
+	objectHasProp,
+} from '@woocommerce/types';
+
+/**
+ * Whether the given status is a valid FieldValidationStatus.
+ */
+export const isValidFieldValidationStatus = (
+	status: unknown
+): status is FieldValidationStatus => {
+	return (
+		isObject( status ) &&
+		objectHasProp( status, 'message' ) &&
+		objectHasProp( status, 'hidden' ) &&
+		isString( status.message ) &&
+		isBoolean( status.hidden )
+	);
+};
+
+/**
+ * Whether the passed object is a valid validation errors object. If this is true, it can be set on the
+ * wc/store/validation store without any issue.
+ */
+export const isValidValidationErrorsObject = (
+	errors: unknown
+): errors is Record< string, FieldValidationStatus > => {
+	return (
+		isObject( errors ) &&
+		Object.entries( errors ).every(
+			( [ key, value ] ) =>
+				isString( key ) && isValidFieldValidationStatus( value )
+		)
+	);
+};