Send flow: "maxiumum call stack exceeded" error on the input_amount screen

[OmarBasem]

The above error happens randomly when reaching the input_amount screen in debug mode.

Steps to reproduce:

Just going normally through the send flow:

1. Enter address
2. Select token
3. Upon reaching the input_amount screen you may get that error

[ulisesmac]

This is being solved in:

• [#18495] Fix token not ready for next screen #18532

These errors are thrown due to Malli. In this case we were calling the token with nil, since this is not expected by the malli schema, it breaks in this way.

@ilmotta maybe something that needs to be fixed?

[ilmotta]

This is being solved in:

• [#18495] Fix token not ready for next screen #18532

These errors are thrown due to Malli. In this case we were calling the token with nil, since this is not expected by the malli schema, it breaks in this way.

@ilmotta maybe something that needs to be fixed?

Oh my, actually good news you can reproduce. I hit this bug a few times recently and it's very frustrating, I ended up just commenting out a schema to get something else done. I thought it was correlated with the recent RN upgrades as you and me recently talked in private. Maybe the RN upgrade exacerbated a problem that could already happen. At least that's what I believe at this point.

@ilmotta maybe something that needs to be fixed?

I think so, these max call stack errors should never happen, even if the arguments passed to malli are considered invalid (such as nil). It's worth investigating, maybe someone already reported on the lib's repo, or the lib has upgrades already that could alleviate the problem (malli gets commits very frequently).

[ulisesmac]

At least that's what I believe at this point.

Yeah, I think the same.
To me seems like an infinite loop or something like that 🤔

[ilmotta]

Hey, coming back to this issue because this error is becoming a major blocker during my daily work. It's not caused by Malli or an infinite loop, although when Malli schemas fail the exception is thrown, but it's not the root cause.

I could pinpoint the culprit commit, it's 761a7df (upgrade react-native to 0.72.5 (#17241)`.

The following error is triggered when we evaluate certain buffers in the REPL (e.g. utils.money), as well as when some Malli schemas fail. For example:

1. Evaluate the entire utils.money buffer in a REPL. Many other namespaces are affected too.
2. Or, call a malli schema with invalid args. One of the simplest, utils.money/format.

Maximum call stack size exceeded (native stack depth), js engine: hermes
 ERROR  REPL Invoke Exception [RangeError: Maximum call stack size exceeded (native stack depth)]

---

It does look like Hermes has a native call stack size that's too small for our use case. Rebuilding Hermes with an increased MAX_NATIVE_CALL_FRAME_DEPTH could be an option, or our only option.

From Herme's source code:

https://github.com/facebook/hermes/blob/c65d640e1710cf9717b2898c9a0f110e1385dc07/include/hermes/VM/Runtime.h#L941

This is the last commit in develop where Hermes was behaving well f155d95

@siddarthkay, would you have capacity to help investigate this problem soon(ish)? I'm really just asking because if this is not the best time for you, next week max I'll have to fully dig deeper in hopes of finding a solution. It's so bad that I disabled Malli entirely during development because it's what's blocking me the most.

@yqrashawn, @clauxx, can you also reproduce the problem?

[siddarthkay]

It does look like Hermes has a native call stack size that's too small for our use case. Rebuilding Hermes with an increased MAX_NATIVE_CALL_FRAME_DEPTH could be an option, or our only option.

From Herme's source code:

https://github.com/facebook/hermes/blob/c65d640e1710cf9717b2898c9a0f110e1385dc07/include/hermes/VM/Runtime.h#L941

This is the last commit in develop where Hermes was behaving well f155d95

@siddarthkay, would you have capacity to help investigate this problem soon(ish)? I'm really just asking because if this is not the best time for you, next week max I'll have to fully dig deeper in hopes of finding a solution. It's so bad that I disabled Malli entirely during development because it's what's blocking me the most.

wow thanks for bringing this to my attention.
before the 0.72.5 upgrade we actually would keep Hermes off for debug builds ( or development builds )
ref ->

status-mobile/android/app/build.gradle

Line 86 in f155d95

def disableHermes = System.getenv('DISABLE_HERMES') == 'true'

So it does seem like the problem you are facing is because Hermes is enabled.
I'll also check out the MAX_NATIVE_CALL_FRAME_DEPTH flag and how we can use it.

another interesting thing would be to wait for 0.73 upgrade to land ( which is just 1 short bug away and has a newer version of Hermes ) -> #18563

[siddarthkay]

according to facebook/hermes#135 (comment) hermes for react-native 0.73 would be more stable but lets see :D

[siddarthkay]

@ilmotta : I did have a look on how to bump MAX_NATIVE_CALL_FRAME_DEPTH and for that we would have to create our own fork of hermes lib and patch react-native to use our forked version.
Its a tedious experiment but it is possible to do.

there are 3 options at this point :

1. experiment with disabling hermes only for develop/debug builds, locally this can be achieved by setting https://github.com/status-im/status-mobile/blob/develop/android/gradle.properties#L60 to false

2. try out new hermes release for react-native 0.73 in this branch -> upgrade-react-native-to-0_73

3. we fork hermes engine and increase MAX_NATIVE_CALL_FRAME_DEPTH according to our needs

[siddarthkay]

update on option 3, you can use latest hermes + my patch to increase MAX_NATIVE_CALL_FRAME_DEPTH times 5 here -> https://github.com/status-im/status-mobile/tree/use-hermes-fork I've tested it locally and I got the android app to work but didn't test any further.

[ilmotta]

Thank you so much @siddarthkay. I will test your experiment with Hermes forked. I saw other folks reported in GH issues similar problems with the native call stack size, so we are not alone.

It would be best if option 0.73 fixed or even improved this situation. But from what I read in issues, the stack size is conservative on purpose, and I believe that won't change. I'll see if disabling Hermes helps.

[ilmotta]

Hey @siddarthkay. I tried the branch where you increased the stack size, but unfortunately I get the same error whenever a malli schema fails. This problem also happens when evaluating many namespaces in the REPL, so our current Hermes version is not playing nice with CLJS overall.

Is there a way to run the app with Hermes disabled, just as before, where we could set the env var DISABLE_HERMES?

[siddarthkay]

Hmmm,
Thanks for checking and reporting back.
For now can you check by modifying https://github.com/status-im/status-mobile/blob/develop/android/gradle.properties#L60 to false locally? This will disable hermes for your local env.

If it works well then I'll introduce a way to keep hermes disabled locally but enabled in PR builds and release builds.

[ilmotta]

If it works well then I'll introduce a way to keep hermes disabled locally but enabled in PR builds and release builds.

Forgot to say, yes it does work without Hermes. Without it, all namespaces can be evaluated as usual and malli never causes the error.

[siddarthkay]

Okay thanks @ilmotta ,I will introduce DISABLE_HERMES back again.