Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Network: Retry requests that failed in flight #6567

Merged
merged 21 commits into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3360e84
NetworkTests: Add test confirming failed requests are not retried
kidroca Dec 2, 2021
3974014
API.requestErrorHandler - retry some requests
kidroca Dec 2, 2021
1371639
API extract reusable retry and shouldRetry functions
kidroca Dec 2, 2021
4029c72
Network: update post method docs
kidroca Dec 2, 2021
24a91ef
Fix API: don't allow more than one auth requests to run at the same time
kidroca Dec 2, 2021
7489bfb
Fix Session: reauthenticate pusher is still causing multiple authenti…
kidroca Dec 2, 2021
b3e3ba6
tests: update expired token test - request order is no longer reversed
kidroca Dec 2, 2021
badccf5
tests: reduce expired token test size
kidroca Dec 2, 2021
2a2c367
tests: Add API and Session tests regarding parallel calls
kidroca Dec 2, 2021
262469d
API: update docs
kidroca Dec 2, 2021
3045aca
API: better names for shouldRetry and retry
kidroca Dec 10, 2021
b1e2a39
Session: revert changes made to reauthenticatePusher
kidroca Dec 10, 2021
d3a9d1c
Merge branch 'main' into kidroca/retry-failed-requests
kidroca Jan 7, 2022
a2b45ec
Revert "Network: update post method docs"
kidroca Jan 7, 2022
6b5a5d1
Revert "Fix API: don't allow more than one auth requests to run at th…
kidroca Jan 7, 2022
99e57e6
Revert "tests: Add API and Session tests regarding parallel calls"
kidroca Jan 7, 2022
2fd5104
Revert "tests: reduce expired token test size"
kidroca Jan 7, 2022
c3a0e5c
Use correct function name
kidroca Jan 7, 2022
13397ea
Revert "API extract reusable retry and shouldRetry functions"
kidroca Jan 7, 2022
470eaaa
Revert "tests: update expired token test - request order is no longer…
kidroca Jan 7, 2022
294a8f1
Network - move the retry on network failure logic
kidroca Jan 12, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/libs/Network.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,16 @@ function processNetworkRequestQueue() {
onRequest(queuedRequest, finalParameters);
HttpUtils.xhr(queuedRequest.command, finalParameters, queuedRequest.type, queuedRequest.shouldUseSecure)
.then(response => onResponse(queuedRequest, response))
.catch(error => onError(queuedRequest, error));
.catch((error) => {
// When the request did not reach its destination add it back the queue to be retried
const shouldRetry = lodashGet(queuedRequest, 'data.shouldRetry');
if (shouldRetry) {
networkRequestQueue.push(queuedRequest);
return;
}

onError(queuedRequest, error);
});
});

// We should clear the NETWORK_REQUEST_QUEUE when we have loaded the persisted requests & they are processed.
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/NetworkTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,3 +271,35 @@ test('retry network request if auth and credentials are not read from Onyx yet',
expect(spyHttpUtilsXhr).toHaveBeenCalled();
});
});

test('retry network request if connection is lost while request is running', () => {
// Given a xhr mock that will fail as if network connection dropped
const xhr = jest.spyOn(HttpUtils, 'xhr')
.mockImplementationOnce(() => {
Onyx.merge(ONYXKEYS.NETWORK, {isOffline: true});
return Promise.reject(new Error('Network request failed'));
})
.mockResolvedValue({jsonCode: 200, fromRetriedResult: true});

// Given a regular "retriable" request (that is bound to fail)
const promise = Network.post('Get');

return waitForPromisesToResolve()
.then(() => {
// When network connection is recovered
Onyx.merge(ONYXKEYS.NETWORK, {isOffline: false});
return waitForPromisesToResolve();
})
.then(() => {
// Advance the network request queue by 1 second so that it can realize it's back online
jest.advanceTimersByTime(1000);
return waitForPromisesToResolve();
})
.then(() => {
// Then the request should be attempted again
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});
});
});