Attachment upload requests are only persisted while offline

[thienlnam]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Creating separate ticket for #3867 (comment)

NetworkRequestQueue.saveRetryableRequests is only called in the if(isOffline) block

App/src/libs/Network.js

Lines 185 to 206 in 425e718

	if (isOffline) {
	if (!networkRequestQueue.length) {
	return;
	}
	const retryableRequests = [];
	// If we have a request then we need to check if it can be persisted in case we close the tab while offline.
	// We filter persisted requests from the normal Queue to remove duplicates
	networkRequestQueue = _.reject(networkRequestQueue, (request) => {
	const shouldRetry = lodashGet(request, 'data.shouldRetry');
	if (shouldRetry && request.data.persist) {
	// exclude functions as they cannot be persisted
	const requestToPersist = _.omit(request, val => _.isFunction(val));
	retryableRequests.push(requestToPersist);
	return true;
	}
	});
	if (retryableRequests.length) {
	NetworkRequestQueue.saveRetryableRequests(retryableRequests);
	}
	return;
	}

Recent changes alleviate this to some extent - when a request is persisted to storage it will only be removed after it's successfully performed - it'll be retried until eventually succeeding
But it would only get to be persisted if you were offline to begin with

Action Performed:

1. Try to send a large PDF file (Makes it easier to test)
2. After sending, close the app
3. Re-open the app and see that it was not uploaded

Expected Result:

Attachment would try to re-upload after the app was reopened

Actual Result:

Attachment stopped sending

Workaround:

Stay online while uploading, or start offline for the request to be persisted

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @kidroca
Slack conversation:

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @SofiedeVreese (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[thienlnam]

@kidroca Proposal:

Requests with the persist flag should be persisted regardless of being offline or not
If the user quits the app while the request isn't completed existing logic would retry it on next launch
Otherwise added logic would remove it from persistent storage when it succeeds

The following

App/src/libs/Network.js

Lines 229 to 230 in 425e718

	processRequest(queuedRequest)
	.then(response => onResponse(queuedRequest, response))

becomes

 if (queuedRequest.data.persist) {
   NetworkRequestQueue.saveRetryableRequests([queuedRequest]);
 }

 processRequest(queuedRequest) 
     .then(response => {
        onResponse(queuedRequest, response);
        NetworkRequestQueue.removeRetryableRequest(queuedRequest);
     }) 

[kidroca]

This comment is also relevant here: #7934 (comment)
The new network write queue (yet to be implemented) would by default persist requests and avoid the above mentioned problem

I think we can still start with the above proposal as in both the "new write queue" and this proposal - network requests would be persisted in the body of Network.post

[thienlnam]

Agreed, let's just knock this one out since the changes are small since we don't know when the new write queue will be live

[thienlnam]

🟢 to your proposal

[SofiedeVreese]

Posted on Upwork here: https://www.upwork.com/jobs/~01b8d656edb6997ef1

cc @kidroca