Skip to content

Commit

Permalink
Merge pull request #9101 from Expensify/marcaaron-stopRetryingCancell…
Browse files Browse the repository at this point in the history
…edRequests

Handle cancelled requests correctly
  • Loading branch information
neil-marcellini authored May 20, 2022
2 parents bb8d32b + a24b1f2 commit ff42d7a
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 3 deletions.
4 changes: 3 additions & 1 deletion src/libs/Middleware/Logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ function Logging(response, request) {
if (persisted) {
PersistedRequests.remove(request);
}
return;

// Re-throw this error so the next handler can manage it
throw error;
}

if (error.message === CONST.ERROR.FAILED_TO_FETCH) {
Expand Down
6 changes: 6 additions & 0 deletions src/libs/Middleware/Reauthentication.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ import Log from '../Log';
function Reauthentication(response, request, isFromSequentialQueue) {
return response
.then((data) => {
// If there is no data for some reason then we cannot reauthenticate
if (!data) {
Log.hmmm('Undefined data in Reauthentication');
return;
}

if (NetworkStore.isOffline()) {
// If we are offline and somehow handling this response we do not want to reauthenticate
throw new Error('Unable to reauthenticate because we are offline');
Expand Down
7 changes: 5 additions & 2 deletions src/libs/Middleware/RecheckConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ function RecheckConnection(response) {
const cancelRequestTimeoutTimer = startRecheckTimeoutTimer();
return response
.catch((error) => {
// Because we ran into an error we assume we might be offline and do a "connection" health test
NetworkConnection.recheckNetworkConnection();
if (error.name !== CONST.ERROR.REQUEST_CANCELLED) {
// Because we ran into an error we assume we might be offline and do a "connection" health test
NetworkConnection.recheckNetworkConnection();
}

throw error;
})
.finally(cancelRequestTimeoutTimer);
Expand Down
5 changes: 5 additions & 0 deletions src/libs/Middleware/Retry.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ function retryFailedRequest(queuedRequest, error) {
function Retry(response, request, isFromSequentialQueue) {
return response
.catch((error) => {
// Do not retry any requests that are cancelled
if (error.name === CONST.ERROR.REQUEST_CANCELLED) {
return;
}

if (isFromSequentialQueue) {
const retryCount = PersistedRequests.incrementRetries(request);
Log.info('Persisted request failed', false, {retryCount, command: request.command, error: error.message});
Expand Down
8 changes: 8 additions & 0 deletions src/libs/Network/MainQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,17 @@ function clear() {
HttpUtils.cancelPendingRequests();
}

/**
* @returns {Array}
*/
function getAll() {
return networkRequestQueue;
}

export {
clear,
replay,
push,
process,
getAll,
};
24 changes: 24 additions & 0 deletions tests/unit/NetworkTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -765,3 +765,27 @@ test('persistable request will move directly to the SequentialQueue when we are
expect(thirdRequestCommandName).toBe('MockCommandTwo');
});
});

test('cancelled requests should not be retried', () => {
const xhr = jest.spyOn(HttpUtils, 'xhr');

// GIVEN a mock that will return a "cancelled" request error
global.fetch = jest.fn()
.mockRejectedValue(new DOMException('Aborted', CONST.ERROR.REQUEST_CANCELLED));

return Onyx.set(ONYXKEYS.NETWORK, {isOffline: false})
.then(() => {
// WHEN we make a few requests and then cancel them
Network.post('MockCommandOne');
Network.post('MockCommandTwo');
Network.post('MockCommandThree');

// WHEN we wait for the requests to all cancel
return waitForPromisesToResolve();
})
.then(() => {
// THEN expect our queue to be empty and for no requests to have been retried
expect(MainQueue.getAll().length).toBe(0);
expect(xhr.mock.calls.length).toBe(3);
});
});

0 comments on commit ff42d7a

Please sign in to comment.