From f948f6c399fdc2d3d1cd1245a2f4097fd13a8597 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 2 Oct 2020 14:56:09 -0700 Subject: [PATCH 1/9] Bubble api errors up --- src/lib/API.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index 881628585124b..b64f3b3bfc03f 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -105,12 +105,13 @@ function createLogin(login, password) { * @returns {Promise} */ function queueRequest(command, data) { - return new Promise((resolve) => { + return new Promise((resolve, reject) => { // Add the write request to a queue of actions to perform networkRequestQueue.push({ command, data, callback: resolve, + reject, }); // Try to fire off the request as soon as it's queued so we don't add a delay to every queued command @@ -237,6 +238,9 @@ function request(command, parameters, type = 'post') { if (parametersWithAuthToken.doNotRetry !== true) { queueRequest(command, parametersWithAuthToken); } + + // Throw an error so we can pass the error up the chain + throw new Error(); }); } @@ -263,7 +267,8 @@ function processNetworkRequestQueue() { _.each(networkRequestQueue, (queuedRequest) => { request(queuedRequest.command, queuedRequest.data) - .then(queuedRequest.callback); + .then(queuedRequest.callback) + .catch(queuedRequest.reject); }); networkRequestQueue = []; From 81488dafd57203dbe7fd1cd751727778644e625a Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 2 Oct 2020 16:09:08 -0700 Subject: [PATCH 2/9] Move authenticate requests out of the network queue --- src/lib/API.js | 54 ++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index b64f3b3bfc03f..c91b17fdd95d4 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -153,34 +153,6 @@ function request(command, parameters, type = 'post') { return queueRequest(command, parameters); } - // We treat Authenticate in a special way because unlike other commands, this one can't fail - // with 407 authToken expired. When other api commands fail with this error we call Authenticate - // to get a new authToken and then fire the original api command again - if (command === 'Authenticate') { - return xhr(command, parameters, type) - .then((response) => { - // If we didn't get a 200 response from authenticate we either failed to authenticate with - // an expensify login or the login credentials we created after the initial authentication. - // In both cases, we need the user to sign in again with their expensify credentials - if (response.jsonCode !== 200) { - throw new Error(response.message); - } - - // Update the authToken so it's used in the call to createLogin below - authToken = response.authToken; - return response; - }) - .then((response) => { - // If Expensify login, it's the users first time signing in and we need to - // create a login for the user - if (parameters.useExpensifyLogin) { - return createLogin(Str.generateDeviceLoginID(), Guid()) - .then(() => setSuccessfulSignInData(response, parameters.exitTo)); - } - }) - .catch(error => Ion.merge(IONKEYS.SESSION, {error: error.message})); - } - // Add authToken automatically to all commands const parametersWithAuthToken = {...parameters, ...{authToken}}; @@ -360,7 +332,11 @@ function getAuthToken() { */ function authenticate(parameters) { Ion.merge(IONKEYS.SESSION, {loading: true, error: ''}); - return queueRequest('Authenticate', { + + // We treat Authenticate in a special way because unlike other commands, this one can't fail + // with 407 authToken expired. When other api commands fail with this error we call Authenticate + // to get a new authToken and then fire the original api command again + return xhr('Authenticate', { // When authenticating for the first time, we pass useExpensifyLogin as true so we check for credentials for // the expensify partnerID to let users authenticate with their expensify user and password. useExpensifyLogin: true, @@ -371,11 +347,29 @@ function authenticate(parameters) { twoFactorAuthCode: parameters.twoFactorAuthCode, exitTo: parameters.exitTo, }) + .then((response) => { + // If we didn't get a 200 response from authenticate we either failed to authenticate with + // an expensify login or the login credentials we created after the initial authentication. + // In both cases, we need the user to sign in again with their expensify credentials + if (response.jsonCode !== 200) { + throw new Error(response.message); + } + + // Update the authToken so it's used in the call to createLogin below + authToken = response.authToken; + return response; + }) + .then(response => ( + // It's the users first time signing in and we need to create a login for the user + createLogin(Str.generateDeviceLoginID(), Guid()) + .then(() => setSuccessfulSignInData(response, parameters.exitTo)) + )) .catch((error) => { console.error(error); console.debug('[SIGNIN] Request error'); Ion.merge(IONKEYS.SESSION, {error: error.message}); - }).finally(() => { + }) + .finally(() => { Ion.merge(IONKEYS.SESSION, {loading: false}); }); } From ec57288c98c32a7dd5fe4213404fecb227d4ad79 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 2 Oct 2020 16:20:27 -0700 Subject: [PATCH 3/9] Style --- src/lib/API.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/API.js b/src/lib/API.js index c91b17fdd95d4..90128f1e82c10 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -360,6 +360,7 @@ function authenticate(parameters) { return response; }) .then(response => ( + // It's the users first time signing in and we need to create a login for the user createLogin(Str.generateDeviceLoginID(), Guid()) .then(() => setSuccessfulSignInData(response, parameters.exitTo)) From 3606f2e96351a40792ead4b5085991cb540b4a21 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Oct 2020 11:10:03 -0700 Subject: [PATCH 4/9] PR feedback --- src/lib/API.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index 90128f1e82c10..e7f196921c40f 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -110,7 +110,7 @@ function queueRequest(command, data) { networkRequestQueue.push({ command, data, - callback: resolve, + resolve, reject, }); @@ -212,7 +212,7 @@ function request(command, parameters, type = 'post') { } // Throw an error so we can pass the error up the chain - throw new Error(); + throw new Error(`API Command ${command} failed`); }); } @@ -239,7 +239,7 @@ function processNetworkRequestQueue() { _.each(networkRequestQueue, (queuedRequest) => { request(queuedRequest.command, queuedRequest.data) - .then(queuedRequest.callback) + .then(queuedRequest.resolve) .catch(queuedRequest.reject); }); @@ -355,12 +355,11 @@ function authenticate(parameters) { throw new Error(response.message); } - // Update the authToken so it's used in the call to createLogin below + // Update the authToken so it's used in the call to createLogin below authToken = response.authToken; return response; }) .then(response => ( - // It's the users first time signing in and we need to create a login for the user createLogin(Str.generateDeviceLoginID(), Guid()) .then(() => setSuccessfulSignInData(response, parameters.exitTo)) @@ -370,9 +369,7 @@ function authenticate(parameters) { console.debug('[SIGNIN] Request error'); Ion.merge(IONKEYS.SESSION, {error: error.message}); }) - .finally(() => { - Ion.merge(IONKEYS.SESSION, {loading: false}); - }); + .finally(() => Ion.merge(IONKEYS.SESSION, {loading: false})); } /** From 76d018d8309a591d7d7f09a7a0c6ca1b023a78ce Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Oct 2020 11:19:09 -0700 Subject: [PATCH 5/9] Style --- src/lib/API.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/API.js b/src/lib/API.js index e7f196921c40f..c7e801c79b6a4 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -359,8 +359,9 @@ function authenticate(parameters) { authToken = response.authToken; return response; }) + + // It's the users first time signing in and we need to create a login for the user .then(response => ( - // It's the users first time signing in and we need to create a login for the user createLogin(Str.generateDeviceLoginID(), Guid()) .then(() => setSuccessfulSignInData(response, parameters.exitTo)) )) From 00f75ce168f0232286d0a191134b55d9395a4471 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Oct 2020 11:32:12 -0700 Subject: [PATCH 6/9] Remove unneeded comment --- src/lib/API.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index c7e801c79b6a4..147474200deff 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -359,8 +359,6 @@ function authenticate(parameters) { authToken = response.authToken; return response; }) - - // It's the users first time signing in and we need to create a login for the user .then(response => ( createLogin(Str.generateDeviceLoginID(), Guid()) .then(() => setSuccessfulSignInData(response, parameters.exitTo)) From b9f0b620556538057deac16f3a97a34c4ada6f6b Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Oct 2020 11:42:03 -0700 Subject: [PATCH 7/9] Readded comment --- src/lib/API.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/API.js b/src/lib/API.js index 2855966138096..c9ea93b3a07ae 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -372,6 +372,9 @@ function authenticate(parameters) { authToken = response.authToken; return response; }) + + // After the user authenticates, create a new login for the user so that we can reauthenticate when the + // authtoken expires .then(response => ( createLogin(Str.generateDeviceLoginID(), Guid()) .then(() => setSuccessfulSignInData(response, parameters.exitTo)) From 2b38e49865936d15c6a776b660c94e58a8b58381 Mon Sep 17 00:00:00 2001 From: Alexander Date: Mon, 5 Oct 2020 16:02:20 -0700 Subject: [PATCH 8/9] don't swallow errors --- src/lib/API.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lib/API.js b/src/lib/API.js index c9ea93b3a07ae..5ffced0f4494c 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -217,14 +217,19 @@ function request(command, parameters, type = 'post') { } return responseData; }) - .catch(() => { + .catch((error) => { // If the request failed, we need to put the request object back into the queue as long as there is no // doNotRetry option set in the parametersWithAuthToken if (parametersWithAuthToken.doNotRetry !== true) { queueRequest(command, parametersWithAuthToken); } - // Throw an error so we can pass the error up the chain + // If we already have an error, throw that so we do not swallow it + if (error && error instanceof Error) { + throw error; + } + + // Throw a generic error so we can pass the error up the chain throw new Error(`API Command ${command} failed`); }); } From f6d86a3c342663bf216cd0f16a33168e325263cb Mon Sep 17 00:00:00 2001 From: Alexander Date: Tue, 6 Oct 2020 16:56:15 -0700 Subject: [PATCH 9/9] Remove unneeded check --- src/lib/API.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/API.js b/src/lib/API.js index 5ffced0f4494c..b91d715f1f8b2 100644 --- a/src/lib/API.js +++ b/src/lib/API.js @@ -225,7 +225,7 @@ function request(command, parameters, type = 'post') { } // If we already have an error, throw that so we do not swallow it - if (error && error instanceof Error) { + if (error instanceof Error) { throw error; }