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

Feature Request - Add a background task that would push data to the backend for offline messages - Reported by: @kidroca #6690

Closed
mvtglobally opened this issue Dec 10, 2021 · 10 comments

Comments

@mvtglobally
Copy link

mvtglobally commented Dec 10, 2021

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


Context

We have logic to persist/retry message requests that didn't make it, like for example when you write a message while you're offline
We would usually send those message when you're back online, but if you decide to quit or suspend the app, we'll only send them the next time you open Expensify
You could switch to Desktop or not open the mobile app any time soon delaying the delivery of the persisted messages

Action Performed:

  1. Put the app in offline mode
  2. Send few messages while offline,
  3. Kill the mobile app
  4. Wait 15+ minutes

Expected Result:

background tasks should run and push offline messages automatically within some time

Actual Result:

if user decides to quit or suspend the app, we'll only send offline messages the next time user opens Expensify

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.18-0
Reproducible in staging?: Y
Reproducible in production?: Y
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: https://expensify.slack.com/archives/C01GTK53T8Q/p1638901022330100

View all open jobs on GitHub

@mvtglobally mvtglobally added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Dec 10, 2021
@MelvinBot
Copy link

Triggered auto assignment to @sonialiap (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Dec 10, 2021
@kidroca
Copy link
Contributor

kidroca commented Dec 10, 2021

This ticket is related #4264 but it focuses on pulling data in the background
I see we lean towards using react-native-background-fetch and it can cover our needs here as well:

Background Fetch now provides a scheduleTask method for scheduling arbitrary "one-shot" or periodic tasks.


In case we're not going with react-native-background-fetch whatever is ultimately used for #4264 should be used here as well

@kidroca
Copy link
Contributor

kidroca commented Dec 10, 2021

Another related ticket is this one #5987
Here we define a persisted requests queue.
The queue takes care of sending any requests that were persisted to storage. Then it would remove each completed requests from persistent storage

All we have to do here is trigger the flush of this queue from a periodic background task

  • if there are persisted requests they would be sent and cleaned up

A plus of the new persisted queue (#5987) is it returns a promise, which can be used to signal the task runner that we're done
As shown in the example

// IMPORTANT:  You must signal to the OS that your task is complete.
BackgroundFetch.finish(taskId);

We have about 30s to perform a background task, when we're done we need to let the system know

@kidroca
Copy link
Contributor

kidroca commented Dec 13, 2021

I're researched the topic a bit more, and now I don't think react-native-fetch-blob and the background task that it provide is suitable here due to

  • 30 sec limitation
  • iOS scheduleTask seems only to fire when the device is plugged into power.

We have the intention of supporting large attachments, and so uploading some would probably take much more than 30sec.

This library specializes in background file upload, and doesn't have any time restrictions as it uses upload specific handling: https://bestofreactjs.com/repo/Vydia-react-native-background-upload-react-native-backend
The upload would continue even if the app is terminated by the os

The interface might also be better, especially for large attachments

  1. We can instantly write an attachment comment
  2. Then start an upload (from the foreground)
  3. If the user stays on the chat we can show the upload progress on the attachment comment
  4. If the app is backgrounded the upload continues in the background

In the end a combination of the two might be the optimal solution

  1. A background-fetch event wakes up the app
  2. We check if we have anything to upload
  3. We start the upload with react-native-background-upload and finish the background-fetch task

Note for 1) - we're not using the scheduleTask but react to a background fetch event that happens from time to time

@MelvinBot
Copy link

@sonialiap Whoops! This issue is 2 days overdue. Let's get this updated quick!

@sonialiap
Copy link
Contributor

LGTM!

@MelvinBot MelvinBot removed the Overdue label Dec 14, 2021
@sonialiap sonialiap removed their assignment Dec 14, 2021
@MelvinBot
Copy link

Triggered auto assignment to @marcaaron (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@marcaaron marcaaron removed their assignment Dec 15, 2021
@marcaaron
Copy link
Contributor

Sorry, don't have time to assess whether this is a problem or not - but took a quick look and seems a bit complicated compared to the value that would be added. Going to leave this open to see if anyone in the engineering pool has interest in investigating further.

@marcaaron
Copy link
Contributor

@mvtglobally actually, sorry I'm going to close this. We need to document this process better, but my understanding is that "Feature Requests" are supposed to gather feedback in Slack about whether they are valuable to do before heading to GitHub. Only obvious bugs should be making it into the triage process. Thanks!

@kidroca
Copy link
Contributor

kidroca commented Dec 16, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants