diff --git a/src/CONST.js b/src/CONST.js index 77491a18d469..9b186163611f 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -299,6 +299,8 @@ const CONST = { API_OFFLINE: 'session.offlineMessageRetry', UNKNOWN_ERROR: 'Unknown error', REQUEST_CANCELLED: 'AbortError', + FAILED_TO_FETCH: 'Failed to fetch', + ENSURE_BUGBOT: 'ENSURE_BUGBOT', }, NETWORK: { METHOD: { @@ -308,10 +310,9 @@ const CONST = { PROCESS_REQUEST_DELAY_MS: 1000, MAX_PENDING_TIME_MS: 10 * 1000, }, - HTTP_STATUS_CODE: { + JSON_CODE: { SUCCESS: 200, - BAD_REQUEST: 400, - UNAUTHORIZED: 401, + NOT_AUTHENTICATED: 407, }, NVP: { IS_FIRST_TIME_NEW_EXPENSIFY_USER: 'isFirstTimeNewExpensifyUser', diff --git a/src/libs/API.js b/src/libs/API.js index 948f2a64b2e5..0dd443b93792 100644 --- a/src/libs/API.js +++ b/src/libs/API.js @@ -1,9 +1,7 @@ import _ from 'underscore'; import lodashGet from 'lodash/get'; -import Onyx from 'react-native-onyx'; import CONST from '../CONST'; import CONFIG from '../CONFIG'; -import ONYXKEYS from '../ONYXKEYS'; import getPlaidLinkTokenParameters from './getPlaidLinkTokenParameters'; import redirectToSignIn from './actions/SignInRedirect'; import isViaExpensifyCashNative from './isViaExpensifyCashNative'; @@ -12,87 +10,10 @@ import Log from './Log'; import * as Network from './Network'; import updateSessionAuthTokens from './actions/Session/updateSessionAuthTokens'; import setSessionLoadingAndError from './actions/Session/setSessionLoadingAndError'; +import * as NetworkStore from './Network/NetworkStore'; +import enhanceParameters from './Network/enhanceParameters'; let isAuthenticating; -let credentials; -let authToken; -let currentUserEmail; - -function checkRequiredDataAndSetNetworkReady() { - if (_.isUndefined(authToken) || _.isUndefined(credentials)) { - return; - } - - Network.setIsReady(true); -} - -Onyx.connect({ - key: ONYXKEYS.CREDENTIALS, - callback: (val) => { - credentials = val || null; - checkRequiredDataAndSetNetworkReady(); - }, -}); - -Onyx.connect({ - key: ONYXKEYS.SESSION, - callback: (val) => { - authToken = lodashGet(val, 'authToken', null); - currentUserEmail = lodashGet(val, 'email', null); - checkRequiredDataAndSetNetworkReady(); - }, -}); - -/** - * Does this command require an authToken? - * - * @param {String} command - * @return {Boolean} - */ -function isAuthTokenRequired(command) { - return !_.contains([ - 'Log', - 'Graphite_Timer', - 'Authenticate', - 'GetAccountStatus', - 'SetPassword', - 'User_SignUp', - 'ResendValidateCode', - 'ResetPassword', - 'User_ReopenAccount', - 'ValidateEmail', - ], command); -} - -/** - * Adds default values to our request data - * - * @param {String} command - * @param {Object} parameters - * @returns {Object} - */ -function addDefaultValuesToParameters(command, parameters) { - const finalParameters = {...parameters}; - - if (isAuthTokenRequired(command) && !parameters.authToken) { - finalParameters.authToken = authToken; - } - - finalParameters.referer = CONFIG.EXPENSIFY.EXPENSIFY_CASH_REFERER; - - // This application does not save its authToken in cookies like the classic Expensify app. - // Setting api_setCookie to false will ensure that the Expensify API doesn't set any cookies - // and prevents interfering with the cookie authToken that Expensify classic uses. - finalParameters.api_setCookie = false; - - // Unless email is already set include current user's email in every request and the server logs - finalParameters.email = lodashGet(parameters, 'email', currentUserEmail); - - return finalParameters; -} - -// Tie into the network layer to add auth token to the parameters of all requests -Network.registerParameterEnhancer(addDefaultValuesToParameters); /** * Function used to handle expired auth tokens. It re-authenticates with the API and @@ -118,7 +39,7 @@ function handleExpiredAuthToken(originalCommand, originalParameters, originalTyp return reauthenticate(originalCommand) .then(() => { // Now that the API is authenticated, make the original request again with the new authToken - const params = addDefaultValuesToParameters(originalCommand, originalParameters); + const params = enhanceParameters(originalCommand, originalParameters); return Network.post(originalCommand, params, originalType); }) .catch(() => ( @@ -159,8 +80,10 @@ Network.registerResponseHandler((queuedRequest, response) => { }); } - if (response.jsonCode === 407) { - // Credentials haven't been initialized. We will not be able to re-authenticates with the API + if (response.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) { + const credentials = NetworkStore.getCredentials(); + + // Credentials haven't been initialized. We will not be able to re-authenticate with the API const unableToReauthenticate = (!credentials || !credentials.autoGeneratedLogin || !credentials.autoGeneratedPassword); @@ -169,13 +92,21 @@ Network.registerResponseHandler((queuedRequest, response) => { // of the new response created by handleExpiredAuthToken. const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry'); if (!shouldRetry || unableToReauthenticate) { - queuedRequest.resolve(response); + // Check to see if queuedRequest has a resolve method as this could be a persisted request which had it's promise handling logic stripped + // from it when persisted to storage + if (queuedRequest.resolve) { + queuedRequest.resolve(response); + } return; } handleExpiredAuthToken(queuedRequest.command, queuedRequest.data, queuedRequest.type) - .then(queuedRequest.resolve) - .catch(queuedRequest.reject); + .then(queuedRequest.resolve || (() => Promise.resolve())) + .catch(queuedRequest.reject || (() => Promise.resolve())); + return; + } + + if (!queuedRequest.resolve) { return; } @@ -197,8 +128,12 @@ Network.registerErrorHandler((queuedRequest, error) => { // Set an error state and signify we are done loading setSessionLoadingAndError(false, 'Cannot connect to server'); - // Reject the queued request with an API offline error so that the original caller can handle it. - queuedRequest.reject(new Error(CONST.ERROR.API_OFFLINE)); + // Reject the queued request with an API offline error so that the original caller can handle it + // Note: We are checking for the reject method as this could be a persisted request which had it's promise handling logic stripped + // from it when persisted to storage + if (queuedRequest.reject) { + queuedRequest.reject(new Error(CONST.ERROR.API_OFFLINE)); + } }); /** @@ -280,6 +215,7 @@ function Authenticate(parameters) { * @returns {Promise} */ function reauthenticate(command = '') { + const credentials = NetworkStore.getCredentials(); return Authenticate({ useExpensifyLogin: false, partnerName: CONFIG.EXPENSIFY.PARTNER_NAME, @@ -297,7 +233,11 @@ function reauthenticate(command = '') { // Update authToken in Onyx and in our local variables so that API requests will use the // new authToken updateSessionAuthTokens(response.authToken, response.encryptedAuthToken); - authToken = response.authToken; + + // Note: It is important to manually set the authToken that is in the store here since any requests that are hooked into + // reauthenticate .then() will immediate post and use the local authToken. Onyx updates subscribers lately so it is not + // enough to do the updateSessionAuthTokens() call above. + NetworkStore.setAuthToken(response.authToken); // The authentication process is finished so the network can be unpaused to continue // processing requests diff --git a/src/libs/HttpUtils.js b/src/libs/HttpUtils.js index b58ff52e4dcd..f377ad4ae95b 100644 --- a/src/libs/HttpUtils.js +++ b/src/libs/HttpUtils.js @@ -30,7 +30,13 @@ function processHTTPRequest(url, method = 'get', body = null, canCancel = true) method, body, }) - .then(response => response.json()); + .then((response) => { + if (!response.ok) { + throw Error(response.statusText); + } + + return response.json(); + }); } /** diff --git a/src/libs/Network/NetworkStore.js b/src/libs/Network/NetworkStore.js new file mode 100644 index 000000000000..bb3725d67d0c --- /dev/null +++ b/src/libs/Network/NetworkStore.js @@ -0,0 +1,89 @@ +import lodashGet from 'lodash/get'; +import Onyx from 'react-native-onyx'; +import _ from 'underscore'; +import ONYXKEYS from '../../ONYXKEYS'; + +let credentials; +let authToken; +let currentUserEmail; +let networkReady = false; + +/** + * @param {Boolean} ready + */ +function setIsReady(ready) { + networkReady = ready; +} + +/** + * This is a hack to workaround the fact that Onyx may not yet have read these values from storage by the time Network starts processing requests. + * If the values are undefined we haven't read them yet. If they are null or have a value then we have and the network is "ready". + */ +function checkRequiredDataAndSetNetworkReady() { + if (_.isUndefined(authToken) || _.isUndefined(credentials)) { + return; + } + + setIsReady(true); +} + +Onyx.connect({ + key: ONYXKEYS.SESSION, + callback: (val) => { + authToken = lodashGet(val, 'authToken', null); + currentUserEmail = lodashGet(val, 'email', null); + checkRequiredDataAndSetNetworkReady(); + }, +}); + +Onyx.connect({ + key: ONYXKEYS.CREDENTIALS, + callback: (val) => { + credentials = val || null; + checkRequiredDataAndSetNetworkReady(); + }, +}); + +/** + * @returns {String} + */ +function getAuthToken() { + return authToken; +} + +/** + * @param {String} newAuthToken + */ +function setAuthToken(newAuthToken) { + authToken = newAuthToken; +} + +/** + * @returns {Object} + */ +function getCredentials() { + return credentials; +} + +/** + * @returns {String} + */ +function getCurrentUserEmail() { + return currentUserEmail; +} + +/** + * @returns {Boolean} + */ +function isReady() { + return networkReady; +} + +export { + getAuthToken, + setAuthToken, + getCredentials, + getCurrentUserEmail, + isReady, + setIsReady, +}; diff --git a/src/libs/Network/enhanceParameters.js b/src/libs/Network/enhanceParameters.js new file mode 100644 index 000000000000..4bf55e77b51c --- /dev/null +++ b/src/libs/Network/enhanceParameters.js @@ -0,0 +1,52 @@ +import lodashGet from 'lodash/get'; +import _ from 'underscore'; +import CONFIG from '../../CONFIG'; +import * as NetworkStore from './NetworkStore'; + +/** + * Does this command require an authToken? + * + * @param {String} command + * @return {Boolean} + */ +function isAuthTokenRequired(command) { + return !_.contains([ + 'Log', + 'Graphite_Timer', + 'Authenticate', + 'GetAccountStatus', + 'SetPassword', + 'User_SignUp', + 'ResendValidateCode', + 'ResetPassword', + 'User_ReopenAccount', + 'ValidateEmail', + ], command); +} + +/** + * Adds default values to our request data + * + * @param {String} command + * @param {Object} parameters + * @returns {Object} + */ +export default function enhanceParameters(command, parameters) { + const finalParameters = {...parameters}; + + if (isAuthTokenRequired(command) && !parameters.authToken) { + finalParameters.authToken = NetworkStore.getAuthToken(); + } + + finalParameters.referer = CONFIG.EXPENSIFY.EXPENSIFY_CASH_REFERER; + + // This application does not save its authToken in cookies like the classic Expensify app. + // Setting api_setCookie to false will ensure that the Expensify API doesn't set any cookies + // and prevents interfering with the cookie authToken that Expensify classic uses. + finalParameters.api_setCookie = false; + + // Include current user's email in every request and the server logs + finalParameters.email = lodashGet(parameters, 'email', NetworkStore.getCurrentUserEmail()); + + return finalParameters; +} diff --git a/src/libs/Network.js b/src/libs/Network/index.js similarity index 71% rename from src/libs/Network.js rename to src/libs/Network/index.js index 16ce0acc0203..f27e418dbaf9 100644 --- a/src/libs/Network.js +++ b/src/libs/Network/index.js @@ -1,14 +1,15 @@ import _ from 'underscore'; import lodashGet from 'lodash/get'; import Onyx from 'react-native-onyx'; -import HttpUtils from './HttpUtils'; -import ONYXKEYS from '../ONYXKEYS'; -import * as ActiveClientManager from './ActiveClientManager'; -import CONST from '../CONST'; -import createCallback from './createCallback'; -import * as NetworkRequestQueue from './actions/NetworkRequestQueue'; - -let isReady = false; +import HttpUtils from '../HttpUtils'; +import ONYXKEYS from '../../ONYXKEYS'; +import * as ActiveClientManager from '../ActiveClientManager'; +import CONST from '../../CONST'; +import createCallback from '../createCallback'; +import * as NetworkRequestQueue from '../actions/NetworkRequestQueue'; +import * as NetworkStore from './NetworkStore'; +import enhanceParameters from './enhanceParameters'; + let isOffline = false; let isQueuePaused = false; let persistedRequestsQueueRunning = false; @@ -16,11 +17,6 @@ let persistedRequestsQueueRunning = false; // Queue for network requests so we don't lose actions done by the user while offline let networkRequestQueue = []; -// This is an optional function that this lib can be configured with (via registerParameterEnhancer()) -// that accepts all request parameters and returns a new copy of them. This allows other code to inject -// parameters such as authTokens or CSRF tokens, etc. -let enhanceParameters; - // These handlers must be registered so we can process the request, response, and errors returned from the queue. // The first argument passed will be the queuedRequest object and the second will be either the parameters, response, or error. const [onRequest, registerRequestHandler] = createCallback(); @@ -39,9 +35,7 @@ const [recheckConnectivity, registerConnectionCheckCallback] = createCallback(); * @returns {Promise} */ function processRequest(request) { - const finalParameters = _.isFunction(enhanceParameters) - ? enhanceParameters(request.command, request.data) - : request.data; + const finalParameters = enhanceParameters(request.command, request.data); // If request is still in processing after this time, we might be offline const timerId = setTimeout(recheckConnectivity, CONST.NETWORK.MAX_PENDING_TIME_MS); @@ -51,6 +45,14 @@ function processRequest(request) { .finally(() => clearTimeout(timerId)); } +/** + * This method will get any persisted requests and fire them off in parallel to retry them. + * If we get any jsonCode besides 407 the request is a success. It doesn't make sense to + * continually retry things that have returned a response. However, we can retry any requests + * with known networking errors like "Failed to fetch". + * + * @returns {Promise} + */ function processPersistedRequestsQueue() { const persistedRequests = NetworkRequestQueue.getPersistedRequests(); @@ -61,17 +63,32 @@ function processPersistedRequestsQueue() { const tasks = _.map(persistedRequests, request => processRequest(request) .then((response) => { - if (response.jsonCode !== CONST.HTTP_STATUS_CODE.SUCCESS) { - throw new Error(`Persisted request failed due to jsonCode: ${response.jsonCode}`); + if (response.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) { + getLogger().info('Persisted optimistic request needs authentication'); + } else { + getLogger().info('Persisted optimistic request returned a valid jsonCode. Not retrying.'); } - + onResponse(request, response); NetworkRequestQueue.removeRetryableRequest(request); }) .catch((error) => { - const retryCount = NetworkRequestQueue.incrementRetries(request); - getLogger().info('Persisted request failed', false, {retryCount, command: request.command, error: error.message}); - if (retryCount >= CONST.NETWORK.MAX_REQUEST_RETRIES) { - // Request failed too many times removing from persisted storage + // If we are catching a known network error like "Failed to fetch" allow this request to be retried if we have retries left + if (error.message === CONST.ERROR.FAILED_TO_FETCH) { + const retryCount = NetworkRequestQueue.incrementRetries(request); + getLogger().info('Persisted request failed', false, {retryCount, command: request.command, error: error.message}); + if (retryCount >= CONST.NETWORK.MAX_REQUEST_RETRIES) { + getLogger().info('Request failed too many times removing from storage', false, {retryCount, command: request.command, error: error.message}); + NetworkRequestQueue.removeRetryableRequest(request); + } + } else if (error.name === CONST.ERROR.REQUEST_CANCELLED) { + getLogger().info('Persisted request was cancelled. Not retrying.'); + onError(request); + NetworkRequestQueue.removeRetryableRequest(request); + } else { + getLogger().alert(`${CONST.ERROR.ENSURE_BUGBOT} unknown error while retrying persisted request. Not retrying.`, { + command: request.command, + error: error.message, + }); NetworkRequestQueue.removeRetryableRequest(request); } })); @@ -123,13 +140,6 @@ Onyx.connect({ }, }); -/** - * @param {Boolean} val - */ -function setIsReady(val) { - isReady = val; -} - /** * Checks to see if a request can be made. * @@ -141,7 +151,7 @@ function setIsReady(val) { * @return {Boolean} */ function canMakeRequest(request) { - if (!isReady) { + if (!NetworkStore.isReady()) { onRequestSkipped({command: request.command, type: request.type}); return false; } @@ -236,27 +246,42 @@ function processNetworkRequestQueue() { processRequest(queuedRequest) .then(response => onResponse(queuedRequest, response)) .catch((error) => { + // Cancelled requests should not be retried + if (error.name === CONST.ERROR.REQUEST_CANCELLED) { + onError(queuedRequest, error); + return; + } + recheckConnectivity(); - // When the request did not reach its destination add it back the queue to be retried - const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry'); - if (shouldRetry && error.name !== CONST.ERROR.REQUEST_CANCELLED) { - const retryCount = NetworkRequestQueue.incrementRetries(queuedRequest); - getLogger().info('A retrieable request failed', false, { - retryCount, + // Retry any request that returns a "Failed to fetch" error. Very common if a user is offline or experiencing an unlikely scenario + // like incorrect url, bad cors headers returned by the server, DNS lookup failure etc. + if (error.message === CONST.ERROR.FAILED_TO_FETCH) { + // When the request did not reach its destination add it back the queue to be retried if we can + const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry'); + if (shouldRetry) { + const retryCount = NetworkRequestQueue.incrementRetries(queuedRequest); + getLogger().info('A retryable request failed', false, { + retryCount, + command: queuedRequest.command, + error: error.message, + }); + + if (retryCount < CONST.NETWORK.MAX_REQUEST_RETRIES) { + networkRequestQueue.push(queuedRequest); + return; + } + + getLogger().info('Request was retried too many times with no success. No more retries left'); + } + + onError(queuedRequest, error); + } else { + getLogger().alert(`${CONST.ERROR.ENSURE_BUGBOT} unknown error caught while processing request`, { command: queuedRequest.command, error: error.message, }); - - if (retryCount < CONST.NETWORK.MAX_REQUEST_RETRIES) { - networkRequestQueue.push(queuedRequest); - return; - } - - getLogger().info('Request was retried too many times with no success. No more retries left'); } - - onError(queuedRequest, error); }); }); @@ -337,23 +362,12 @@ function unpauseRequestQueue() { isQueuePaused = false; } -/** - * Register a function that will accept all the parameters being sent in a request - * and will return a new set of parameters to send instead. Useful for adding data to every request - * like auth or CRSF tokens. - * - * @param {Function} callback - */ -function registerParameterEnhancer(callback) { - enhanceParameters = callback; -} - /** * Clear the queue and cancels all pending requests * Non-cancellable requests like Log would not be cleared */ function clearRequestQueue() { - networkRequestQueue = _.filter(networkRequestQueue, r => !r.data.canCancel); + networkRequestQueue = _.filter(networkRequestQueue, request => !request.data.canCancel); HttpUtils.cancelPendingRequests(); } @@ -361,12 +375,10 @@ export { post, pauseRequestQueue, unpauseRequestQueue, - registerParameterEnhancer, clearRequestQueue, registerResponseHandler, registerErrorHandler, registerRequestHandler, - setIsReady, registerRequestSkippedHandler, registerLogHandler, registerConnectionCheckCallback, diff --git a/src/libs/actions/Session/index.js b/src/libs/actions/Session/index.js index d58583855e66..c508ca2a72ef 100644 --- a/src/libs/actions/Session/index.js +++ b/src/libs/actions/Session/index.js @@ -385,13 +385,13 @@ function changePasswordAndSignIn(authToken, password) { signIn(password); return; } - if (responsePassword.jsonCode === 407 && !credentials.login) { + if (responsePassword.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED && !credentials.login) { // authToken has expired, and we don't have the email set to request a new magic link. // send user to login page to enter email. Navigation.navigate(ROUTES.HOME); return; } - if (responsePassword.jsonCode === 407) { + if (responsePassword.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) { // authToken has expired, and we have the account email, so we request a new magic link. Onyx.merge(ONYXKEYS.ACCOUNT, {accountExists: true, validateCodeExpired: true, error: null}); resetPassword(); @@ -478,7 +478,7 @@ function authenticatePusher(socketID, channelName, callback) { forceNetworkRequest: true, }) .then((data) => { - if (data.jsonCode === 407) { + if (data.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) { callback(new Error('Expensify session expired'), {auth: ''}); // Attempt to refresh the authToken then reconnect to Pusher @@ -486,7 +486,7 @@ function authenticatePusher(socketID, channelName, callback) { return; } - if (data.jsonCode !== 200) { + if (data.jsonCode !== CONST.JSON_CODE.SUCCESS) { Log.hmmm('[PusherConnectionManager] Unable to authenticate Pusher for some reason other than expired session'); return; } diff --git a/tests/actions/ReimbursementAccountTest.js b/tests/actions/ReimbursementAccountTest.js index c711b0fbf35f..05db25959fcc 100644 --- a/tests/actions/ReimbursementAccountTest.js +++ b/tests/actions/ReimbursementAccountTest.js @@ -6,7 +6,7 @@ import HttpUtils from '../../src/libs/HttpUtils'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import CONST from '../../src/CONST'; import BankAccount from '../../src/libs/models/BankAccount'; -import * as Network from '../../src/libs/Network'; +import * as NetworkStore from '../../src/libs/Network/NetworkStore'; const TEST_BANK_ACCOUNT_ID = 1; const TEST_BANK_ACCOUNT_CITY = 'Opa-locka'; @@ -35,7 +35,7 @@ Onyx.connect({ beforeEach(() => Onyx.clear() .then(() => { - Network.setIsReady(true); + NetworkStore.setIsReady(true); TestHelper.signInWithTestUser(); return waitForPromisesToResolve(); })); diff --git a/tests/actions/SessionTest.js b/tests/actions/SessionTest.js index 62604fd77c67..cf03d8611405 100644 --- a/tests/actions/SessionTest.js +++ b/tests/actions/SessionTest.js @@ -5,6 +5,7 @@ import HttpUtils from '../../src/libs/HttpUtils'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import ONYXKEYS from '../../src/ONYXKEYS'; import * as TestHelper from '../utils/TestHelper'; +import CONST from '../../src/CONST'; // We are mocking this method so that we can later test to see if it was called and what arguments it was called with. // We test HttpUtils.xhr() since this means that our API command turned into a network request and isn't only queued. @@ -57,12 +58,12 @@ test('Authenticate is called with saved credentials when a session expires', () // This will make the call to API.Get() below return with an expired session code .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) // The next call should be Authenticate since we are reauthenticating .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, accountID: TEST_USER_ACCOUNT_ID, authToken: TEST_REFRESHED_AUTH_TOKEN, email: TEST_USER_LOGIN, diff --git a/tests/unit/NetworkTest.js b/tests/unit/NetworkTest.js index 5dc1bb9f7980..c587f220b69a 100644 --- a/tests/unit/NetworkTest.js +++ b/tests/unit/NetworkTest.js @@ -11,8 +11,10 @@ import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import ONYXKEYS from '../../src/ONYXKEYS'; import CONST from '../../src/CONST'; import * as Network from '../../src/libs/Network'; +import * as NetworkStore from '../../src/libs/Network/NetworkStore'; import * as Session from '../../src/libs/actions/Session'; -import * as NetworkQueue from '../../src/libs/actions/NetworkRequestQueue'; +import * as NetworkRequestQueue from '../../src/libs/actions/NetworkRequestQueue'; +import Log from '../../src/libs/Log'; // Set up manual mocks for methods used in the actions so our test does not fail. jest.mock('../../src/libs/Notification/PushNotification', () => ({ @@ -27,10 +29,17 @@ Onyx.init({ keys: ONYXKEYS, }); -beforeEach(() => Onyx.clear().then(waitForPromisesToResolve)); +const originalXHR = HttpUtils.xhr; + +beforeEach(() => { + HttpUtils.xhr = originalXHR; + NetworkRequestQueue.clearPersistedRequests(); + Network.clearRequestQueue(); + return Onyx.clear().then(waitForPromisesToResolve); +}); afterEach(() => { - Network.setIsReady(false); + NetworkStore.setIsReady(false); Onyx.addDelayToConnectCallback(0); jest.clearAllMocks(); }); @@ -61,7 +70,7 @@ test('failing to reauthenticate while offline should not log out user', () => { HttpUtils.xhr = jest.fn(); HttpUtils.xhr .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) // Fail the call to re-authenticate @@ -69,18 +78,18 @@ test('failing to reauthenticate while offline should not log out user', () => { // The next call should still be using the old authToken .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) // Succeed the call to set a new authToken .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, authToken: 'qwerty12345', })) // All remaining requests should succeed .mockImplementation(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, })); // This should first trigger re-authentication and then an API is offline error @@ -158,39 +167,39 @@ test('consecutive API calls eventually succeed when authToken is expired', () => // This will make the first call to API.Get() return with an expired session code .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) // The next 2 API calls will also fire and also return a 407 .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 407, + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, })) // The request to Authenticate should succeed and we mock the responses for the remaining calls .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, authToken: 'qwerty12345', })) // Get&returnValueList=personalDetailsList .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, personalDetailsList: TEST_PERSONAL_DETAILS, })) // Get&returnValueList=account .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, account: TEST_ACCOUNT_DATA, })) // Get&returnValueList=chatList .mockImplementationOnce(() => Promise.resolve({ - jsonCode: 200, + jsonCode: CONST.JSON_CODE.SUCCESS, chatList: TEST_CHAT_LIST, })); @@ -224,11 +233,8 @@ test('consecutive API calls eventually succeed when authToken is expired', () => }); test('retry network request if auth and credentials are not read from Onyx yet', () => { - // For this test we're having difficulty creating a situation where Onyx.connect() has not yet run - // because some Onyx.connect callbacks are already registered in API.js (which happens before this - // unit test is setup), so in order to test an scenario where the auth token and credentials hasn't - // been read from storage we set Network.setIsReady(false) and trigger an update in the Onyx.connect - // callbacks registered in API.js merging an empty object. + // In order to test an scenario where the auth token and credentials hasn't been read from storage we set + // NetworkStore.setIsReady(false) and set the session and credentials to "ready" the Network // Given a test user login and account ID const TEST_USER_LOGIN = 'test@testguy.com'; @@ -238,15 +244,14 @@ test('retry network request if auth and credentials are not read from Onyx yet', Onyx.addDelayToConnectCallback(ONYX_DELAY_MS); // Given initial state to Network - Network.setIsReady(false); + NetworkStore.setIsReady(false); - // Given any initial value to trigger an update - Onyx.merge(ONYXKEYS.CREDENTIALS, {}); - Onyx.merge(ONYXKEYS.SESSION, {}); + // Given an initial value to trigger an update + Onyx.merge(ONYXKEYS.CREDENTIALS, {login: 'test-login'}); + Onyx.merge(ONYXKEYS.SESSION, {authToken: 'test-auth-token'}); // Given some mock functions to track the isReady // flag in Network and the http requests made - const spyNetworkSetIsReady = jest.spyOn(Network, 'setIsReady'); const spyHttpUtilsXhr = jest.spyOn(HttpUtils, 'xhr').mockImplementation(() => Promise.resolve({})); // When we make an arbitrary request that can be retried @@ -254,7 +259,7 @@ test('retry network request if auth and credentials are not read from Onyx yet', Session.fetchAccountDetails(TEST_USER_LOGIN); return waitForPromisesToResolve().then(() => { // Then we expect not having the Network ready and not making an http request - expect(spyNetworkSetIsReady).not.toHaveBeenCalled(); + expect(NetworkStore.isReady()).toBe(false); expect(spyHttpUtilsXhr).not.toHaveBeenCalled(); // When we resolve Onyx.connect callbacks @@ -262,7 +267,7 @@ test('retry network request if auth and credentials are not read from Onyx yet', // Then we should expect call Network.setIsReady(true) // And We should expect not making an http request yet - expect(spyNetworkSetIsReady).toHaveBeenLastCalledWith(true); + expect(NetworkStore.isReady()).toBe(true); expect(spyHttpUtilsXhr).not.toHaveBeenCalled(); // When we run processNetworkRequestQueue in the setInterval of Network.js @@ -278,11 +283,11 @@ test('retry network request if connection is lost while request is running', () const xhr = jest.spyOn(HttpUtils, 'xhr') .mockImplementationOnce(() => { Onyx.merge(ONYXKEYS.NETWORK, {isOffline: true}); - return Promise.reject(new Error('Network request failed')); + return Promise.reject(new Error(CONST.ERROR.FAILED_TO_FETCH)); }) - .mockResolvedValue({jsonCode: 200, fromRetriedResult: true}); + .mockResolvedValue({jsonCode: CONST.JSON_CODE.SUCCESS, fromRetriedResult: true}); - // Given a regular "retriable" request (that is bound to fail) + // Given a regular "retryable" request (that is bound to fail) const promise = Network.post('Get'); return waitForPromisesToResolve() @@ -301,7 +306,7 @@ test('retry network request if connection is lost while request is running', () expect(xhr).toHaveBeenCalledTimes(2); // And the promise should be resolved with the 2nd call that succeeded - return expect(promise).resolves.toEqual({jsonCode: 200, fromRetriedResult: true}); + return expect(promise).resolves.toEqual({jsonCode: CONST.JSON_CODE.SUCCESS, fromRetriedResult: true}); }); }); @@ -322,7 +327,7 @@ test('requests should be persisted while offline', () => { // Then `xhr` should not be used and requests should be persisted to storage expect(xhr).not.toHaveBeenCalled(); - const persisted = NetworkQueue.getPersistedRequests(); + const persisted = NetworkRequestQueue.getPersistedRequests(); expect(persisted).toEqual([ expect.objectContaining({command: 'mock command', data: expect.objectContaining({param1: 'value1'})}), expect.objectContaining({command: 'mock command', data: expect.objectContaining({param3: 'value3'})}), @@ -332,7 +337,7 @@ test('requests should be persisted while offline', () => { test('requests should resume when we are online', () => { // We're setting up a basic case where all requests succeed when we resume connectivity - const xhr = jest.spyOn(HttpUtils, 'xhr').mockResolvedValue({jsonCode: 200}); + const xhr = jest.spyOn(HttpUtils, 'xhr').mockResolvedValue({jsonCode: CONST.JSON_CODE.SUCCESS}); // Given we have some requests made while we're offline return Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}) @@ -354,12 +359,12 @@ test('requests should resume when we are online', () => { expect.arrayContaining(['mock command', expect.objectContaining({param2: 'value2'})]), ]); - const persisted = NetworkQueue.getPersistedRequests(); + const persisted = NetworkRequestQueue.getPersistedRequests(); expect(persisted).toEqual([]); }); }); -test('persisted request should not be cleared unit a backend response', () => { +test('persisted request should not be cleared until a backend response occurs', () => { // We're setting up xhr handler that will resolve calls programmatically const xhrCalls = []; const promises = []; @@ -386,41 +391,41 @@ test('persisted request should not be cleared unit a backend response', () => { .then(() => Onyx.set(ONYXKEYS.NETWORK, {isOffline: false})) .then(() => { // Then requests should remain persisted until the xhr call is resolved - expect(_.size(NetworkQueue.getPersistedRequests())).toEqual(2); + expect(_.size(NetworkRequestQueue.getPersistedRequests())).toEqual(2); - xhrCalls[0].resolve({jsonCode: 200}); + xhrCalls[0].resolve({jsonCode: CONST.JSON_CODE.SUCCESS}); return waitForPromisesToResolve(); }) .then(waitForPromisesToResolve) .then(() => { - expect(_.size(NetworkQueue.getPersistedRequests())).toEqual(1); - expect(NetworkQueue.getPersistedRequests()).toEqual([ + expect(_.size(NetworkRequestQueue.getPersistedRequests())).toEqual(1); + expect(NetworkRequestQueue.getPersistedRequests()).toEqual([ expect.objectContaining({command: 'mock command', data: expect.objectContaining({param2: 'value2'})}), ]); // When a request fails it should be retried - xhrCalls[1].resolve({jsonCode: 401}); + xhrCalls[1].reject(new Error(CONST.ERROR.FAILED_TO_FETCH)); return waitForPromisesToResolve(); }) .then(() => { - expect(_.size(NetworkQueue.getPersistedRequests())).toEqual(1); - expect(NetworkQueue.getPersistedRequests()).toEqual([ + expect(_.size(NetworkRequestQueue.getPersistedRequests())).toEqual(1); + expect(NetworkRequestQueue.getPersistedRequests()).toEqual([ expect.objectContaining({command: 'mock command', data: expect.objectContaining({param2: 'value2'})}), ]); // Finally, after it succeeds the queue should be empty - xhrCalls[2].resolve({jsonCode: 200}); + xhrCalls[2].resolve({jsonCode: CONST.JSON_CODE.SUCCESS}); return waitForPromisesToResolve(); }) .then(() => { - expect(_.size(NetworkQueue.getPersistedRequests())).toEqual(0); + expect(_.size(NetworkRequestQueue.getPersistedRequests())).toEqual(0); }); }); test(`persisted request should be retried up to ${CONST.NETWORK.MAX_REQUEST_RETRIES} times`, () => { - // We're setting up xhr handler that always returns an error response + // We're setting up xhr handler that always rejects with a fetch error const xhr = jest.spyOn(HttpUtils, 'xhr') - .mockResolvedValue({jsonCode: 401}); + .mockRejectedValue(new Error(CONST.ERROR.FAILED_TO_FETCH)); // Given we have a request made while we're offline return Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}) @@ -443,3 +448,77 @@ test(`persisted request should be retried up to ${CONST.NETWORK.MAX_REQUEST_RETR }); }); }); + +test('test bad response will log alert', () => { + global.fetch = jest.fn() + .mockResolvedValueOnce({ok: false, status: 502, statusText: 'Bad Gateway'}); + + const logAlertSpy = jest.spyOn(Log, 'alert'); + + // Given we have a request made while online + return Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}) + .then(() => { + Network.post('MockBadNetworkResponse', {param1: 'value1'}); + return waitForPromisesToResolve(); + }) + .then(() => { + expect(logAlertSpy).toHaveBeenCalled(); + }); +}); + +test('test Failed to fetch error for requests not flagged with shouldRetry will throw API OFFLINE error', () => { + // Setup xhr handler that rejects once with a 502 Bad Gateway + global.fetch = jest.fn(() => new Promise((_resolve, reject) => reject(new Error(CONST.ERROR.FAILED_TO_FETCH)))); + + const onRejected = jest.fn(); + + // Given we have a request made while online + return Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}) + .then(() => { + // When network calls with are made + Network.post('mock command', {param1: 'value1', shouldRetry: false}) + .catch(onRejected); + + return waitForPromisesToResolve(); + }) + .then(() => { + const error = onRejected.mock.calls[0][0]; + expect(onRejected).toHaveBeenCalled(); + expect(error.message).toBe(CONST.ERROR.API_OFFLINE); + }); +}); + +test('persisted request can trigger reauthentication for anything retryable', () => { + // We're setting up xhr handler that rejects once with a 407 code and again with success + const xhr = jest.spyOn(HttpUtils, 'xhr') + .mockResolvedValue({jsonCode: CONST.JSON_CODE.SUCCESS}) // Default + .mockResolvedValueOnce({jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED}) // Initial call to test command return 407 + .mockResolvedValueOnce({jsonCode: CONST.JSON_CODE.SUCCESS}) // Call to Authenticate return 200 + .mockResolvedValueOnce({jsonCode: CONST.JSON_CODE.SUCCESS}); // Original command return 200 + + // Given we have a request made while we're offline and we have credentials available to reauthenticate + Onyx.merge(ONYXKEYS.CREDENTIALS, {autoGeneratedLogin: 'caca', autoGeneratedPassword: 'caca'}); + return waitForPromisesToResolve() + .then(() => Onyx.set(ONYXKEYS.NETWORK, {isOffline: true})) + .then(() => { + Network.post('Mock', {param1: 'value1', persist: true, shouldRetry: true}); + return waitForPromisesToResolve(); + }) + + // When we resume connectivity + .then(() => Onyx.set(ONYXKEYS.NETWORK, {isOffline: false})) + .then(waitForPromisesToResolve) + .then(() => { + const nonLogCalls = _.filter(xhr.mock.calls, ([commandName]) => commandName !== 'Log'); + + // The request should be retried once and reauthenticate should be called the second time + // expect(xhr).toHaveBeenCalledTimes(3); + const [call1, call2, call3] = nonLogCalls; + const [commandName1] = call1; + const [commandName2] = call2; + const [commandName3] = call3; + expect(commandName1).toBe('Mock'); + expect(commandName2).toBe('Authenticate'); + expect(commandName3).toBe('Mock'); + }); +}); diff --git a/tests/unit/enhanceParametersTest.js b/tests/unit/enhanceParametersTest.js new file mode 100644 index 000000000000..a4fc9f70a9cf --- /dev/null +++ b/tests/unit/enhanceParametersTest.js @@ -0,0 +1,44 @@ +import Onyx from 'react-native-onyx'; +import ONYXKEYS from '../../src/ONYXKEYS'; +import enhanceParameters from '../../src/libs/Network/enhanceParameters'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import CONFIG from '../../src/CONFIG'; + +beforeEach(() => Onyx.clear()); + +test('Enhance parameters adds correct parameters for Log command with no authToken', () => { + const command = 'Log'; + const parameters = {testParameter: 'test'}; + const email = 'test-user@test.com'; + const authToken = 'test-token'; + Onyx.merge(ONYXKEYS.SESSION, {email, authToken}); + return waitForPromisesToResolve() + .then(() => { + const finalParameters = enhanceParameters(command, parameters); + expect(finalParameters).toEqual({ + testParameter: 'test', + api_setCookie: false, + email, + referer: CONFIG.EXPENSIFY.EXPENSIFY_CASH_REFERER, + }); + }); +}); + +test('Enhance parameters adds correct parameters for a command that requires authToken', () => { + const command = 'Report_AddComment'; + const parameters = {testParameter: 'test'}; + const email = 'test-user@test.com'; + const authToken = 'test-token'; + Onyx.merge(ONYXKEYS.SESSION, {email, authToken}); + return waitForPromisesToResolve() + .then(() => { + const finalParameters = enhanceParameters(command, parameters); + expect(finalParameters).toEqual({ + testParameter: 'test', + api_setCookie: false, + email, + authToken, + referer: CONFIG.EXPENSIFY.EXPENSIFY_CASH_REFERER, + }); + }); +});