Skip to content

[$250] Latest distance type is updated in 'Simulate failing network requests' mode #89701

Closed

Description

@jponikarchuk

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.3.67-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from BrowserStack: #82786
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team
Bug source: Pull Request QA execution
Device used: Windows 11 Home/Chrome
App Component: Money Requests

Action Performed:

  1. Open https://staging.new.expensify.com/
  2. Troubleshoot and toggle Simulate failing network requests
  3. Click FAB
  4. Click on 'Track distance'
  5. Select another distance type than default one (ex: default is Map, select Manual)
  6. Create the distance expense
  7. Click on the FAB
  8. Select 'Track distance'

Expected Result:

The last-used distance type should not be updated.

Actual Result:

The last-used distance type is udpated

Workaround:

Unknown

Platforms:

  • Android: App
  • Android: mWeb Chrome
  • iOS: App
  • iOS: mWeb Safari
  • iOS: mWeb Chrome
  • Windows: Chrome
  • MacOS: Chrome / Safari

Screenshots/Videos

Video.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~022051866017210375016
  • Upwork Job ID: 2051866017210375016
  • Last Price Increase: 2026-05-13
Issue OwnerCurrent Issue Owner: @justinpersaud

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on May 6, 2026
  2. MelvinBot commented on May 6, 2026

    @MelvinBot
    Contributor

    Proposal

    What is the root cause of that problem?

    When a distance expense is created, the last-used distance type (NVP_LAST_DISTANCE_EXPENSE_TYPE) is updated via Onyx optimisticData — meaning it's applied immediately, before the API call completes. However, there is no corresponding failureData entry to roll it back when the API call fails. So when "Simulate failing network requests" is on (or any real network failure), the preference persists even though the expense wasn't actually created.

    The bug exists in two places:

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

    Add failureData entries that revert NVP_LAST_DISTANCE_EXPENSE_TYPE to its previous value when the API call fails, in both TrackExpense.ts and Split.ts. The previous value needs to be passed into these functions (or read before the optimistic update) so it can be restored on failure.

    The correct pattern is already demonstrated right next to the bug — the ODOMETER_DRAFT handling at TrackExpense.ts:2461-2472 has both optimisticData and failureData. Apply the same pattern to NVP_LAST_DISTANCE_EXPENSE_TYPE.


    Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, @MelvinBot <your feedback> to refine this analysis, or explain why you are rejecting Melvin's proposal.

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

    @melvin-bot

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

  6. mukhrr commented on May 6, 2026

    @mukhrr
    Contributor

    🚨 Edited by proposal-police: This proposal was edited at 2026-05-11 17:00:43 UTC.

    Verified: the pinned SHA 39d1d17e9b8… still resolves and the line numbers are correct (TrackExpense.ts L2442-L2448, Split.ts L1992-L1998), so the permalinks still render.

    Here's the corrected proposal to replace comment 4384844594 with:


    Proposal

    What is the root cause of that problem?

    NVP_LAST_DISTANCE_EXPENSE_TYPE is written via optimisticData — the "remembered distance type" is recorded the instant the create is attempted, before it's known to succeed:

    • if (isDistanceRequest && isDistanceExpenseType(transaction?.iouRequestType)) {
      onyxData?.optimisticData?.push({
      onyxMethod: Onyx.METHOD.SET,
      key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
      value: transaction?.iouRequestType,
      });
      }
    • if (isDistanceExpenseType(transaction.iouRequestType)) {
      onyxData?.optimisticData?.push({
      onyxMethod: Onyx.METHOD.SET,
      key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
      value: transaction.iouRequestType,
      });
      }

    So when the create fails (Simulate failing network requests, or any real failure), the attempted type is already persisted and drives the next Track-distance flow. startDistanceRequest routes off this NVP, and DistanceRequestStartPage falls back to it / the persisted selectedTab_distanceRequestType:

    • switch (requestType) {
      case CONST.IOU.REQUEST_TYPE.DISTANCE_MAP:
      Navigation.navigate(ROUTES.DISTANCE_REQUEST_CREATE_TAB_MAP.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID, backToReport));
      return;
      case CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL:
      Navigation.navigate(ROUTES.DISTANCE_REQUEST_CREATE_TAB_MANUAL.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID, backToReport));
      return;
      case CONST.IOU.REQUEST_TYPE.DISTANCE_GPS:
      Navigation.navigate(ROUTES.DISTANCE_REQUEST_CREATE_TAB_GPS.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID, backToReport));
      return;
      default:
      Navigation.navigate(ROUTES.DISTANCE_REQUEST_CREATE.getRoute(CONST.IOU.ACTION.CREATE, iouType, CONST.IOU.OPTIMISTIC_TRANSACTION_ID, reportID, backToReport));
      }
    • const transactionRequestType = useMemo(() => {
      if (!transaction?.iouRequestType) {
      return lastDistanceExpenseType ?? selectedTab ?? CONST.IOU.REQUEST_TYPE.DISTANCE_MAP;
      }
      return transaction.iouRequestType;
      }, [transaction?.iouRequestType, selectedTab, lastDistanceExpenseType]);

    Crucially, a "restore the previous value on failureData" approach cannot fix this (this is why every prior proposal of that shape doesn't work). Empirically, with a restore-on-failure patch applied: attempt 1 reverts correctly (→ Map), but attempt 2 in the same session stays stuck on the failed type, and it only resets if you reload before the 2nd attempt. The reason: by the time the 2nd attempt captures the "previous" value, that value is itself the polluted optimistic value left by attempt 1's optimistic write (applied synchronously by API.write; the deferred-write path widens the window). Restoring to a corrupted baseline compounds the bug. A reload clears the in-memory pollution, which is why it then behaves correctly — proving the defect is the optimistic write itself, not a missing rollback.

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

    Record the remembered type only once the create actually succeeds, and clear the immediately-persisted distance tab on failure (a plain clear — no captured "previous" value, so nothing can compound).

    In src/libs/actions/IOU/TrackExpense.ts (and the equivalent block in createDistanceRequest in Split.ts):

     if (isDistanceRequest && isDistanceExpenseType(transaction?.iouRequestType)) {
    -    onyxData?.optimisticData?.push({
    +    onyxData?.successData?.push({
             onyxMethod: Onyx.METHOD.SET,
             key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
             value: transaction?.iouRequestType,
         });
    +    onyxData?.failureData?.push({
    +        onyxMethod: Onyx.METHOD.SET,
    +        key: `${ONYXKEYS.COLLECTION.SELECTED_TAB}${CONST.TAB.DISTANCE_REQUEST_TYPE}`,
    +        value: null,
    +    });
     }

    successData ensures a pending/failed/offline attempt never records the type, so there is no optimistic value to leak and nothing to "restore" — attempt N behaves identically to attempt 1, with or without a reload. The unconditional failureData clear of selectedTab_distanceRequestType handles the tab that OnyxTabNavigator persists the moment the user switches it (independent of the request), which DistanceRequestStartPage falls back to. Supporting change: add typeof ONYXKEYS.COLLECTION.SELECTED_TAB to the OnyxData<…> key unions in TrackedExpenseParams.ts and the local unions in shareTrackedExpense/createDistanceRequest so the selectedTab_ entry typechecks.

    What alternative solutions did you explore? (Optional)

    N/A

  7. melvin-bot commented on May 6, 2026

    @melvin-bot
  8. changed the title [-]Latest distance type is updated in 'Simulate failing network requests' mode[/-] [+][$250] Latest distance type is updated in 'Simulate failing network requests' mode[/+] on May 6, 2026
  9. aswin-s commented on May 6, 2026

    @aswin-s
    Contributor

    🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

  10. github-actions commented on May 6, 2026

    @github-actions
    Contributor

    ⚠️ @aswin-s Your proposal is a duplicate of an already existing proposal and has been automatically withdrawn to prevent spam. Please review the existing proposals before submitting a new one.

  11. wildan-m commented on May 6, 2026

    @wildan-m
    Contributor

    🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

  12. trasnake87 commented on May 6, 2026

    @trasnake87
    Contributor

    Proposal

    The "last-used distance type" preference is being persisted optimistically with no rollback, so toggling "Simulate failing network requests" (or any real network failure) leaves the NVP updated even though the expense never made it to the server.

    Root cause

    NVP_LAST_DISTANCE_EXPENSE_TYPE is written into optimisticData in two places, neither of which adds a corresponding failureData entry to revert the value when the API call fails. Onyx applies the optimistic write immediately, the request errors out under the simulator, and because there is no failure entry, the new value sticks — driving the next "Track distance" tap to the wrong default.

    1 - trackExpense flow (src/libs/actions/IOU/TrackExpense.ts:2448-2457):

    // src/libs/actions/IOU/TrackExpense.ts (current)
    const isDistanceRequest =
        isMapDistanceRequest(transaction) || isManualDistanceRequestTransactionUtils(transaction) || isOdometerDistanceRequestTransactionUtils(transaction) || isGPSDistanceRequest;
    
    if (isDistanceRequest && isDistanceExpenseType(transaction?.iouRequestType)) {
        onyxData?.optimisticData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: transaction?.iouRequestType,
        });
    }

    Compare this with the NVP_RECENT_WAYPOINTS handling 9 lines above, which does push a corresponding failureData entry that restores recentServerValidatedWaypoints (src/libs/actions/IOU/TrackExpense.ts:2439-2444). The distance-type write is missing that symmetric rollback.

    2 - createDistanceRequest flow (src/libs/actions/IOU/Split.ts:1996-2002):

    // src/libs/actions/IOU/Split.ts (current)
    if (isDistanceExpenseType(transaction.iouRequestType)) {
        onyxData?.optimisticData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: transaction.iouRequestType,
        });
    }

    Same shape, same omission. This is the path used when the user is on the global FAB → Track distance flow and a single (non-track) distance request is created — which matches the reproduction steps in the issue (FAB → Track distance → Manual → submit).

    3 - Why this surfaces only under the failure simulator:

    In normal use the API succeeds, the optimistic value happens to coincide with what the server would have stored anyway, so the bug is invisible. With "Simulate failing network requests" on, every write fails and the missing failureData becomes observable — the preference is now whatever the user attempted, not what was actually saved.

    Proposed fix

    Capture the prior value of NVP_LAST_DISTANCE_EXPENSE_TYPE via a module-level Onyx.connect (the same pattern used for allPersonalDetails / allTransactions in src/libs/actions/IOU/index.ts:285-358), then add a failureData entry that restores it when the optimistic distance-type write is queued.

    Change 1 — src/libs/actions/IOU/TrackExpense.ts

    Add a module-level cache near the existing top-of-file imports/state (i.e. immediately after the imports block, before function trackExpense(...)):

    // src/libs/actions/IOU/TrackExpense.ts (new — add near other module-level state)
    let lastDistanceExpenseType: OnyxEntry<OnyxTypes.DistanceExpenseType>;
    Onyx.connect({
        key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
        callback: (value) => {
            lastDistanceExpenseType = value;
        },
    });

    Then replace src/libs/actions/IOU/TrackExpense.ts:2451-2457 with:

    // src/libs/actions/IOU/TrackExpense.ts (replace lines 2451-2457)
    if (isDistanceRequest && isDistanceExpenseType(transaction?.iouRequestType)) {
        onyxData?.optimisticData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: transaction?.iouRequestType,
        });
        onyxData?.failureData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: lastDistanceExpenseType ?? null,
        });
    }

    Make sure DistanceExpenseType is imported (it's already exported from src/types/onyx/IOU.ts:234 and re-exported via OnyxTypes).

    Change 2 — src/libs/actions/IOU/Split.ts

    Add the same module-level connect once near the top of the file:

    // src/libs/actions/IOU/Split.ts (new — add near other module-level state)
    let lastDistanceExpenseType: OnyxEntry<OnyxTypes.DistanceExpenseType>;
    Onyx.connect({
        key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
        callback: (value) => {
            lastDistanceExpenseType = value;
        },
    });

    Then replace src/libs/actions/IOU/Split.ts:1996-2002 with:

    // src/libs/actions/IOU/Split.ts (replace lines 1996-2002)
    if (isDistanceExpenseType(transaction.iouRequestType)) {
        onyxData?.optimisticData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: transaction.iouRequestType,
        });
        onyxData?.failureData?.push({
            onyxMethod: Onyx.METHOD.SET,
            key: ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE,
            value: lastDistanceExpenseType ?? null,
        });
    }

    Using ?? null (rather than ?? undefined) is important — the very first time a user creates a distance expense, the NVP doesn't exist yet, and on failure the value should go back to "unset" so consumers like useExpenseActions.ts:148 and DistanceRequestStartPage.tsx:49 fall back to their defaults exactly as they did before the failed attempt. Onyx.METHOD.SET with null clears the key.

    Why this approach

    • It is exactly symmetric to the pre-existing rollback applied to NVP_RECENT_WAYPOINTS 10 lines above the bug (TrackExpense.ts:2440-2444, Split.ts:2047-2051) — the same offline-first rollback pattern, applied to the same kind of NVP. Reviewers don't need to reason about a new mechanism.
    • A module-level Onyx.connect is the established way both action files already read live NVPs/collections without forcing every caller to thread a new prop through (see the allPersonalDetails / allTransactionDrafts connects in src/libs/actions/IOU/index.ts:285-358). The alternative — adding a lastDistanceExpenseType parameter to trackExpense and createDistanceRequest — was rejected because it would touch every caller (MoneyRequest.ts:215, MoneyRequest.ts:670, plus Share/SubmitDetailsPage.tsx, IOURequestStepDistanceManual.tsx, IOURequestStepDistanceMap.tsx, IOURequestStepDistanceGPS/index.native.tsx, IOURequestStepAmount.tsx) for no behavioral gain.
    • Reading the value at the moment the optimistic write is constructed (rather than deriving it from request params) is the only safe way to roll back to the true prior value — including the undefined first-time case — instead of guessing it from the new transaction's type.
    • The fix is fully scoped to the two production sites that write the NVP (grep -rn "NVP_LAST_DISTANCE_EXPENSE_TYPE" src/libs/actions/ confirms no other writers exist), so no other call paths can regress.
  13. 33 remaining items

  14. MelvinBot commented on May 18, 2026

    @MelvinBot
    Contributor

    The bug was introduced by PR #69372 ("[Manual distance] Create the distance creation tabs v2", merged 2025-08-29).

    That PR added the NVP_LAST_DISTANCE_EXPENSE_TYPE Onyx key and wrote it via optimisticData in createDistanceRequest without a corresponding failureData entry to roll it back on API failure. The same pattern was later carried into trackExpense when the code was refactored (the extraction into TrackExpense.ts / Split.ts).

    The feature was initially behind a MANUAL_DISTANCE beta flag. PR #71474 ("Remove the manual distance beta", merged 2025-09-29) removed that gate, exposing the bug to all users — though it was only caught during QA of v9.3.67.

  15. huult commented on May 18, 2026

    @huult
    Contributor

    Thanks for all proposal.

    @emkhalid @dilshodmackbook-sketch Your solution can cause a bug if you only update successData, because it waits for the API response before updating the value. So before the response returns, the value is not updated, meaning it won’t update immediately.

    Screen.Recording.2026-05-18.at.10.18.12.mov

    @mukhrr Did you test your proposal? If you're sure it works, could you share your test branch or record a video and post it here?

  16. huult commented on May 18, 2026

    @huult
    Contributor

    Looking for the proposal

  17. emkhalid commented on May 18, 2026

    @emkhalid
    Contributor

    @huult thanks again for the feedback. I updated my proposal with a different approach.

    Instead of writing the confirmed last distance type optimistically, the updated proposal keeps NVP_LAST_DISTANCE_EXPENSE_TYPE as the last successfully created distance type, and uses a temporary pending distance type only for the immediate pre-response UX. I also stopped treating selectedTab_distanceRequestType as persisted state for the distance flow, so it should not keep a failed tab selection in IndexedDB.

    This should preserve the immediate update you pointed out while keeping the failed-request state clean. I’ll test this version and attach a fresh video demo.

  18. mukhrr commented on May 18, 2026

    @mukhrr
    Contributor

    @mukhrr Did you test your proposal? If you're sure it works, could you share your test branch or record a video and post it here?

    @huult here is demo of the fix and also updated proposal

    REC-20260518092355.mp4
  19. huult commented on May 20, 2026

    @huult
    Contributor
    Screen.Recording.2026-05-20.at.11.37.12.mov

    if (newSelectedTab) {
    Tab.setSelectedTab<TTabName>(id, newSelectedTab as TTabName);
    }

    After reviewing and tracing the code changes again, I don’t think this is a bug. It looks like expected behavior. I’d like to ask Internal to confirm before making a decision.

    Hi Internal team, could you help review the expected behavior for this ticket?

    When tracking distance and the network is offline or fails:

    I successfully track distance online with selected tab = manual
    Then I switch the tab to GPS and submit while offline or with a failed network

    When I open the tracking distance tab again, should the selected type be GPS or manual?

    The expected behavior in this ticket is manual, but I think it should be GPS since we don’t need to restore the previously selected online tab. Could you please confirm if this is correct?

    🎀 👀 🎀 c+ reviewed.

  20. melvin-bot commented on May 20, 2026

    @melvin-bot

    Triggered auto assignment to @justinpersaud, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

  21. nicktofel208 commented on May 20, 2026

    @nicktofel208

    🚫 Duplicated proposal withdrawn by 🤖 ProposalPolice.

  22. melvin-bot commented on May 20, 2026

    @melvin-bot

    📣 @nicktofel208! 📣
    Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
    Please follow these steps:

    1. Make sure you've read and understood the contributing guidelines.
    2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
    3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
    4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
      Screen Shot 2022-11-16 at 4 42 54 PM
      Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
    
  23. github-actions commented on May 20, 2026

    @github-actions
    Contributor

    ⚠️ @nicktofel208 Your proposal is a duplicate of an already existing proposal and has been automatically withdrawn to prevent spam. Please review the existing proposals before submitting a new one.

  24. melvin-bot commented on May 20, 2026

    @melvin-bot

    @justinpersaud @huult this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

  25. justinpersaud commented on May 21, 2026

    @justinpersaud
    Contributor

    What @huult posted is correct. Lets just close this.

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

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributorHelp WantedApply this label when an issue is open to proposals by contributors

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions