Skip to content

Conversation

@bheemreddy-samsara
Copy link
Contributor

@bheemreddy-samsara bheemreddy-samsara commented Dec 23, 2025

Problem

On some apps, harness runs can intermittently fail at startup with a redbox:

Expected HMRClient.setup() call at startup

Root cause: @react-native-harness/runtime schedules HMRClient.disable() on a timer, but React Native hasn't always invoked HMRClient.setup() yet, so RN throws and the harness never reports ready.

Fix

  • Add disableHMRWhenReady helper that retries disable() when it fails with the specific RN invariant, then succeeds once setup is complete.
  • Keep behavior identical once RN has initialized HMRClient.

Tests

  • Add a unit test that simulates the invariant on the first call and verifies we retry and eventually disable successfully.

Validation

  • pnpm nx test @react-native-harness/runtime
  • pnpm nx lint @react-native-harness/runtime
  • pnpm nx typecheck @react-native-harness/runtime
  • pnpm nx build @react-native-harness/runtime

React Native may throw "Expected HMRClient.setup()" if disable() runs before setup().

Retry disable() briefly on that specific invariant so harness startup stays stable.

Add a unit test for the retry helper.
@vercel
Copy link

vercel bot commented Dec 23, 2025

@bheemreddy-samsara is attempting to deploy a commit to the Callstack Team on Vercel.

A member of the Team first needs to authorize it.

@bheemreddy-samsara
Copy link
Contributor Author

@V3RON Can you review this ?

@V3RON
Copy link
Contributor

V3RON commented Dec 23, 2025

I wonder if it wouldn't be more bulletproof to monkey-patch HMRClient and override the .setup method so it does nothing, then call .disable() afterward (just to be sure), all wrapped in a try/catch to swallow any potential errors. That would remove the need for retries and make the whole flaky setTimeout obsolete. WDYT?

@bheemreddy-samsara
Copy link
Contributor Author

I wonder if it wouldn't be more bulletproof to monkey-patch HMRClient and override the .setup method so it does nothing, then call .disable() afterward (just to be sure), all wrapped in a try/catch to swallow any potential errors. That would remove the need for retries and make the whole flaky setTimeout obsolete. WDYT?

I considered monkey-patching HMRClient.setup, but that changes RN’s HMR init semantics (setup is what instantiates the underlying MetroHMRClient) and could have unintended side effects for devtools/log forwarding.The issue here is purely ordering: harness calls HMRClient.disable() before RN has invoked setup().The PR keeps RN behavior intact and just retries disable() when we hit the known invariant, then succeeds once setup completes. This also avoids blocking harness startup because we schedule the retry and continue initialization.

@V3RON
Copy link
Contributor

V3RON commented Dec 23, 2025

This also avoids blocking harness startup because we schedule the retry and continue initialization.

In fact, we should block the Harness setup. HMRClient should be disabled before any tests run, because if you start a Harness run and modify test files at the same time, Metro will push updates to the runtime, causing issues.

Ensure we only report harness ready after HMR has been disabled.

This prevents Metro from pushing updates mid-run if tests change during execution.
@bheemreddy-samsara
Copy link
Contributor Author

Good call — updated.

disableHMRWhenReady now returns a Promise and initialize.ts only calls reportReady after HMR has been successfully disabled.

So harness won't start executing tests / report ready until HMR is off, preventing Metro updates mid-run.

(Also added/updated a unit test for the Promise-based retry behavior.)

@bheemreddy-samsara
Copy link
Contributor Author

is this approach good to move forward ? @V3RON

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants