Skip to content

[$250] New Expensify often appears offline on stable network connections in Safari #89252

Closed

Description

@roryabraham

Problem

I mostly use the Safari PWA for New Expensify, and it almost always shows me as offline, even when I have a stable connection and everything else is normal. This happens if I use Safari directly as well. More notes:

  • The problem seems to be that ~80-90% of Ping requests fail (after ~10s timeout)

Given that Safari is the 2nd-most-popular browser (~15% market share for desktop browsers), we should fix this.

Solution

Still trying to nail down the root cause. It's not the root cause, but one thing we should probably do regardless to reduce unnecessary network traffic is to implement shouldReachabilityRun such that we don't send Ping if we already received a success response from the same domain expensify.com in the normal ping period.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~022049937108558464199
  • Upwork Job ID: 2049937108558464199
  • Last Price Increase: 2026-05-07
Issue OwnerCurrent Issue Owner: @Pujan92

Activity

  1. self-assigned this
    on Apr 30, 2026
  2. roryabraham commented on Apr 30, 2026

    @roryabraham
    ContributorAuthor

    Pretty busy, so I'm opening this up as an external issue

  3. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Apr 30, 2026
  4. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Apr 30, 2026
  5. melvin-bot commented on Apr 30, 2026

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @Pujan92 (External)

  6. changed the title [-]New Expensify often appears offline on stable network connections in Safari[/-] [+][$250] New Expensify often appears offline on stable network connections in Safari[/+] on Apr 30, 2026
  7. melvin-bot commented on Apr 30, 2026

    @melvin-bot
  8. KJ21-ENG commented on Apr 30, 2026

    @KJ21-ENG
    Contributor

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    In Safari (both standalone and PWA), New Expensify frequently shows the offline indicator even when the underlying connection is stable. Other browsers do not show this regression as often. This is on top of the recent network‑state‑detection rework in src/libs/NetworkState.ts.

    What is the root cause of that problem?

    Two interacting properties of the current offline detection make Safari the worst case.

    1. A single failed api/Ping flips the app offline immediately — no debouncing.

    NetInfo is configured with useNativeReachability: false, so every platform (including web) uses JS fetch polling against api/Ping:

    if (!CONFIG.IS_USING_LOCAL_WEB) {
    NetInfo.configure({
    reachabilityUrl: `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping?accountID=${accountID ?? 'unknown'}`,
    reachabilityMethod: 'GET',
    reachabilityTest: (response) => {
    if (!response.ok) {
    return Promise.resolve(false);
    }
    return response
    .json()
    .then((json: {jsonCode: number}) => Promise.resolve(json.jsonCode === 200))
    .catch(() => Promise.resolve(false));
    },
    reachabilityRequestTimeout: CONST.NETWORK.MAX_PENDING_TIME_MS,
    // Use JS fetch polling (api/Ping) on all platforms instead of native OS reachability.
    // This aligns behavior across web and mobile: poll every 60s when reachable, 5s when unreachable.
    useNativeReachability: false,
    });
    }

    The listener flips internetUnreachable on the very first isInternetReachable === false it sees:

    unsubscribeNetInfo = NetInfo.addEventListener((state) => {
    const radio = state.isConnected !== false;
    Log.info(`[NetworkState] NetInfo state change: isConnected=${state.isConnected}, isInternetReachable=${state.isInternetReachable}, type=${state.type}`);
    // Always track real radio/reachability state so values are never stale
    // when shouldForceOffline is turned off. Only gate the reconnect side effect.
    setHasRadio(radio);
    // When isInternetReachable transitions to false, api/Ping failed — go offline.
    // Fire on any non-false→false transition (true→false, null→false, undefined→false)
    // to catch cold start with no internet and post-recovery resets where prev is null.
    // Only skip false→false (redundant, already offline).
    if (state.isInternetReachable === false && prevIsInternetReachable !== false) {
    setInternetUnreachable(true);
    }
    // Treat false→true and null→true as genuine recovery. Both mean NetInfo previously
    // lost reachability (false = confirmed unreachable, null = lost track during outage)
    // and has now confirmed it's back. Only block undefined→true — that's the initial
    // NetInfo event on subscribe which delivers current state, not a recovery. Firing
    // onReachabilityRestored() on boot would duplicate openApp()/reconnectApp().
    if (!shouldForceOffline && state.isInternetReachable === true && prevIsInternetReachable !== true && prevIsInternetReachable !== undefined) {
    Log.info(`[NetworkState] Internet reachability restored (${prevIsInternetReachable}→true)`);
    onReachabilityRestored();
    }
    prevIsInternetReachable = state.isInternetReachable;
    });

    And setInternetUnreachable(true) immediately satisfies getIsOffline(), so the offline indicator turns on:

    function getIsOffline(): boolean {
    return !hasRadio || internetUnreachable || sustainedFailuresActive || shouldForceOffline || simulatedOffline;
    }

    This is asymmetric with how regular request failures are treated. FailureTracker requires 3 consecutive connectivity errors within 10 s before flipping sustainedFailuresActive:

    function recordFailure() {
    const now = Date.now();
    if (failureCount === 0) {
    firstFailureTimestamp = now;
    }
    failureCount++;
    const elapsed = now - firstFailureTimestamp;
    const thresholdCount = CONST.NETWORK.SUSTAINED_FAILURE_THRESHOLD_COUNT;
    const thresholdWindow = CONST.NETWORK.SUSTAINED_FAILURE_WINDOW_MS;
    console.debug(`[FailureTracker] Failure #${failureCount} (elapsed: ${elapsed}ms, threshold: ${thresholdCount} in ${thresholdWindow}ms)`);
    // Only trigger sustained failure when BOTH count AND time thresholds are met
    if (failureCount >= thresholdCount && elapsed >= thresholdWindow) {
    console.debug('[FailureTracker] Sustained failure threshold reached — triggering hard stop');
    for (const cb of sustainedFailureListeners) {
    cb(true);
    }
    }
    }

    Ping has no equivalent debouncing. Any single transient Ping failure — fetch rejection, 10 s timeout (MAX_PENDING_TIME_MS in src/CONST/index.ts:2244), non‑200 jsonCode, or non‑JSON body — drives the app offline. Safari is unusually good at producing exactly that: WebKit aborts in‑flight fetch calls when the tab/PWA is hidden or suspended, throttles setTimeout/setInterval aggressively in background tabs, and navigator.onLine is known to flicker briefly to false on network‑interface or sleep/wake transitions.

    2. On web, recovery does not run when the tab becomes visible again.

    When internetUnreachable is true, NetInfo polls Ping every ~5 s. Recovery therefore depends on the next polling tick succeeding. The intentional "foreground refresh" path that exists for native is in Reconnect.ts:

    AppStateMonitor.addBecameActiveListener(() => {
        Log.info('[Reconnect] App became active');
        if (getIsOffline()) {
            refreshNetworkState();
        }
        reconnect();
        flush();
    });

    But that listener is gated by shouldReportActivity, which is false on web:

    import type ShouldReportActivity from './types';
    // We only need to report when the app becomes active on native since web maintains most of it's network functions while
    // in the "background" and the concept is not quite the same on mobile. We avoid setting this to true for web since
    // the event would fire much more frequently than it does on native causing performance issues.
    const shouldReportActivity: ShouldReportActivity = false;
    export default shouldReportActivity;

    function appStateChangeCallback(state: AppStateStatus) {
    if (shouldReportActivity && (previousAppState === CONST.APP_STATE.INACTIVE || previousAppState === CONST.APP_STATE.BACKGROUND) && state === CONST.APP_STATE.ACTIVE) {
    callback();
    }
    previousAppState = state;
    }

    So the web build never calls NetworkState.refresh() on visibilitychange → visible. When a Safari user returns to a tab whose Ping timer was throttled while hidden, the offline indicator stays on until the next polling tick eventually completes — which can be many seconds later, and longer if the first post‑resume Ping itself fails.

    The combination is what produces "almost always shows me as offline": Safari makes single Ping failures more likely, the listener has zero‑tolerance for them, and there is no foreground hook to force an immediate recheck.

    What changes do you think we should make in order to solve the problem?

    Two targeted changes in src/libs/NetworkState.ts, both Safari‑safe and platform‑neutral.

    A. Debounce Ping failures the same way request failures are debounced.

    Track consecutive Ping failures inside NetworkState. Only call setInternetUnreachable(true) after N consecutive isInternetReachable === false events (start with N=2 inside a short window, e.g. 6 s). Reset the counter on the first isInternetReachable === true event. Concretely, in the NetInfo listener:

    // in module scope
    let consecutivePingFailures = 0;
    const PING_FAILURE_THRESHOLD = 2;
    
    // inside addEventListener callback
    if (state.isInternetReachable === false) {
        consecutivePingFailures += 1;
        if (consecutivePingFailures >= PING_FAILURE_THRESHOLD && prevIsInternetReachable !== false) {
            setInternetUnreachable(true);
        }
    } else if (state.isInternetReachable === true) {
        consecutivePingFailures = 0;
        // ... existing recovery branch ...
    }

    This keeps real outages working — three Ping failures still arrive within ~15 s when Ping is genuinely unreachable — but absorbs the Safari‑specific transient failures (one aborted fetch on hide/show, one navigator.onLine flicker) that don't represent a real outage.

    B. Add a web‑only visibilitychange → visible hook that mirrors what addBecameActiveListener does on native.

    Either lift the foreground hook into NetworkState's own subscribe path (web only, behind a typeof document !== 'undefined' guard) or extend AppStateMonitor so a web‑safe variant fires only on hidden→visible transitions. The handler should call refresh() and notify reconnect listeners when getIsOffline() is true. This is the same approach that was discussed on #83780 but never landed; the umbrella PR (#84277) was closed in favour of the broader fix-network-state-detection rework, which addresses request‑level failures but not Ping‑level Safari behaviour.

    Either change alone would noticeably reduce Safari false‑offlines; both together address both the trigger (A) and the slow recovery (B).

    Before merging a fix, I'd suggest temporarily adding a Log.info line at NetworkState.ts:308 that records state.type, state.isConnected, and the previous reachable value. The Safari "stuck offline" sessions in production logs should show a clear pattern (single transient false, then true again on the next tick) that confirms (A) before we tune the threshold.

    What alternative solutions did you explore? (Optional)

    1. Drop useNativeReachability: false on web and rely on navigator.onLine/online/offline events. Rejected: navigator.onLine is the strictly less reliable signal — Safari and Firefox both flicker it spuriously, and it doesn't catch backend outages where the radio is fine but api/Ping is failing. The current Ping‑based design is the right one; it just needs debouncing.

    2. Set shouldReportActivity = true on web. This was explicitly rejected with a comment in shouldReportActivity/index.ts:3-5 — the event fires too frequently on web and would regress unrelated AppState consumers (UnreadIndicatorUpdater, sendMemoryContext). A scoped visibilitychange hook inside NetworkState avoids that blast radius.

    3. Lower MAX_PENDING_TIME_MS (the Ping timeout). Doesn't help — a shorter timeout makes Safari false‑offlines more likely, not fewer.

    4. Skip the offline flip entirely if navigator.onLine === true (the original PR [ON HOLD] Fix false offline state on web (Safari) #84277 approach). Reasonable, but it only catches the Ping‑contention case and still leaves the slow‑recovery problem unsolved when the tab actually was hidden. (A)+(B) covers both.

    Note on prior bot proposals

    A MelvinBot proposal on the closely‑related #83780 suggested a visibilitychange listener that calls a (then‑existing) recheckNetworkConnection(). That proposal was written against the pre‑rework NetworkConnection.ts, which has since been replaced by NetworkState.ts; the same idea adapted to the new module is captured in change (B) above. The proposal did not address the single‑failure trigger (A), which is the reason a visibilitychange‑only fix would still leave the Safari case partially broken.

  9. roryabraham commented on Apr 30, 2026

    @roryabraham
    ContributorAuthor

    I had my AI agent suggest similar solutions, but I don't think any of them address the real root cause - why are 80-90% of Ping requests failing?

  10. aswin-s commented on Apr 30, 2026

    @aswin-s
    Contributor

    🚨 Edited by proposal-police: This proposal was edited at 2026-04-30 19:52:48 UTC.

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    New Expensify sometimes shows the app as offline in Safari or the Safari PWA even though the user still has a stable working connection and Expensify API requests can succeed.

    What is the root cause of that problem?

    src/libs/NetworkState.ts configures NetInfo with useNativeReachability: false, so state.isInternetReachable is driven by NetInfo's JS reachability fetch to:

    `${CONFIG.EXPENSIFY.DEFAULT_API_ROOT}api/Ping?accountID=...`

    When that standalone api/Ping request times out or fails in Safari, the NetInfo listener immediately calls setInternetUnreachable(true). Since getIsOffline() includes internetUnreachable, one failed reachability probe can put the whole app into offline mode.

    The deeper issue is that internetUnreachable is treated independently from normal Expensify API success. The existing FailureTracking middleware already follows the right model for sustained failures: a successful API response proves connectivity and clears failure state. But that success path does not inform internetUnreachable, so a recent successful response from the same Expensify API origin is ignored and a later isolated api/Ping failure can still win.

    We should not solve this with NetInfo's reachabilityShouldRun. In the NetInfo implementation used by this repo, when reachabilityShouldRun() returns false, NetInfo does not preserve the previous reachable state. It calls _setIsInternetReachable(false). Because our listener treats state.isInternetReachable === false as a hard offline signal, using reachabilityShouldRun to skip probes during recent API success can itself produce the offline state we are trying to avoid.

    What changes do you think we should make in order to solve the problem?

    1. In src/CONST/index.ts, add a reachability freshness window under CONST.NETWORK, for example:
    REACHABILITY_SUCCESS_WINDOW_MS: 60 * 1000,

    This matches NetInfo's normal reachable polling interval.

    1. In src/libs/NetworkState.ts, track the last successful response from the same origin as the reachability URL.

    Add helpers along these lines:

    let lastSuccessfulReachabilityDomainResponseAt = 0;
    
    function isReachabilityDomainURL(url: string): boolean {
        try {
            return new URL(url).origin === new URL(CONFIG.EXPENSIFY.DEFAULT_API_ROOT).origin;
        } catch {
            return false;
        }
    }
    
    function hasRecentReachabilityDomainSuccess(): boolean {
        return Date.now() - lastSuccessfulReachabilityDomainResponseAt < CONST.NETWORK.REACHABILITY_SUCCESS_WINDOW_MS;
    }
    
    function recordSuccessfulReachabilityDomainResponse(url: string) {
        if (!isReachabilityDomainURL(url)) {
            return;
        }
    
        lastSuccessfulReachabilityDomainResponseAt = Date.now();
    
        if (internetUnreachable) {
            onReachabilityRestored();
        }
    }

    Export recordSuccessfulReachabilityDomainResponse.

    1. In configureAndSubscribe(), keep NetInfo polling enabled, but ignore a false isInternetReachable event when a recent successful response from the same Expensify API origin already proved reachability.

    Change the false transition branch to:

    if (state.isInternetReachable === false && prevIsInternetReachable !== false) {
        if (!hasRecentReachabilityDomainSuccess()) {
            setInternetUnreachable(true);
        } else {
            Log.info('[NetworkState] Ignoring api/Ping failure because recent API response proved reachability');
        }
    }

    This fixes the false Safari offline transition without relying on reachabilityShouldRun, whose false return value maps to isInternetReachable=false.

    1. In src/libs/HttpUtils.ts, after the test-tool guards and after confirming response.ok, call:
    recordSuccessfulReachabilityDomainResponse(url);

    This should happen before parsing response.json(), so any successful HTTP response from the reachability origin refreshes the freshness window.

    1. Add unit coverage in tests/unit/NetworkStateReachabilityTest.ts for:
    • a recent same-origin API success prevents a following isInternetReachable=false Ping result from setting offline
    • an expired success window allows isInternetReachable=false to set offline
    • different-origin successes do not suppress the Ping failure
    • a successful same-origin response while internetUnreachable is active clears the hard stop and triggers the recovery path
    • no reachabilityShouldRun/shouldReachabilityRun suppression is used, because skipping NetInfo reachability checks can emit false

    What alternative solutions did you explore? (Optional)

    Debouncing failed Ping events would reduce flicker, but it still lets an isolated Safari Ping timeout override stronger evidence from successful Expensify API traffic. Using NetInfo's reachabilityShouldRun is also unsafe here because a false return causes NetInfo to publish isInternetReachable=false. The safer fix is to keep polling active and make NetworkState ignore stale or contradicted Ping failures when recent same-origin API success has already proved reachability.

  11. roryabraham commented on Apr 30, 2026

    @roryabraham
    ContributorAuthor

    cc @adhorodyski in case you have thoughts. Seems like you were working on this recently

  12. 45 remaining items

  13. mallenexpensify commented on May 22, 2026

    @mallenexpensify
    Contributor

    Please comment and tag me if/when payment is due, thx

  14. adhorodyski commented on May 25, 2026

    @adhorodyski
    Contributor

    @mallenexpensify can you confirm if this now works fine for you in staging after #90986? If not, this looks like a separate issue I can look into.

  15. melvin-bot commented on May 26, 2026

    @melvin-bot

    @Pujan92 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

  16. mallenexpensify commented on May 26, 2026

    @mallenexpensify
    Contributor

    @adhorodyski it's working fine, so far as I can tell. Let's assume so and I'll re-report if I have an issue. Thx for fixing!

  17. mallenexpensify commented on May 26, 2026

    @mallenexpensify
    Contributor

    Payment Summary

    Contributor+: @Pujan92 due $250 via NewDot

    Anything else needed here? Don't think a test case is needed.

  18. Pujan92 commented on May 27, 2026

    @Pujan92
    Contributor

    No, I think we are good to close this.

  19. twisterdotcom commented on May 27, 2026

    @twisterdotcom
    Contributor

    $250 approved for @Pujan92 in Report ID: R007bye9LNQS.

    Zapier Logs Run ID: 00040eee-729c-ac8a-cdbe-e70ab6a19c94

    SO: https://stackoverflowteams.com/c/expensify/questions/7582

  20. melvin-bot commented on Jun 1, 2026

    @melvin-bot

    @Pujan92 Eep! 4 days overdue now. Issues have feelings too...

  21. Pujan92 commented on Jun 2, 2026

    @Pujan92
    Contributor

    This issue can be closed @mallenexpensify

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

Metadata

Metadata

Labels

DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions