Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mangane hangs the browser on an internet/connection hiccup #355

Open
TruncatedDinoSour opened this issue Jan 2, 2025 · 12 comments
Open

Comments

@TruncatedDinoSour
Copy link

The issue's in the title; Mangane's front-end somehow manages to (semi-)hang Firefox, that is until you close and reopen the tab until a fetch is complete(?)

I understand that JS is a single-threaded language, however, maybe it would be beneficial for Mangane to make use of async in JS to avoid hanging the tab page?

:)

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

to clarify, i am on unstable 5g home internet. if the internet pauses for a moment and i try to navigate or perform any action that relies on a network fetch, it will quite literally freeze the entire firefox tab until the network request completes. in my experience i wasnt even able to normally close it or refresh it before my internet came back. this happened twice so far. things like ff screenshot (was gonna screenshot it when it was hung) will also not pop up until the network comes back.

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

image
i tried to use firefox performance recording tool thingy when it hung ;-;

@TruncatedDinoSour
Copy link
Author

I was able to reproduce the bug. It seems that when fetching a(?) status it goes into an infinite loop trying to fetch the context then the status then the context then the status ... and so on

The related API calls:

image

@TruncatedDinoSour
Copy link
Author

image

The bug is clearly in the fetchStatus function, having nearly 40000 samples

@TruncatedDinoSour
Copy link
Author

TruncatedDinoSour commented Jan 2, 2025

image

And this answers why it goes crazy. It uses the one (1) thread JS has to use and just dies, however, for now, I am unsure how we could resolve this since it is in a promise.

furthermore,

image

the flame graph also supports this

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

idk im a nodejs pleb but i guess stupid goes with typeof res === "promise" or sum, or theres probably a better way to do that. theres also a whole js thing to wrap promises and check them

also when mangane isnt hanging it seems to be using an obscene amount of ram?
image

@TruncatedDinoSour
Copy link
Author

TruncatedDinoSour commented Jan 2, 2025

idk im a nodejs pleb but i guess stupid goes with typeof res === "promise" or sum, or theres probably a better way to do that. theres also a whole js thing to wrap promises and check them

also when mangane isnt hanging it seems to be using an obscene amount of ram? image

Could you hover over the ari.lt tab and click the "profile" button that appears next to it on hover? Give it a minute or two to profile, scroll around and stuff, and then try to stop the profiling by hitting the blue profiling button in your navline.

ok its js 5 secs

image

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

never mind i tried to use it and it was hung, i killed it again and reloaded and its fine for now

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

ill try to do that if it happens again but last time firefox just spontaneously crashed

@TruncatedDinoSour
Copy link
Author

ill try to do that if it happens again but last time firefox just spontaneously crashed

I believe this is probably due to the nature of the bug, it seems to be going into a recursion? or maybe just an infinite loop? and pushing things on the stack over and over and over again

b-2025-01-02_14.12.15.mp4

this smells like recursion to me honestly

@jjj333-p
Copy link

jjj333-p commented Jan 2, 2025

gecko tail recursion optimization when

@TruncatedDinoSour
Copy link
Author

TruncatedDinoSour commented Jan 2, 2025

I think the network thing is just a side-effect of the core issue. I tried to make a reliable way to reproduce the bug and it did not end up freezing the browser, however, the network effects of API calls (status => context => status => context => ...) still persisted, but with my changes this is only reproduceable when you navigate through notifications to a post.

I changed the fetchStatus function to a fake endpoint (v1 => v2, which returns a 404) and have this:

const fetchStatus = (id: string) => {
  return (dispatch: AppDispatch, getState: () => RootState) => {
    const skipLoading = statusExists(getState, id);

    dispatch({ type: STATUS_FETCH_REQUEST, id, skipLoading });

    return api(getState).get(`/api/v2/statuses/${id}`).then(({ data: status }) => {
      dispatch(importFetchedStatus(status));
      dispatch({ type: STATUS_FETCH_SUCCESS, status, skipLoading });
      return status;
    }).catch(error => {
      dispatch({ type: STATUS_FETCH_FAIL, id, error, skipLoading, skipAlert: true });
    });
  };
};

And it began going in a loop.

Through, I believe once your connection faces a hiccup either some state fails or everything just catches on fire, metaphorically, and if every single hook begins going in an infinite loop, you end up with a frozen tab. Just like while (1) console.log(1); slows down the tab, if we have multiple, say like while (1) console.log(1);while (1) console.log(1);while (1) console.log(1); all at once, it makes it unresponsive.

Since, imagine:

image

Every single one of these could turn into an infinite loop which adds onto the browser load. Maybe this wouldn't be as much of an issue on chromium browsers because they tend to generally handle these things better, but on Firefox this issue is very prominent.

Now, I'm wondering whether this is a question of inappropriate error handling...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants