Bubble API errors up. #588
Conversation
| return response; | ||
| }) | ||
| .then(response => ( | ||
|
|
There was a problem hiding this comment.
Linter wanted a blank line here 🙃
There was a problem hiding this comment.
If you move the comment to be outside of the .then() then the blank line before the comment will look nicer
There was a problem hiding this comment.
Better to update the linter to not complain about this when the comment in one level more indented than the previous line
There was a problem hiding this comment.
Couldn't find a rule in the linter that would make this pass. Ended up removing the comment, since (imo) it doesn't add a ton to this function overall
src/lib/API.js
Outdated
| @@ -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, | |||
There was a problem hiding this comment.
To make this code a little more consistent, rename callback to resolve.
src/lib/API.js
Outdated
| @@ -237,6 +210,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(); | |||
There was a problem hiding this comment.
Specify an error message with this error so it's not just a generic error.
src/lib/API.js
Outdated
| throw new Error(response.message); | ||
| } | ||
|
|
||
| // Update the authToken so it's used in the call to createLogin below |
There was a problem hiding this comment.
| // 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 |
| return response; | ||
| }) | ||
| .then(response => ( | ||
|
|
There was a problem hiding this comment.
If you move the comment to be outside of the .then() then the blank line before the comment will look nicer
src/lib/API.js
Outdated
| .finally(() => { | ||
| Ion.merge(IONKEYS.SESSION, {loading: false}); | ||
| }); |
There was a problem hiding this comment.
| .finally(() => { | |
| Ion.merge(IONKEYS.SESSION, {loading: false}); | |
| }); | |
| .finally(() => Ion.merge(IONKEYS.SESSION, {loading: false})); |
# Conflicts: # src/lib/API.js
|
Updated |
|
Updated |
src/lib/API.js
Outdated
| // 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); | ||
| } | ||
|
|
||
| // If we already have an error, throw that so we do not swallow it | ||
| if (error && error instanceof Error) { |
|
Updated, removed the extra check that wasn't needed |
# Conflicts: # src/lib/API.js
|
Merge conflicts resolved. |
Jag96
dismissed
tgolen’s stale review
Requested changes were made and Tim is OOO, dismissing
@marcaaron and @cead22 will you review please?
Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/142656
Tests
Screenshots
n/a