fix: only treat a reachability confirmation as a recovery when the app was offline

[mountiny]

Problem

Production traces show serial Ping+ReconnectApp storms (up to 23 ReconnectApp calls in 52 seconds in one session) where nothing about the network changed. NetworkState fires reconnectApp() from raw NetInfo null→true transitions manufactured by boot's undefined→null→true sequence slipping past guards, NetInfo.refresh() re-emitting cached state, and the SHOULD_USE_STAGING_SERVER Onyx callback rebuilding the NetInfo subscription on every write.

Solution

The recovery branch now fires only when getIsOffline() is true at the moment reachability is confirmed, or when a one-shot pendingReachabilityRecovery token was armed by debug paths (setForceOffline, setFailAllRequests, simulatePoorConnection). The staging-server callback rebuilds the NetInfo subscription only when the effective reachability URL changed, avoiding unnecessary teardown and extra Pings. Regression tests pin that bare null→true transitions never fire reconnect while real offline recovery still works.

PR

#96504

Issue Owner

Current Issue Owner: @mkhutornyi

[MelvinBot]

Issue Analysis

Root Cause: NetworkState treats any NetInfo null→true / undefined→null→true reachability transition as a network recovery and fires notifyReconnectListeners() (→ ReconnectApp), even when the app was never offline. Three independent code paths manufacture that transition without any real connectivity change, so a healthy session emits repeated Ping+ReconnectApp storms. All three are verifiable on main:

1. Boot loophole — the undefined guard is dead. The recovery gate at NetworkState.ts:337 only blocks undefined→true. But boot actually emits undefined→null→true: the intermediate null event sets prevIsInternetReachable = null (:350), so when true arrives prev is null, not undefined, the guard passes, and onReachabilityRestored() fires a spurious reconnect on every cold start.

2. Debug paths re-arm the loophole via refresh(). setForceOffline(false) (:161-168), setFailAllRequests(false) (:184-191), and simulatePoorConnection(false) (:250-252) all deliberately set prevIsInternetReachable = null and call NetInfo.refresh() to synthesize a null→true. refresh() re-emits cached state, so this fabricates a "recovery" whether or not the app was ever offline.

3. Staging-server callback rebuilds the subscription on every write. The SHOULD_USE_STAGING_SERVER Onyx callback at :382-387 calls configureAndSubscribe() unconditionally — with no check that the effective reachability URL changed. Each rebuild tears down and re-subscribes NetInfo (:286-294), firing a fresh Ping and a synthetic null→true. The suppressNextReachabilityRestored band-aid only suppresses the reconnect when prev !== false and does nothing to stop the extra Ping.

The common flaw: recovery is inferred from the shape of the transition (x→true) rather than from whether the app was actually offline.

Recommended Fix: Gate the recovery branch on real offline state instead of the raw transition — fire only when getIsOffline() is true at the moment reachability is confirmed, or when a one-shot token was explicitly armed by a debug path. Make the debug paths arm that token rather than resetting prev = null, and rebuild the NetInfo subscription only when the effective reachability URL actually changed. This is exactly what PR #96504 does (pendingReachabilityRecovery token + URL-diffed reconfigure + deletion of suppressNextReachabilityRestored), and its regression tests pin that bare null→true never fires reconnect while false→true / false→null→true still fire exactly once. I verified the fix's target symbols don't exist on main, so the PR is the source of the change.

Investigation details, confidence & alternatives considered

Confidence: High — each of the three mechanisms is reproducible by reading the code path on main (SHA abbe05c); no assumption is presented as fact. The "23 ReconnectApp calls in 52s" figure is from the reporter's production traces and is consistent with (but was not independently re-derived from) the code.

Alternative explanations ruled out:

• The SESSION callback (:365-375) also calls configureAndSubscribe() — but it early-returns when newAccountID === accountID, so it does not rebuild on same-value writes. Only the staging callback is unguarded, which is why cause Add Expensify eslint #3 is scoped to it.
• setSustainedFailures recovery (:135-150) could stampede — but it's already jittered and only fires on a genuine active→!active edge, so it isn't a source of the fake-recovery storms described here.

Scope match: the fix touches only NetworkState.ts (+ its two test suites and the detection doc) and changes the gating condition, not the recovery behavior — matching a bug that is about when reconnect fires, not what it does. That's the right blast radius.

Related issues

• [Due for payment 2026-07-03] [Due for payment 2026-06-29] [Performance] App hangs when OpenApp / ReconnectApp / GetMissingOnyxMessages fire in rapid stampedes #92541 — [Performance] App hangs when OpenApp / ReconnectApp / GetMissingOnyxMessages fire in rapid stampedes (open). Strongly related: this issue removes a source of the redundant ReconnectApp calls that [Due for payment 2026-07-03] [Due for payment 2026-06-29] [Performance] App hangs when OpenApp / ReconnectApp / GetMissingOnyxMessages fire in rapid stampedes #92541 is about surviving. Worth cross-linking but they are distinct (this is prevention at the source; [Due for payment 2026-07-03] [Due for payment 2026-06-29] [Performance] App hangs when OpenApp / ReconnectApp / GetMissingOnyxMessages fire in rapid stampedes #92541 is stampede resilience).

---

This analysis validates the fix already in review at #96504. No new PR is needed — reply with @MelvinBot <feedback> to refine this analysis.