Skip to content

[Due for payment 2025-05-27] [$500 AND $250] Investigate codebase for potential endless loops #60889

Closed

Description

@mountiny

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:
Reproducible in staging?:
Reproducible in production?:
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @JmillsExpensify
Slack conversation (hyperlinked to channel name): https://expensify.slack.com/archives/C05LX9D6E07/p1745490519573089

Action Performed:

Break down in numbered steps

  1. Go to Reports
  2. Search some query

Expected Result:

Describe what you think should've happened

User gets valid reponse

Actual Result:

Describe what actually happened

The browser just hangs as there is some js loop

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Select the officially supported platforms where the issue was reproduced:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • Windows: Chrome
  • MacOS: Chrome / Safari
  • MacOS: Desktop
Platforms Tested: On which of our officially supported platforms was this issue tested:
  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • Windows: Chrome
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @alexpensify

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Apr 25, 2025
  2. melvin-bot commented on Apr 25, 2025

    @melvin-bot

    Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

  3. melvin-bot commented on Apr 25, 2025

    @melvin-bot

    Triggered auto assignment to @aldo-expensify (AutoAssignerNewDotQuality)

  4. mountiny commented on Apr 25, 2025

    @mountiny
    ContributorAuthor

    @aldo-expensify Check out the slack thread, the idea here is that someone from Callstack will jump on this and try to investigate the codebase around search for some possible endless loops

  5. sosek108 commented on Apr 25, 2025

    @sosek108
    Contributor

    Hi, I'm Hubert from Callstack and I would like to take a look at this issue.

  6. aldo-expensify commented on Apr 28, 2025

    @aldo-expensify
    Contributor

    👋 @sosek108 how is this going?

  7. sosek108 commented on Apr 28, 2025

    @sosek108
    Contributor

    On slack conversation we diagnosed that potential cause for problem is SuffixUkkonenTree algorithm. There is no endless loop, but a lot of calls for finite loops

    I have proposal for minor fixes to that algorithm,

  8. sosek108 commented on Apr 28, 2025

    @sosek108
    Contributor

    🚨 Edited by proposal-police: This proposal was edited at 2025-04-28 13:06:15 UTC.

    Proposal

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

    The convertToBase26 function incorrectly divides by 32 using a bitwise shift (>> 5) instead of properly dividing by 26. This causes wrong base-26 conversions for many numbers, especially above 26, and leads to incorrect results. Additionally, the caching mechanism incorrectly stores results under the wrong key due to modifying the input num inside the function.

    What is the root cause of that problem?

    The root cause is twofold:

    Misuse of bitwise shifting (>> 5) under the false assumption that it mimics division by 26, while it actually divides by 32.

    Caching the result after modifying the input parameter, resulting in wrong cache keys and cache misses.

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

    Replace the bitwise shift (num >>= 5) with correct integer division (Math.floor(num / 26)).

    Preserve the original num value in a separate variable before any mutation, and use the original number as the key when caching the result.

    Slightly improve readability by avoiding mutation of the input parameter directly.

    What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

    Validate that numbers around 26, 32, 52, 53, and 63 correctly convert to base-26 arrays.

    Add tests for small numbers (e.g., 1, 2, 25, 26, 27) to ensure basic behavior.

    Include tests with numbers where division by 26 and division by 32 would differ to specifically catch this kind of error

    Add a test ensuring that negative inputs throw the correct error message.

    These tests ensure correctness of logic for both typical cases and edge cases, and will immediately fail if future changes accidentally introduce wrong division logic again.

    What alternative solutions did you explore? (Optional)

    N/A

  9. 65 remaining items

  10. mountiny commented on May 21, 2025

    @mountiny
    ContributorAuthor

    Ok yeah so we have $500 to @ZhenjaHorbach and $250 to @jayeshmangwani

  11. alexpensify commented on May 21, 2025

    @alexpensify
    Contributor

    Ok, we will use those numbers on the payment date of 2025-05-27.

  12. changed the title [-][Due for payment 2025-05-27] [Due for payment 2025-05-22] [Due for payment 2025-05-14] Investigate codebase for potential endless loops[/-] [+][Due for payment 2025-05-27] Investigate codebase for potential endless loops[/+] on May 21, 2025
  13. melvin-bot commented on May 27, 2025

    @melvin-bot

    Payment Summary

    Upwork Job

    BugZero Checklist (@alexpensify)

    • I have verified the correct assignees and roles are listed above and updated the necessary manual offers
    • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
    • I have paid out the Upwork contracts or cancelled the ones that are incorrect
    • I have verified the payment summary above is correct
  14. changed the title [-][Due for payment 2025-05-27] Investigate codebase for potential endless loops[/-] [+][Due for payment 2025-05-27] [$500] Investigate codebase for potential endless loops[/+] on May 28, 2025
  15. melvin-bot commented on May 28, 2025

    @melvin-bot

    ⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

  16. alexpensify commented on May 28, 2025

    @alexpensify
    Contributor

    Payment Summary

    Contributor: @jayeshmangwani due $250 via NewDot - please submit a request
    Contributor: @ZhenjaHorbach due $500 via NewDot - please submit a request

  17. jayeshmangwani commented on May 28, 2025

    @jayeshmangwani
    Contributor

    @alexpensify Please update the payment summary - $250 is due for me.

  18. alexpensify commented on May 28, 2025

    @alexpensify
    Contributor

    @jayeshmangwani - done! Thanks for the update.

  19. changed the title [-][Due for payment 2025-05-27] [$500] Investigate codebase for potential endless loops[/-] [+][Due for payment 2025-05-27] [$500 + $250] Investigate codebase for potential endless loops[/+] on May 28, 2025
  20. changed the title [-][Due for payment 2025-05-27] [$500 + $250] Investigate codebase for potential endless loops[/-] [+][Due for payment 2025-05-27] [$500 AND $250] Investigate codebase for potential endless loops[/+] on May 28, 2025
  21. garrettmknight commented on Jun 5, 2025

    @garrettmknight
    Contributor

    $250 approved for @jayeshmangwani

  22. ZhenjaHorbach commented on Jun 19, 2025

    @ZhenjaHorbach
    Contributor

    Requested in ND

  23. garrettmknight commented on Jun 26, 2025

    @garrettmknight
    Contributor

    $250 approved for @ZhenjaHorbach

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

Metadata

Metadata

Labels

AutoAssignerNewDotQualityUsed to assign quality issues to engineersAwaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.WeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions