diff --git a/src/feedback/data/constants.js b/src/feedback/data/constants.js index e7b3c4200..8257fc988 100644 --- a/src/feedback/data/constants.js +++ b/src/feedback/data/constants.js @@ -12,3 +12,9 @@ export const MESSAGE_TYPES = { WARNING: 'warning', ERROR: 'error', }; + +export const ERROR_CODES = { + FALLBACK: 'fallback-error', + BASKET_CHANGED: 'basket-changed-error-message', + TRANSACTION_DECLINED: 'transaction-declined-message', +}; diff --git a/src/feedback/data/sagas.js b/src/feedback/data/sagas.js index bc3b761d3..301119f70 100644 --- a/src/feedback/data/sagas.js +++ b/src/feedback/data/sagas.js @@ -2,7 +2,7 @@ import { put } from 'redux-saga/effects'; import { logError, logInfo } from '@edx/frontend-platform/logging'; import { addMessage, clearMessages } from './actions'; -import { MESSAGE_TYPES } from './constants'; +import { ERROR_CODES, MESSAGE_TYPES } from './constants'; export function* handleErrors(e, clearExistingMessages) { if (clearExistingMessages) { @@ -11,15 +11,15 @@ export function* handleErrors(e, clearExistingMessages) { // If this doesn't contain anything we understand, add a fallback error message if (e.errors === undefined && e.fieldErrors === undefined && e.messages === undefined) { - yield put(addMessage('fallback-error', null, {}, MESSAGE_TYPES.ERROR)); + yield put(addMessage(ERROR_CODES.FALLBACK, null, {}, MESSAGE_TYPES.ERROR)); } if (e.errors !== undefined) { for (let i = 0; i < e.errors.length; i++) { // eslint-disable-line no-plusplus const error = e.errors[i]; - if (error.code === 'basket-changed-error-message') { + if (error.code === ERROR_CODES.BASKET_CHANGED) { yield put(addMessage(error.code, error.userMessage, {}, MESSAGE_TYPES.ERROR)); } else if (error.data === undefined && error.messageType === null) { - yield put(addMessage('transaction-declined-message', error.userMessage, {}, MESSAGE_TYPES.ERROR)); + yield put(addMessage(ERROR_CODES.TRANSACTION_DECLINED, error.userMessage, {}, MESSAGE_TYPES.ERROR)); } else { yield put(addMessage(error.code, error.userMessage, error.data, error.messageType)); } diff --git a/src/payment/data/__snapshots__/redux.test.js.snap b/src/payment/data/__snapshots__/redux.test.js.snap index 0c07dc638..8d0be26ce 100644 --- a/src/payment/data/__snapshots__/redux.test.js.snap +++ b/src/payment/data/__snapshots__/redux.test.js.snap @@ -8,6 +8,7 @@ Object { "loading": true, "paymentState": "checkout", "paymentStatePolling": Object { + "errorCount": 5, "keepPolling": false, }, "products": Array [], @@ -34,6 +35,7 @@ Object { "loading": true, "paymentState": "checkout", "paymentStatePolling": Object { + "errorCount": 5, "keepPolling": false, }, "products": Array [], @@ -60,6 +62,7 @@ Object { "loading": true, "paymentState": "checkout", "paymentStatePolling": Object { + "errorCount": 5, "keepPolling": false, }, "products": Array [], diff --git a/src/payment/data/constants.js b/src/payment/data/constants.js index a487a2974..e3af1f2f3 100644 --- a/src/payment/data/constants.js +++ b/src/payment/data/constants.js @@ -63,3 +63,19 @@ export const POLLING_PAYMENT_STATES = [ PAYMENT_STATE.PENDING, PAYMENT_STATE.HTTP_ERROR, ]; + +/** + * + * @type {number} + * + * This can be configured by setting `PAYMENT_STATE_POLLING_DELAY_SECS` in your config. + */ +export const DEFAULT_PAYMENT_STATE_POLLING_DELAY_SECS = 5; + +/** + * + * @type {number} + * + * This can be configured by setting `PAYMENT_STATE_POLLING_MAX_ERRORS` in your config. + */ +export const DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS = 5; diff --git a/src/payment/data/handleRequestError.js b/src/payment/data/handleRequestError.js index df23c4ebd..e84f9f746 100644 --- a/src/payment/data/handleRequestError.js +++ b/src/payment/data/handleRequestError.js @@ -1,6 +1,28 @@ import { logError, logInfo } from '@edx/frontend-platform/logging'; import { camelCaseObject } from './utils'; +import { ERROR_CODES } from '../../feedback/data/constants'; +/** + * @class RequestError + * + * @property {AxiosResponse} [response] + * @property {string?} [code] + * @property {string?} [type] + * + * @extends Error + */ +/** + * @typedef ApiErrorMessage + * + * @property {string?} [error_code] + * @property {string?} [user_message] + * @property {string?} [message_type] + * + */ + +/** + * @throws + */ function handleFieldErrors(errors) { const fieldErrors = Object.entries(errors).map(([name, value]) => ({ code: value.error_code ? value.error_code : null, @@ -13,7 +35,13 @@ function handleFieldErrors(errors) { throw validationError; } -function handleApiErrors(errors) { +/** + * Process API Errors and Generate an Error Object + * @param {ApiErrorMessage[]} errors + * @param {boolean} shouldThrow + * @throws {Error} (Conditionally, but usually) + */ +export function generateApiError(errors, shouldThrow = true) { const apiErrors = errors.map(err => ({ code: err.error_code ? err.error_code : null, userMessage: err.user_message ? err.user_message : null, @@ -22,9 +50,16 @@ function handleApiErrors(errors) { const apiError = new Error(); apiError.errors = apiErrors; - throw apiError; + + if (shouldThrow) { throw apiError; } + + return apiError; } +/** + * @param {*} messages + * @throws + */ function handleApiMessages(messages) { const apiError = new Error(); apiError.messages = camelCaseObject(messages); @@ -39,9 +74,8 @@ function handleApiMessages(messages) { * * Field errors will be packaged with a fieldErrors field usable by the client. * - * @param error The original error object. - * @param unpackFunction (Optional) A function to use to unpack the field errors as a replacement - * for the default. + * @param {RequestError|Error} error The original error object. + * @throws */ export default function handleRequestError(error) { // Validation errors @@ -53,7 +87,7 @@ export default function handleRequestError(error) { // API errors if (error.response && error.response.data.errors !== undefined) { logInfo('API Errors', error.response.data.errors); - handleApiErrors(error.response.data.errors); + generateApiError(error.response.data.errors); } // API messages @@ -65,7 +99,7 @@ export default function handleRequestError(error) { // Single API error if (error.response && error.response.data.error_code) { logInfo('API Error', error.response.data.error_code); - handleApiErrors([ + generateApiError([ { error_code: error.response.data.error_code, user_message: error.response.data.user_message, @@ -76,9 +110,9 @@ export default function handleRequestError(error) { // SKU mismatch error if (error.response && error.response.data.sku_error) { logInfo('SKU Error', error.response.data.sku_error); - handleApiErrors([ + generateApiError([ { - error_code: 'basket-changed-error-message', + error_code: ERROR_CODES.BASKET_CHANGED, user_message: 'error', }, ]); @@ -87,9 +121,9 @@ export default function handleRequestError(error) { // Basket already purchased if (error.code === 'payment_intent_unexpected_state' && error.type === 'invalid_request_error') { logInfo('Basket Changed Error', error.code); - handleApiErrors([ + generateApiError([ { - error_code: 'basket-changed-error-message', + error_code: ERROR_CODES.BASKET_CHANGED, user_message: 'error', }, ]); @@ -100,7 +134,11 @@ export default function handleRequestError(error) { throw error; } -// Processes API errors and converts them to error objects the sagas can use. +/** + * Processes API errors and converts them to error objects the sagas can use. + * @param requestError + * @throws + */ export function handleApiError(requestError) { try { // Always throws an error: diff --git a/src/payment/data/reducers.js b/src/payment/data/reducers.js index 112918b83..9f08b651c 100644 --- a/src/payment/data/reducers.js +++ b/src/payment/data/reducers.js @@ -1,5 +1,6 @@ import { combineReducers } from 'redux'; +import { getConfig } from '@edx/frontend-platform'; import { BASKET_DATA_RECEIVED, BASKET_PROCESSING, @@ -18,7 +19,11 @@ import { } from './actions'; import { DEFAULT_STATUS } from '../checkout/payment-form/flex-microform/constants'; -import { PAYMENT_STATE, POLLING_PAYMENT_STATES } from './constants'; +import { + DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS, + PAYMENT_STATE, + POLLING_PAYMENT_STATES, +} from './constants'; import { chainReducers } from './utils'; /** @@ -29,6 +34,11 @@ const paymentStatePollingInitialState = { * @see paymentProcessStatusIsPollingSelector */ keepPolling: false, + /** + * This is replaceable by a configuration value. (`PAYMENT_STATE_POLLING_MAX_ERRORS`), + * however, this is our default. + */ + errorCount: DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS, }; /** @@ -147,16 +157,33 @@ const clientSecret = (state = clientSecretInitialState, action = null) => { }; const paymentState = (state = basketInitialState, action = null) => { + // noinspection JSUnresolvedReference + const maxErrors = getConfig().PAYMENT_STATE_POLLING_MAX_ERRORS || paymentStatePollingInitialState.errorCount; const shouldPoll = (payState) => POLLING_PAYMENT_STATES.includes(payState); if (action !== null && action !== undefined) { switch (action.type) { + // The modal relies on the basket's paymentState + // The Inner paymentStatePolling object is used only by the saga handler/worker + case pollPaymentState.TRIGGER: return { ...state, paymentStatePolling: { ...state.paymentStatePolling, keepPolling: shouldPoll(state.paymentState), + errorCount: maxErrors, + }, + }; + + case pollPaymentState.FAILURE: + return { + ...state, + paymentState: null, + paymentStatePolling: { + ...state.paymentStatePolling, + keepPolling: false, + errorCount: maxErrors, }, }; @@ -166,6 +193,7 @@ const paymentState = (state = basketInitialState, action = null) => { paymentStatePolling: { ...state.paymentStatePolling, keepPolling: false, + errorCount: maxErrors, }, }; @@ -176,7 +204,8 @@ const paymentState = (state = basketInitialState, action = null) => { paymentStatePolling: { ...state.paymentStatePolling, keepPolling: shouldPoll(action.payload.state), - // ...action.payload, // debugging + errorCount: (action.payload.state === PAYMENT_STATE.HTTP_ERROR + ? state.paymentStatePolling.errorCount - 1 : maxErrors), }, }; diff --git a/src/payment/data/redux.test.js b/src/payment/data/redux.test.js index 262a621e2..471c3c5c7 100644 --- a/src/payment/data/redux.test.js +++ b/src/payment/data/redux.test.js @@ -13,7 +13,7 @@ import { } from './actions'; import { currencyDisclaimerSelector, paymentSelector } from './selectors'; import { localizedCurrencySelector } from './utils'; -import { PAYMENT_STATE } from './constants'; +import { DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS, PAYMENT_STATE } from './constants'; jest.mock('universal-cookie', () => { class MockCookies { @@ -114,6 +114,7 @@ describe('redux tests', () => { isRedirect: false, paymentState: PAYMENT_STATE.DEFAULT, paymentStatePolling: { + errorCount: DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS, keepPolling: false, }, }); @@ -138,6 +139,7 @@ describe('redux tests', () => { isRedirect: true, // this is also now true. paymentState: PAYMENT_STATE.DEFAULT, paymentStatePolling: { + errorCount: DEFAULT_PAYMENT_STATE_POLLING_MAX_ERRORS, keepPolling: false, }, }); diff --git a/src/payment/data/sagas.js b/src/payment/data/sagas.js index d961e3cdd..20e237553 100644 --- a/src/payment/data/sagas.js +++ b/src/payment/data/sagas.js @@ -3,8 +3,9 @@ import { } from 'redux-saga/effects'; import { stopSubmit } from 'redux-form'; import { getConfig } from '@edx/frontend-platform'; +import { logError } from '@edx/frontend-platform/logging/interface'; import { getReduxFormValidationErrors, MINS_AS_MS, SECS_AS_MS } from './utils'; -import { MESSAGE_TYPES } from '../../feedback/data/constants'; +import { ERROR_CODES, MESSAGE_TYPES } from '../../feedback/data/constants'; // Actions import { @@ -41,7 +42,8 @@ import { checkout as checkoutPaypal } from '../payment-methods/paypal'; import { checkout as checkoutApplePay } from '../payment-methods/apple-pay'; import { checkout as checkoutStripe } from '../payment-methods/stripe'; import { paymentProcessStatusShouldRunSelector } from './selectors'; -import { PAYMENT_STATE } from './constants'; +import { PAYMENT_STATE, DEFAULT_PAYMENT_STATE_POLLING_DELAY_SECS } from './constants'; +import { generateApiError } from './handleRequestError'; export const paymentMethods = { cybersource: checkoutWithToken, @@ -310,43 +312,70 @@ export function* handleSubmitPayment({ payload }) { * - This handler/worker loops until it is told to stop. via a state property (keepPolling), or a fatal state. */ export function* handlePaymentState() { - const DEFAULT_DELAY_SECS = 5; - let keepPolling = true; + // We will be using exceptions to halt execution and fiving us a simpler single path + // however this occurs by rethrowing the lower level exceptions. // noinspection JSUnresolvedReference - const delaySecs = getConfig().PAYMENT_STATE_POLLING_DELAY_SECS || DEFAULT_DELAY_SECS; - - // NOTE: We may want to have a max errors check and have a fail state if there's a bad connection or something. - - while (keepPolling) { - try { - const basketId = yield select(state => state.payment.basket.basketId); - const paymentNumber = yield select(state => (state.payment.basket.payments.length === 0 - ? null : state.payment.basket.payments[0].paymentNumber)); - - if (!basketId || !paymentNumber) { - // This shouldn't happen. - // I don't think we need to banner... shouldn't our parent calls recover? (They invoke this) - keepPolling = false; - yield put(pollPaymentState.fulfill()); - return; - } + const delaySecs = getConfig().PAYMENT_STATE_POLLING_DELAY_SECS || DEFAULT_PAYMENT_STATE_POLLING_DELAY_SECS; - const result = yield call(PaymentApiService.getCurrentPaymentState, paymentNumber, basketId); - yield put(paymentStateDataReceived(result)); + const [basketId, paymentNumber] = yield select(state => ([ + /* basketId */ + state.payment.basket.basketId, + /* paymentNumber */ + (state.payment.basket.payments.length === 0 + ? null : state.payment.basket.payments[0].paymentNumber), + ])); + + try { + while (true) { // o/ o/ ive got to break free! o/ o/ ... or throw. + try { + if (!basketId || !paymentNumber) { + // noinspection ExceptionCaughtLocallyJS + throw new ReferenceError('Invalid Basket Id or Payment Number'); + } + + const result = yield call(PaymentApiService.getCurrentPaymentState, paymentNumber, basketId); + yield put(paymentStateDataReceived(result)); + + if (!(yield select(state => state.payment.basket.paymentStatePolling.keepPolling))) { + yield put(pollPaymentState.fulfill()); + break; + } else { + yield delay(SECS_AS_MS(delaySecs)); + } + } catch (innerError) { + debugger; + const shouldExitExecution = innerError instanceof ReferenceError; + + logError(innerError, { basketId, paymentNumber, idFatal: shouldExitExecution }); + + if (shouldExitExecution) { + // noinspection ExceptionCaughtLocallyJS + throw innerError; + } + + yield put(paymentStateDataReceived({ state: PAYMENT_STATE.HTTP_ERROR })); + + if (yield select(state => state.payment.basket.paymentStatePolling.errorCount) < 1) { + // noinspection ExceptionCaughtLocallyJS + throw innerError; + } - if (!(yield select(state => state.payment.basket.paymentStatePolling.keepPolling))) { - keepPolling = false; - yield put(pollPaymentState.fulfill()); - } else { yield delay(SECS_AS_MS(delaySecs)); } - } catch (error) { - // We dont quit on error. - // yield call(handleErrors, error, true); - yield put(paymentStateDataReceived({ state: PAYMENT_STATE.HTTP_ERROR })); - yield delay(SECS_AS_MS(delaySecs)); } + } catch (error) { + debugger; + const basketMessageError = generateApiError([ + { + error_code: ERROR_CODES.BASKET_CHANGED, + user_message: 'error', + }, + ], false); + + yield put(pollPaymentState.failure(basketMessageError)); + + yield handleErrors(basketMessageError); } } diff --git a/src/payment/data/sagas.test.js b/src/payment/data/sagas.test.js index fd3634e91..c9db19e69 100644 --- a/src/payment/data/sagas.test.js +++ b/src/payment/data/sagas.test.js @@ -52,8 +52,7 @@ const axiosMock = new MockAdapter(axios); getAuthenticatedHttpClient.mockReturnValue(axios); const BASKET_API_ENDPOINT = `${getConfig().ECOMMERCE_BASE_URL}/bff/payment/v0/payment/`; -// const CC_ORDER_API_ENDPOINT = `${getConfig().COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/`; -const CC_ORDER_API_ENDPOINT = `${process.env.COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/active/`; +const CC_ORDER_API_ENDPOINT = `${process.env.COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/active`; const DISCOUNT_API_ENDPOINT = `${getConfig().LMS_BASE_URL}/api/discounts/course/`; const COUPON_API_ENDPOINT = `${getConfig().ECOMMERCE_BASE_URL}/bff/payment/v0/vouchers/`; const QUANTITY_API_ENDPOINT = `${getConfig().ECOMMERCE_BASE_URL}/bff/payment/v0/quantity/`; diff --git a/src/payment/data/service.js b/src/payment/data/service.js index bc6cc17a4..1d97b4d63 100644 --- a/src/payment/data/service.js +++ b/src/payment/data/service.js @@ -50,9 +50,9 @@ export async function getBasket(discountJwt) { } export async function getActiveOrder() { + // This call cant end in `/` or it fails to pattern match in CC const { data } = await getAuthenticatedHttpClient() - // .get(`${getConfig().COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/`) - .get(`${process.env.COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/active/`) + .get(`${process.env.COMMERCE_COORDINATOR_BASE_URL}/frontend-app-payment/order/active`) .catch(handleBasketApiError); return transformResults(data); }