search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.53k stars 2.88k forks source link

[$250] Search - App freezes after cache and cookie clean of chat filter #49282

Open izarutskaya opened 1 month ago

izarutskaya commented 1 month ago

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: v9.0.35-7 Reproducible in staging?: Y Reproducible in production?: N Found when validating PR : https://github.com/Expensify/App/pull/49258 Email or phone of affected tester (no customers): applausetester+bp0916w@applause.expensifail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

Precondition; logged in application.

  1. Go to Search page > Chats
  2. Click on Filters > Select some users for From field and save the search
  3. Go to Troubleshoot > Reset cache and restart

Expected Result:

App doesn't crash. App doesn'r freeze

Actual Result:

App is freezed

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

https://github.com/user-attachments/assets/a3ed06fc-0200-4ae0-b91c-7a0f470f6a15

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021835730592300773674
  • Upwork Job ID: 1835730592300773674
  • Last Price Increase: 2024-09-23
  • Automatic offers:
    • rayane-djouah | Reviewer | 104197919
rayane-djouah commented 3 weeks ago

@Guccio163 - I was able to reproduce the bug. To do so, we need to resize the window to a size smaller than the screen size.

https://github.com/user-attachments/assets/15801f9e-4a48-4864-a070-f9132f35e8c0

Console Logs: dev.new.expensify.com-1728983762370.log

Guccio163 commented 3 weeks ago

Thanks for the insight @rayane-djouah, I'll try this one in a moment! 🫶

Guccio163 commented 3 weeks ago

Hi @Christinadobrzyn, I'm coming with my conclusions on this issue: There's a list of tests I performed here to try and reproduce the bug:

Reported bug behaviour: App freezes and doesn't respond

Current observed behaviour (One of two, non-deterministic) :

  1. User's details and inbox loading infinitely
  2. All informations loaded right after refresh

Moreover I'm pretty much sure that this bug isn't connected to Search at all; To illustrate the non-deterministic nature of this bug, below are recordings of app behaviour with and without the search-related steps:

  1. With selecting search filters
  2. Without selecting filters (just clearing cache)

Please notice that no matter with or without filters, app behaves the same in one of 2 defined behaviours above:

Staging: filters then cache clear - loading loop https://github.com/user-attachments/assets/c68ca794-25e0-4904-bf35-d8ff7b5b2f55
Staging: filters then cache clear - loading normally https://github.com/user-attachments/assets/af9b4aab-2fa7-4376-98bd-61d2c2bf8bec
Staging: Just cache clear - loading loop https://github.com/user-attachments/assets/fb362bdd-4d20-40c6-861b-0b625173b5dd
Staging: Just cache clear - loading normally https://github.com/user-attachments/assets/bd3a3ee7-1d3a-4ade-beab-58f1e2ed04da

Also, all these results are possible to spot in Local environment too:

Local: filters then cache clear - loading loop https://github.com/user-attachments/assets/4647ada4-a39a-413b-8e93-218dbb4e0620
Local: filters then cache clear - loading normally https://github.com/user-attachments/assets/55fa3a97-cbb1-4e37-a3cd-6a23af79904c
Local: Just cache clear - loading loop https://github.com/user-attachments/assets/20bd7dde-ce3b-4b79-a66b-8f1874effe4f
Local: Just cache clear - loading normally https://github.com/user-attachments/assets/8a566938-127c-41ac-85bd-56156aec2beb

As a final addition, exactly the same infinite loading happens (non-deterministic) right after logging in:

Staging: Right after logging in - loading loop https://github.com/user-attachments/assets/ce33ac67-da91-4071-9d3b-9dc144872d7d

In my opinion incorrect behaviour isn't linked to Search and is purely matter of "Clear cache and restart" button; Maybe the contributor that added this feature or someone with bigger Onyx knowledge could take this and dive into the button's functionality?

Christinadobrzyn commented 2 weeks ago

Hi @Guccio163! Thank you so much for the thorough review. It sounds like we need to find an Onyx person to investigate more. I think @kidroca does Onyx stuff but I don't know if it relates to this kind of job.

@tylerkaraszewski or @rayane-djouah do you know who else we might be able to reach out to about this?

rayane-djouah commented 2 weeks ago

@tylerkaraszewski or @rayane-djouah do you know who else we might be able to reach out to about this?

@Christinadobrzyn, the feature was worked on by @TMisiukiewicz in this PR: https://github.com/Expensify/App/pull/35306

Christinadobrzyn commented 2 weeks ago

Awesome, thanks @rayane-djouah! I reached out to the Callstack team to see if they might have someone with Onyx experience to help! https://expensify.slack.com/archives/C03UK30EA1Z/p1729654603736149

TMisiukiewicz commented 2 weeks ago

Hey, I'll take a look on this later this week 👍

Christinadobrzyn commented 2 weeks ago

Awesome! Thank you @TMisiukiewicz!

TMisiukiewicz commented 2 weeks ago

Started investigating it, I couldn't reporoduce it based on the steps provided in the issue. However, the browser tab hang to me once when I did clear a couple of times, it felt like each next clear is slowing down the app, however I haven't done any measurements yet. I'll continue looking into that on Monday

TMisiukiewicz commented 1 week ago

I was able to reproduce it with 6x CPU throttling enabled in devtools. However, it was impossible to record a profiling trace because the devtools freezes together with the app 😞 One thing I noticed is the fact that my app hangs every time after merge being called on personalDetailsList with more than 8k reports. @izarutskaya mind trying it out on your own? Please open a Chrome devtools and go to Console tab, enable All levels for logs and try to reproduce the bug. What is the last log before it freezes? Is it any kind of Onyx action with a large payload?

Christinadobrzyn commented 1 week ago

Thanks for the testing @TMisiukiewicz - reaching out to QA to see if they can test again based on those instructions. https://expensify.slack.com/archives/C9YU7BX5M/p1730176431155529

izarutskaya commented 1 week ago

Still reproducible but I am not sure what should I see. Check please my video Build v9.0.55-2

https://github.com/user-attachments/assets/581fa8aa-6516-4df6-8424-e5f3663ded7e

TMisiukiewicz commented 1 week ago

@izarutskaya thanks, looks like it hangs exactly at the same point.

So today I was finally able to reproduce it and found the root cause. When we open the Search in chats selector, we initialize the option list to load the items that are visible in the list: https://github.com/Expensify/App/blob/65fa00a002b3603d335e9a3180b10d6fa365cd58/src/components/Search/SearchFiltersChatsSelector.tsx#L46-L48

This means, now every time the personalDetails list change, we will fall into this useEffect and execute the whole code since the early return won't work anymore: https://github.com/Expensify/App/blob/65fa00a002b3603d335e9a3180b10d6fa365cd58/src/components/OptionListContextProvider.tsx#L82-L128

As you can see, it loops through all the personal details, and for each single one of them, it loops through all reports using filter and then forEach. This issue probably occurs only on heavy loaded accounts - for me it's reproducible on account where I have 8,5k personal details and more than 15k reports. I was able to measure it using profiler and it blocks my app for 50 seconds:

image

And here is the confirmation that loops mentioned earlier are responsible for that:

image

In this useEffect we have early return comparing the previous value of personal details with the current one, and it should do the work in all the other cases where this value gets updated. However in here it's kinda specific scenario, and it will never fall into this early return because the previous value of personal details is just an empty object, so we run the code for each one of them.

Haven't tested any solution yet, but first and easiest thing that comes to my mind is just setting areOptionsInitialized to false when clearing the cache. Will test it tomorrow morning and open a PR if it works properly.

TMisiukiewicz commented 1 week ago

PR open: https://github.com/Expensify/App/pull/51725

mind checking if the issue is gone on your side when testing out this branch?

TMisiukiewicz commented 1 week ago

I'll be ooo until the end of the week so @MrRefactor will take care of this if needed

MrRefactor commented 1 week ago

I will adjust PR and upload all of the recordings necessary!

rayane-djouah commented 5 days ago

PR open: #51725

mind checking if the issue is gone on your side when testing out this branch?

I have tested the branch and confirmed that it resolves the bug 👍 - https://github.com/Expensify/App/pull/51725#issuecomment-2453221465