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

Update docs to reference ability to retry headless JS tasks #771

Merged
merged 2 commits into from
Jun 12, 2019

Conversation

JoshuaJamesOng
Copy link
Contributor

Updates documentation to reference alternative solution introduced in facebook/react-native#23231 for usages of setTimeout in a headless JS task, where the intention of the timeout is to block execution of the remainder of the JS task.

@react-native-bot
Copy link

react-native-bot commented Feb 1, 2019

Deploy preview for react-native ready!

Built with commit ee9c1fe

https://deploy-preview-771--react-native.netlify.com

Changes to docs/ are reflected in the next "master" version.

Thank you for your contributions.

How to ContributeDocumentation Sources

@cpojer cpojer added the 🕐 Waiting on PR These may be waiting for a PR to the facebook/react-native to be merged label Feb 7, 2019
@JoshuaJamesOng JoshuaJamesOng force-pushed the feature/headless-js-retry branch 3 times, most recently from b1eed7b to 98a25e8 Compare May 9, 2019 23:58
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 6, 2019
Summary:
`setTimeout` inside a headless JS task does not always works; the function does not get invoked until the user starts an `Activity`.

This was attempted to be used in the context of widgets. When the widget update or user interaction causes the process and React context to be created, the headless JS task may run before other app-specific JS initialisation logic has completed. If it's not possible to change the behaviour of the pre-requisites to be synchronous, then the headless JS task blocks such asynchronous JS work that it may depend on. A primitive solution is the use of `setTimeout` in order to wait for the pre-conditions to be met before continuing with the rest of the headless JS task. But as the function passed to `setTimeout` is not always called, the task will not run to completion.

This PR solves this scenario by allowing the task to be retried again with a delay. If the task returns a promise that resolves to a `{'timeout': number}` object, `AppRegistry.js` will not notify that the task has finished as per master, instead it will tell `HeadlessJsContext` to `startTask` again (cleaning up any posted `Runnable`s beforehand) via a `Handler` within the `HeadlessJsContext`.

Documentation also updated here: facebook/react-native-website#771

### AppRegistry.js
If the task provider does not return any data, or if the data it returns does not contain `timeout` as a number, then it behaves as `master`; notifies that the task has finished. If the response does contain `{timeout: number}`, then it will attempt to queue a retry. If that fails, then it will behaves as if the task provider returned no response i.e. behaves as `master` again. If the retry was successfully queued, then there is nothing to do as we do not want the `Service` to stop itself.

### HeadlessJsTaskSupportModule.java
Similar to notify start/finished, we simply check if the context is running, and if so, pass the request onto `HeadlessJsTaskContext`. The only difference here is that we return a `Promise`, so that `AppRegistry`, as above, knows whether the enqueuing failed and thus needs to perform the usual task clean-up.

### HeadlessJsTaskContext.java
Before retrying, we need to clean-up any timeout `Runnable`'s posted for the first attempt. Then we need to copy the task config so that if this retry (second attempt) also fails, then on the third attempt (second retry) we do not run into a consumed exception. This is also why in `startTask` we copy the config before putting it in the `Map`, so that the initial attempt does leave the config's in the map as consumed. Then we post a `Runnable` to call `startTask` on the main thread's `Handler`. We use the same `taskId` because the `Service` is keeping track of active task IDs in order to calculate whether it needs to `stopSelf`. This negates the need to inform the `Service` of a new task id and us having to remove the old one.

## Changelog
[Android][added] - Allow headless JS tasks to return a promise that will cause the task to be retried again with the specified delay
Pull Request resolved: #23231

Differential Revision: D15646870

fbshipit-source-id: 4440f4b4392f1fa5c69aab7908b51b7007ba2c40
Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay, we can finally merge this :)

@cpojer cpojer merged commit fad29f0 into facebook:master Jun 12, 2019
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
Summary:
`setTimeout` inside a headless JS task does not always works; the function does not get invoked until the user starts an `Activity`.

This was attempted to be used in the context of widgets. When the widget update or user interaction causes the process and React context to be created, the headless JS task may run before other app-specific JS initialisation logic has completed. If it's not possible to change the behaviour of the pre-requisites to be synchronous, then the headless JS task blocks such asynchronous JS work that it may depend on. A primitive solution is the use of `setTimeout` in order to wait for the pre-conditions to be met before continuing with the rest of the headless JS task. But as the function passed to `setTimeout` is not always called, the task will not run to completion.

This PR solves this scenario by allowing the task to be retried again with a delay. If the task returns a promise that resolves to a `{'timeout': number}` object, `AppRegistry.js` will not notify that the task has finished as per master, instead it will tell `HeadlessJsContext` to `startTask` again (cleaning up any posted `Runnable`s beforehand) via a `Handler` within the `HeadlessJsContext`.

Documentation also updated here: facebook/react-native-website#771

### AppRegistry.js
If the task provider does not return any data, or if the data it returns does not contain `timeout` as a number, then it behaves as `master`; notifies that the task has finished. If the response does contain `{timeout: number}`, then it will attempt to queue a retry. If that fails, then it will behaves as if the task provider returned no response i.e. behaves as `master` again. If the retry was successfully queued, then there is nothing to do as we do not want the `Service` to stop itself.

### HeadlessJsTaskSupportModule.java
Similar to notify start/finished, we simply check if the context is running, and if so, pass the request onto `HeadlessJsTaskContext`. The only difference here is that we return a `Promise`, so that `AppRegistry`, as above, knows whether the enqueuing failed and thus needs to perform the usual task clean-up.

### HeadlessJsTaskContext.java
Before retrying, we need to clean-up any timeout `Runnable`'s posted for the first attempt. Then we need to copy the task config so that if this retry (second attempt) also fails, then on the third attempt (second retry) we do not run into a consumed exception. This is also why in `startTask` we copy the config before putting it in the `Map`, so that the initial attempt does leave the config's in the map as consumed. Then we post a `Runnable` to call `startTask` on the main thread's `Handler`. We use the same `taskId` because the `Service` is keeping track of active task IDs in order to calculate whether it needs to `stopSelf`. This negates the need to inform the `Service` of a new task id and us having to remove the old one.

## Changelog
[Android][added] - Allow headless JS tasks to return a promise that will cause the task to be retried again with the specified delay
Pull Request resolved: facebook#23231

Differential Revision: D15646870

fbshipit-source-id: 4440f4b4392f1fa5c69aab7908b51b7007ba2c40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕐 Waiting on PR These may be waiting for a PR to the facebook/react-native to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants