facebook / react-native

A framework for building native applications using React
https://reactnative.dev
MIT License
118.36k stars 24.25k forks source link

Emiting Events from HeadlessTask on Android in New Arch + Bridgeless + interop layer #44255

Closed lovegaoshi closed 2 months ago

lovegaoshi commented 4 months ago

Description

hi! I was directed from https://github.com/reactwg/react-native-new-architecture/discussions/8#discussioncomment-9223240 to open an issue. I'm looking at RNTP's interop layer compatibility, and there is an issue where the module's hooks using event listeners (with a NOBRIDGE log tag?) are not firing, but listeners registered via AppRegistry.registerHeadlessTask (without the NOBRIDGE log tag?) are. Would anyone share insight on emitting events? Thank you very much in advance

RNTP hooks registering event listeners: here and here registerHeadlessTask: logs

Steps to reproduce

  1. yarn, yarn android
  2. click play
  3. RNTP's hook events are not fired; headlessJs are, without NOBRIDGE

React Native Version

0.74

Affected Platforms

Runtime - Android

Areas

Bridgeless - The New Initialization Flow

Output of npx react-native info

info Fetching system and libraries information...
(node:29688) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
System:
  OS: Windows 11 10.0.22621
  CPU: "(12) x64 AMD Ryzen 5 5600 6-Core Processor              "
  Memory: 12.81 GB / 47.91 GB
Binaries:
  Node:
    version: 21.7.2
    path: C:\Program Files\nodejs\node.EXE
  Yarn:
    version: 1.22.22
    path: C:\Program Files\nodejs\yarn.CMD
  npm:
    version: 10.5.0
    path: C:\Program Files\nodejs\npm.CMD
  Watchman: Not Found
SDKs:
  Android SDK: Not Found
  Windows SDK:
    AllowDevelopmentWithoutDevLicense: Enabled
    AllowAllTrustedApps: Enabled
    Versions:
      - 10.0.17763.0
      - 10.0.19041.0
      - 10.0.22000.0
      - 10.0.22621.0
IDEs:
  Android Studio: AI-241.14494.158.2411.11678081
  Visual Studio:
    - 17.5.33530.505 (Visual Studio Community 2022)
Languages:
  Java: Not Found
  Ruby: Not Found
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.0
    wanted: 0.74.0
  react-native-windows: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: Not found
  newArchEnabled: Not found

Stacktrace or Logs

by adding a console.log within https://github.com/doublesymmetry/react-native-track-player/blob/6c4a3bd7d47c6ba8c85db4c1d031dc904bb0cd1a/src/trackPlayer.ts#L77 (int eventname count) I obtain these various event count logs with and without NOBRIDGE. NOBRIDGE events never fire.

(NOBRIDGE) LOG  Bridgeless mode is enabled
(NOBRIDGE) LOG  Running "RNTPExampleNewArch" with {"rootTag":241,"initialProps":{},"fabric":true}
(NOBRIDGE) LOG  deb playback-progress-updated
(NOBRIDGE) LOG  0 playback-progress-updated count
LOG  0 remote-pause count
LOG  0 remote-play count
LOG  0 remote-next count
(NOBRIDGE) LOG  getInitialURL null
LOG  0 remote-previous count
LOG  0 remote-jump-forward count
LOG  0 remote-jump-backward count
LOG  0 remote-seek count
LOG  0 remote-duck count
LOG  0 playback-queue-ended count
LOG  0 playback-active-track-changed count
LOG  0 playback-progress-updated count
LOG  0 playback-play-when-ready-changed count
LOG  0 playback-state count
LOG  0 playback-metadata-received count
LOG  0 metadata-chapter-received count
LOG  0 metadata-timed-received count
LOG  0 metadata-common-received count
LOG  1 playback-metadata-received count
LOG  Event.PlaybackProgressUpdated {"buffered": 1626.357, "duration": 0, "position": 1575.25, "track": 5}
(NOBRIDGE) LOG  deb playback-active-track-changed
(NOBRIDGE) LOG  0 playback-active-track-changed count
(NOBRIDGE) LOG  0 playback-state count
(NOBRIDGE) LOG  0 playback-play-when-ready-changed count
(NOBRIDGE) LOG  1 playback-state count
LOG  Event.PlaybackProgressUpdated {"buffered": 1657.469, "duration": 0, "position": 1577.249, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1659.454, "duration": 0, "position": 1579.249, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1661.466, "duration": 0, "position": 1581.251, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1663.451, "duration": 0, "position": 1583.252, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1665.462, "duration": 0, "position": 1585.251, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1667.448, "duration": 0, "position": 1587.252, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1669.459, "duration": 0, "position": 1589.251, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1671.444, "duration": 0, "position": 1591.247, "track": 5}
LOG  Event.PlaybackProgressUpdated {"buffered": 1673.456, "duration": 0, "position": 1593.251, "track": 5}

Reproducer

https://github.com/lovegaoshi/RNTPExampleNewArch

Screenshots and Videos

No response

github-actions[bot] commented 4 months ago
:warning: Add or Reformat Version Info
:information_source: We could not find or parse the version number of React Native in your issue report. Please use the template, and report your version including major, minor, and patch numbers - e.g. 0.70.2
cortinico commented 4 months ago

Relevant PR also https://github.com/doublesymmetry/react-native-track-player/pull/2290

lovegaoshi commented 4 months ago

I can confirm by opting out of bridgeless withload(bridgelessEnabled = false), everything works as intended.

RNTP also has a notification no longer working issue as mentioned in the PR mentioned above; this is very puzzling to me because the notification manager component should be purely native. This issue is also "fixed" by opting out of the bridgeless mode.

cortinico commented 4 months ago

I can confirm by opting out of bridgeless withload(bridgelessEnabled = false), everything works as intended.

Thanks for the confirmation. I believe this has to do with Headless mode not going through the bridgeless initialization mode. We'll look into it and get back to you.

n10l commented 4 months ago

Thank you @cortinico for looking into this. I am having same issue as well.

contactsimonwilson commented 3 months ago

@cortinico any update or ETA on this?

cortinico commented 3 months ago

Sorry I haven't had the time to look into this as we had several tasks with higher priority to look into first. We'll get back to it once we have more free time. In the meantime, any investigation from the community is more than welcome

contactsimonwilson commented 3 months ago

Thanks for the speedy response. I'm trying to dig into it but admittedly don't have much experience with the react native source. I've got some logs in AppRegistry and am seeing that startHeadlessTask isn't getting the NOBRIDGE log tag. image

If you have any suggestions of where to start an investigation, I'm more than happy to look into it more.

cortinico commented 3 months ago

@contactsimonwilson @lovegaoshi I spent some time investigating this.

The outcome is that AppRegistry.registerHeadlessTask is broken on Bridgeless mode for Android. The reason why it's broken is because of Headless Tasks are effectively attempting to create a bridge instance (that's why you see BRIDGE on console). The problem is here: https://github.com/facebook/react-native/blob/33aa83a0e6f63d3d50d4803074ad9e2243439100/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/HeadlessJsTaskService.java#L109

We would have to extend the logic there to don't use ReactInstanceManager if bridgeless is enabled, and instead use ReactHost to start a background task.

I've also realized that the library RNTP is in a un-maintained state (maintainers are not responding to PR such as https://github.com/doublesymmetry/react-native-track-player/pull/2290 So I'm a bit afraid that even if we end up fixing this, the library won't be releasing a new version in the future.

We should definitely fix Headless tasks in Bridgeless mode, but we're also potentially considering to deprecate this API as it's not widely used and it's probably affected by numerous bugs (for reference, we don't use it internally at Meta at all).

I'm curious to hear more use cases for .registerHeadlessTask to understand how important is this fix to do.

lovegaoshi commented 3 months ago

hi! really appreciate the work investigating this!

rntp uses headlesstaskservice to provide background playback. while i do not know the details, rntp made the migration to use headless instead of extending to MediaLibraryService in 2019. also other background libraries like react-native-background-timer and react-native-background-actions depend on headlessJsTaskService. would there be an alternative if this is removed?

as far I'm aware, rntp is the definitive react native music library with a notification service. while others are recently providing similar notification functionalities (expo av, rnvideo), rntp has been doing it since 2020? and it is way too much migration effort to transition into other libraries for many, including me. plus, rntp is the only music library that i know that integrates android auto and carplay. i do agree the maintenance has been lacking recently but I'm willing to maintain the android part if necessary. appreciate the work as always!

On Wed, Jun 12, 2024, 7:37 AM Nicola Corti @.***> wrote:

@contactsimonwilson https://github.com/contactsimonwilson @lovegaoshi https://github.com/lovegaoshi I spent some time investigating this.

The outcome is that AppRegistry.registerHeadlessTask is broken on Bridgeless mode for Android. The reason why it's broken is because of Headless Tasks are effectively attempting to create a bridge instance (that's why you see BRIDGE on console). The problem is here:

https://github.com/facebook/react-native/blob/33aa83a0e6f63d3d50d4803074ad9e2243439100/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/HeadlessJsTaskService.java#L109

We would have to extend the logic there to don't use ReactInstanceManager if bridgeless is enabled, and instead use ReactHost to start a background task.

I've also realized that the library RNTP is in a un-maintained state (maintainers are not responding to PR such as doublesymmetry/react-native-track-player#2290 https://github.com/doublesymmetry/react-native-track-player/pull/2290 So I'm a bit afraid that even if we end up fixing this, the library won't be releasing a new version in the future.

We should definitely fix Headless tasks in Bridgeless mode, but we're also potentially considering to deprecate this API as it's not widely used and it's probably affected by numerous bugs (for reference, we don't use it internally at Meta at all).

I'm curious to hear more use cases for .registerHeadlessTask to understand how important is this fix to do.

— Reply to this email directly, view it on GitHub https://github.com/facebook/react-native/issues/44255#issuecomment-2163183638, or unsubscribe https://github.com/notifications/unsubscribe-auth/AZMOVVS5TUM2PMN2AV2TLNTZHBMK5AVCNFSM6AAAAABGY3XVSWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRTGE4DGNRTHA . You are receiving this because you were mentioned.Message ID: @.***>

contactsimonwilson commented 3 months ago

Right now I'm using react-native-background-actions in several projects but the most impacted one is a monitoring app for PEVs (VESC-based onewheels specifically).

I use react-native-background actions with react-native-ble-plx, expo-location and expo-av/expo-speech to provide a critical safety monitoring system with data logging and audio/haptic alerts while riding. In its current state, I have a js task that does all that and will stay in the background indefinitely on iOS and Android, even while a device is locked.

I would be interested to hear about alternatives for library maintainers. This is the last thing holding me back from upgrading my projects as I can't find other libraries that will work with the new arch and have the same functionality.

robik commented 3 months ago

I've put up a draft PR that should fix the issue. Because it is hard to create foreground tasks in new android and test this I will try to build the react-native-track-player test app in bridgeless with patch applied soon (probably during the weekend). If you have capacity @contactsimonwilson to check it on your own with the RN PR let me know :)

lovegaoshi commented 3 months ago

thank you @robik ! I think I applied the patch right but I'm still seeing headlessJsTask going through the bridge, maybe I'm just doing it wrong. NOBRIDGE tag is not applied as in below.

 BUNDLE  ./index.js 

 (NOBRIDGE) LOG  Bridgeless mode is enabled
 (NOBRIDGE) LOG  Running "RNTPExampleNewArch" with {"rootTag":11,"initialProps":{},"fabric":true}
 (NOBRIDGE) LOG  getInitialURL null
 BUNDLE  ./index.js 

 LOG  Event.MetadataCommonReceived {"metadata": {"artist": "David Chavez", "creationDate": "0303", "creationYear": "2012", "title": "Longing"}}
 LOG  Event.PlaybackState {"state": "ready"}
 LOG  Event.RemotePlay
 LOG  Event.RemotePlay
 LOG  Event.RemotePlay
 LOG  Event.RemotePlay
 LOG  Event.RemoteNext
 LOG  Event.PlaybackPlayWhenReadyChanged {"playWhenReady": true}
 LOG  Event.PlaybackState {"state": "playing"}
 LOG  Event.PlaybackProgressUpdated {"buffered": 52.323, "duration": 143.438, "position": 0.378, "track": 0}
 LOG  Event.PlaybackProgressUpdated {"buffered": 52.323, "duration": 143.438, "position": 1.882, "track": 0}
 LOG  Event.PlaybackActiveTrackChanged {"index": 1, "lastIndex": 0, "lastPosition": 1.995, "lastTrack": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Longing.jpeg", "duration": 143, "title": "Longing", "url": "https://rntp.dev/example/Longing.mp3"}, "track": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Soul%20Searching.jpeg", "duration": 77, "title": "Soul Searching (Demo)", "url": "https://rntp.dev/example/Soul%20Searching.mp3"}}
 LOG  Event.MetadataCommonReceived {"metadata": {}}
 LOG  Event.PlaybackState {"state": "loading"}
 LOG  Event.PlaybackState {"state": "buffering"}
 LOG  Event.MetadataCommonReceived {"metadata": {"albumName": "Demos", "artist": "David Chavez", "creationDate": "0202", "creationYear": "2012", "title": "Soul Searching"}}
 LOG  Event.PlaybackState {"state": "ready"}
 LOG  Event.PlaybackActiveTrackChanged {"index": 2, "lastIndex": 1, "lastPosition": 76.773, "lastTrack": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Soul%20Searching.jpeg", "duration": 77, "title": "Soul Searching (Demo)", "url": "https://rntp.dev/example/Soul%20Searching.mp3"}, "track": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Lullaby%20(Demo).jpeg", "duration": 71, "title": "Lullaby (Demo)", "url": "https://rntp.dev/example/Lullaby%20(Demo).mp3"}}
 LOG  Event.MetadataCommonReceived {"metadata": {}}
 LOG  Event.PlaybackState {"state": "playing"}
 LOG  Event.PlaybackState {"state": "loading"}
 LOG  Event.PlaybackState {"state": "ready"}
 LOG  Event.PlaybackState {"state": "playing"}
 LOG  Event.PlaybackProgressUpdated {"buffered": 52.218, "duration": 70.661, "position": 0.731, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 2.727, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 4.726, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 6.729, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 8.724, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 10.663, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 12.666, "track": 2}
 LOG  Event.RemotePause
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 14.676, "track": 2}
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 16.672, "track": 2}
 LOG  Event.RemotePause
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 18.673, "track": 2}
 LOG  Event.RemotePause
 LOG  Event.PlaybackProgressUpdated {"buffered": 70.661, "duration": 70.661, "position": 20.681, "track": 2}
 LOG  Event.PlaybackActiveTrackChanged {"index": 3, "lastIndex": 2, "lastPosition": 22.146, "lastTrack": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Lullaby%20(Demo).jpeg", "duration": 71, "title": "Lullaby (Demo)", "url": "https://rntp.dev/example/Lullaby%20(Demo).mp3"}, "track": {"artist": "David Chavez", "artwork": "https://rntp.dev/example/Rhythm%20City%20(Demo).jpeg", "duration": 106, "title": "Rhythm City (Demo)", "url": "https://rntp.dev/example/Rhythm%20City%20(Demo).mp3"}}
 LOG  Event.PlaybackState {"state": "loading"}
 LOG  Event.MetadataCommonReceived {"metadata": {"albumName": "Eternity", "artist": "Psychosity", "composer": "David Chavez", "creationDate": "2707", "creationYear": "2011", "title": "Rhythm City (Demo)"}}
 LOG  Event.PlaybackState {"state": "buffering"}
 LOG  Event.PlaybackState {"state": "ready"}
 LOG  Event.PlaybackState {"state": "playing"}
 LOG  Event.PlaybackProgressUpdated {"buffered": 52.297, "duration": 105.586, "position": 0.286, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 52.297, "duration": 105.586, "position": 2.287, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 4.288, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 6.284, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 8.292, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 10.287, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 12.297, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 14.296, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 16.293, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 18.293, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 20.37, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 22.368, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 24.37, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 26.368, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 78.55, "duration": 105.586, "position": 28.365, "track": 3}
 LOG  Event.PlaybackProgressUpdated {"buffered": 104.777, "duration": 105.586, "position": 30.368, "track": 3}
cortinico commented 3 months ago

thank you @robik ! I think I applied the patch right but I'm still seeing headlessJsTask going through the bridge, maybe I'm just doing it wrong. NOBRIDGE tag is not applied as in below.

You'll have to enable build from source: https://reactnative.dev/contributing/how-to-build-from-source

As Android is prebuilt and the patches for .java files are ignored

contactsimonwilson commented 3 months ago

I've put up a draft PR that should fix the issue. Because it is hard to create foreground tasks in new android and test this I will try to build the react-native-track-player test app in bridgeless with patch applied soon (probably during the weekend). If you have capacity @contactsimonwilson to check it on your own with the RN PR let me know :)

Thanks, I'll try building from source in the next few days.

contactsimonwilson commented 2 months ago

I've struggled a couple of times to get a local build without success. Any chance you would be able to provide a build of your PR branch?

cortinico commented 2 months ago

I've struggled a couple of times to get a local build without success. Any chance you would be able to provide a build of your PR branch?

Sadly that's not easily possible as artifacts needs to be published to a repository for this to work. @robik are you blocked on external testing here?

As if we merge your changes, @contactsimonwilson and @lovegaoshi will be able to test it with the latest nightly

robik commented 2 months ago

@cortinico Hey, I left it as draft because I did not test it (it is more complicated than I thought), because there are some limitations on the usage of headless tasks in android. So it's best to test with the library directly. Unfortunately, I didn't have any time to test it.

contactsimonwilson commented 2 months ago

Happy to test as soon as it's in a nightly build 👍

cortinico commented 2 months ago

Happy to test as soon as it's in a nightly build 👍

It's on the latest nightly: https://www.npmjs.com/package/react-native/v/0.76.0-nightly-20240710-9a1ae97c2

lovegaoshi commented 2 months ago

~thx! though i cant build with 0.76.0-nightly-20240710-9a1ae97c2, i have:~ should have read the upgrade helper: https://react-native-community.github.io/upgrade-helper/?from=0.74.3&to=0.75.0-rc.4

next I encountered ndk not specified error. I added local.properties in node_modules/react-native to get pass that. then I encountered ANDROID_HOME nor ANDROID_SDK_ROOT not set in hermes build.gradle.kts. I hardcoded my sdk path to get pass it. then I have the following build errors:


CMake Error in CMakeLists.txt:
  Imported target "ReactAndroid::fabricjni" includes non-existent path

    "./RNTPExampleNewArch/node_modules/react-native/ReactAndroid/build/prefab-headers/fabricjni"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.
CMake Error: The source directory "/home/nisadmin/Documents/GitHub/RNTPExampleNewArch/node_modules/react-native/sdks/hermes" does not exist.

any help appreciated