react-native icon indicating copy to clipboard operation
react-native copied to clipboard

[v0.74] Android: Promises from native modules stuck

Open birdofpreyru opened this issue 1 year ago • 4 comments

Description

Hey, I am upgrading an existing app from RN v0.73.6 to v0.74.1, trying to make it work for Android, new architecture, bridge-less (and the app was working fine on RN v0.73 with new arch, and bridge; I haven't tested bridge-less before).

I've encountered an issue that shortly after running the app promises from native modules stuck to resolve or reject. I.e. in my code I have some function, say doSmth(): Promise<void> (which is a TS-side definition for a native function in a native module) that is called repeatedly, does some work on the native side, then does promise.resolve(null) on the native side, to resolve the pending promise on the TS side. It works for me fine at first, but after some call the promise from doSmth() just stuck indefinitely without either resolve or reject.

I've added console logging both at native and TS side, so I see that promise.resolve(null) is called at native side for that pending promise (i.e. I see console log from the line just before it); the call to promise.resolve(null) does not fail (I have the native block of code wrapped into try/catch, and a log statement in catch does not print); but on the TS side that promise stays pending (again, if I haven't miss anything, on TS side I have console statements just before the promise, after awaiting it, and I have entire block wrapped into try/catch with another console statement; and it nor statement after awaiting, nor in the catch prints).

So... I am still trying to further dig into it, but already creating the ticket, just in case somebody has seen something similar, or has some helpful ideas / insights.

P.S.: In my current understanding, smth happens in C++ code implementation of promise resolution; and as of now I have no idea how to debug that in an RN app :grimacing: Ideally, I guess, I want to stick debugger breakpoint here in C++ code, to see what happens, but how? If in Android Studio I set breakpoints into C++ code of RN they do not seem to have effect :(

And I have confirmed that my app works seemingly fine with v0.74.1 if I disable bridge-less mode.

Steps to reproduce

N/A

React Native Version

N/A

Affected Platforms

Runtime - Android

Areas

Other (please specify)

Output of npx react-native info

N/A

Stacktrace or Logs

N/A

Reproducer

N/A

Screenshots and Videos

No response

birdofpreyru avatar May 13 '24 13:05 birdofpreyru

: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

github-actions[bot] avatar May 13 '24 13:05 github-actions[bot]

:warning: Missing Reproducible Example
:information_source: We could not detect a reproducible example in your issue report. Please provide either:
  • If your bug is UI related: a Snack
  • If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

github-actions[bot] avatar May 13 '24 13:05 github-actions[bot]

Hey @cortinico , any thoughts about this one? I am not sure what can I do — I can't get Android Studio debugger to work on C++ RN code; I don't see any apparent issues with my code on TS & Java side; and although the issue reliably happens shortly after the app start-up, it is not in an deterministic way, sometimes I get it sooner, sometimes later.

It feels like perhaps some dependency in the app, not correctly migrated to the new arch, does something wrong with communications to the native layer, and somehow rapidly exhaust the ability of bridgleless communication to handle new callbacks / promises... but I don't see anything in adb logcat logs, or app behavior hinting on where to look for the problem (though my first guess would be react-native-webview, or @react-native-firebase to blame — both poorly maintained, and using some weird patterns in their code, which I haven't seen in RN docs).

birdofpreyru avatar May 13 '24 17:05 birdofpreyru

Hey @cortinico , any thoughts about this one?

I'd say, let's start by attaching a reproducer to this issue. This week we're all busy with ReactConf so we won't be able to take a look at it

cortinico avatar May 14 '24 00:05 cortinico

This week we're all busy with ReactConf

I hope, ReactConf was fun :)

Regarding my issue — following the trial-and-error approach, I tried to comment out different parts of my app relying on different 3rd party libraries, and narrowed it down to react-native-google-mobile-ads — if I don't use any functionality from it, my app works fine. Though, while I was testing that library got some fixes... thus, I am still to try whether the problem is there with v13.3.1 of that library, and if so, what might be causing it.

birdofpreyru avatar May 19 '24 22:05 birdofpreyru

@cortinico I have narrowed it down to the issue still happening with react-native-google-mobile-ads v13.3.0, and the issue not present after it upgrade to v13.3.1 and above. The only code change between these versions seems to be a change in event name: https://github.com/invertase/react-native-google-mobile-ads/compare/v13.3.0...v13.3.1 image

I am not sure why that event name was changed, and how may it affect unrelated promises not being resolved in the bridgeless mode. Though, if it makes any sense, I'd say RN misses to detect some problem here, and warn about it, or probably throw a hard error.

birdofpreyru avatar May 21 '24 20:05 birdofpreyru

@birdofpreyru, it's very tough for us to diagnose this issue without code examples, or a reproducer.

I strongly urge you to to create a minimal reproducer! I think that'll be the quickest way to see this problem resolved.

RSNara avatar May 30 '24 18:05 RSNara

Hi, I don't have a minimal reproducer but I do want to chime in that I believe I also have this issue on ios. When bridgeless is on my promises that cross the bridge do not resolve, but with bridgeless off they do resolve

chrisnojima avatar May 30 '24 19:05 chrisnojima

@chrisnojima @birdofpreyru Thanks for reporting this. We are very committed in fixing this, but as @RSNara mentioned, it is really hard for us to figure out what's going on without a reproducer. If it helps, you can start from this template and you can create a simple app that send over a promise? Is it ok if you need some libraries to do so.

cipolleschi avatar Jun 03 '24 09:06 cipolleschi

Understood, unfortunately I don't have free time to invest into it now, especially once it is not a problem for my projects anymore. Just saying in case somebody else facing the issue has doubts whether to look into creating a repro, or wait till the issue reporter does so :)

birdofpreyru avatar Jun 03 '24 12:06 birdofpreyru

One more piece of random data, when I switched my ios build to bridgless + hermes it worked fine. Its only the combination of JSC + bridgeless that has the issue. JSC/Hermes old bridge / old arch is also fine.

chrisnojima avatar Jun 03 '24 16:06 chrisnojima

@chrisnojima I have experienced it with Android + Bridgeless + Hermes. As I said above, for me it was caused by [email protected], so perhaps a good first take on repro would be to try upgrading & running the example app of that lib version with RN v0.74 / Android / Bridgeless / Hermes, and see how that behaves; and as I said, it might take me a while to actually try it.

birdofpreyru avatar Jun 03 '24 16:06 birdofpreyru

@chrisnojima I have experienced it with Android + Bridgeless + Hermes. As I said above, for me it was caused by [email protected], so perhaps a good first take on repro would be to try upgrading & running the example app of that lib version with RN v0.74 / Android / Bridgeless / Hermes, and see how that behaves; and as I said, it might take me a while to actually try it.

@birdofpreyru sorry for the delay in investigation! i got a bout of covid and the rest of the crew is busy with prepping the release. i'm working on trying to find a repro, but if anyone figures out a way to create a repo that can reproduce with specific steps, that would be awesome!

in the meantime, can you answer a few questions for me?

  • can you confirm that this is not a currently blocking issue for you, since upgrading to v13.3.1 fixed the issue?
  • is your native module a turbomodule? or is it a legacy module? if it's a turbomodule, i think we have some logic that doesn't play nice with interop layer and turbomodules. you can try reverting this.
  • how does your application use react-native-google-mobile-ads? does it just initialize it, like in the docs? or do you do some more advanced stuff?

thanks for reporting this!!!! hope we can understand what's going on soon.

philIip avatar Jun 21 '24 00:06 philIip

Hey @philIip ,

can you confirm that this is not a currently blocking issue for you, since upgrading to v13.3.1 fixed the issue?

Yeap, not a blocker for me, v13.3.1 fixed it for me.

is your native module a turbomodule? ...

Yes, it is, thus what you wrote reads plausible.

how does your application use react-native-google-mobile-ads?

Well... I decided to ruin my Saturday with a bit of unpaid work on my personal projects, and stuff related to them, thus here you go, a repro for this: https://github.com/birdofpreyru/issue-44555

Just run it for Android with dev server, and look at logs. By design it is supposed to keep printing the following, once each second:

(NOBRIDGE) LOG  2024-06-22T12:17:45.531Z Ping
(NOBRIDGE) LOG  Pong

And it works that way without the buggy react-native-google-mobile-ads version, but with it it ends with this message (that seems to be the error patched in the next version of RN Google Mobile Ads):

(NOBRIDGE) ERROR  Error: Unsupported top level event type "topNative" dispatched

Note,

  1. This error message appears with RN v0.74.2 being used in the repro; I have reported the issue using RN v0.74.1 originally, and that message did not appear with that version, despite the error breaking the bridge (I mean breaking the "no-bridge" :sweat_smile: ) the same way; thus, my original experience was that promises just stopped to resolve at random without any sign of anything going wrong.

  2. To add that Ping / Pong messaging, which waits each time for a promise from a call to native to resolve, I've installed a library of mine @dr.pogodin/react-native-audio (turbo module) and added ping() function to it using patch-package. Done this way just to rapidly create the repro. I believe, the actual library, nor the way I patched it, do not matter here — the point is that an error in one library broke promise resolution in unrelated library. Originally, it happened without any error message. And without bridge-less mode it does not cause such disastrous effect for other modules.

birdofpreyru avatar Jun 22 '24 12:06 birdofpreyru

@birdofpreyru

Well... I decided to ruin my Saturday with a bit of unpaid work on my personal projects, and stuff related to them, thus here you go, a repro for this: https://github.com/birdofpreyru/issue-44555

thank you for taking the time to do this on a weekend! please don't give up your weekends for us though. but i'm glad to hear that you are at least unblocked!

Yes, it is, thus what you wrote reads plausible.

ok! so it's a little bit confusing, but essentially only legacy modules will have access to the interop bridge layer in bridgeless mode - turbomodules will not. so if you have a turbomodule that accesses the bridge and also want to use bridgeless mode. i don't think that's what's happening here, so i don't think you need to revert your turbomodule to a native module.

Just run it for Android with dev server, and look at logs. By design it is supposed to keep printing the following, once each second:

(NOBRIDGE) LOG  2024-06-22T12:17:45.531Z Ping
(NOBRIDGE) LOG  Pong

And it works that way without the buggy react-native-google-mobile-ads version, but with it it ends with this message (that seems to be the error patched in the next version of RN Google Mobile Ads):

(NOBRIDGE) ERROR  Error: Unsupported top level event type "topNative" dispatched

Note,

  1. This error message appears with RN v0.74.2 being used in the repro; I have reported the issue using RN v0.74.1 originally, and that message did not appear with that version, despite the error breaking the bridge (I mean breaking the "no-bridge" 😅 ) the same way; thus, my original experience was that promises just stopped to resolve at random without any sign of anything going wrong.
  2. To add that Ping / Pong messaging, which waits each time for a promise from a call to native to resolve, I've installed a library of mine @dr.pogodin/react-native-audio (turbo module) and added ping() function to it using patch-package. Done this way just to rapidly create the repro. I believe, the actual library, nor the way I patched it, do not matter here — the point is that an error in one library broke promise resolution in unrelated library. Originally, it happened without any error message. And without bridge-less mode it does not cause such disastrous effect for other modules.

thanks so much for looking into this! i commented on the original PR for what issue that was fixing and what that was, but in terms of the through line to how it broke promises, my hunch is that firing an error in the promise resolution step is swallowing the redbox from being properly presented which leaves everything in a undefined state, the other theory is that the redbox is fired really early. i will see if i can isolate this example further!

philIip avatar Jun 23 '24 06:06 philIip

Hey @birdofpreyru Following up on this one from @philIip

It seems you're currently unblocked, so I'm wondering if we can close this one?

A lot of the early error/redbox logic has been adjusted in 0.75 so I invite you to give it a try with the latest RC of 0.75 (or the stable which will be out really soon) and report back if this is still a problem or not.

Thanks for your time in reporting this issue and creating a reproducer thought 🙏

cortinico avatar Aug 13 '24 11:08 cortinico

Sounds good to me.

birdofpreyru avatar Aug 13 '24 11:08 birdofpreyru