MR page constantly shows AJAX error banners

Error banners like "Failed to fetch comments", etc.. constantly show up in MRs even when they were loaded successfully.

I can reproduce by reloading the page and switching to another tab while the requests are going on. When I switch back, I see the error even though the comments are loaded fine.

Screen_Recording_2020-03-12_at_2.27.16_PM

I'm on Firefox 73.0.1. And for now, I've only been able to reproduce it in MR pages.

added devopscreate frontend groupsource code typebug labels

I can reproduce on Chrome 80 too. Something went wrong while fetching latest comments.

I know we do a lot of AJAX requests on the MR page, so that may have something to do with this?

Not sure if the browser has a limit on the number of requests when the tab is inactive.

mentioned in issue #211281 (closed)

mentioned in issue #212105 (closed)

mentioned in issue #212801 (closed)

mentioned in issue #213486 (closed)

mentioned in issue #214178 (closed)

mentioned in issue #214948 (closed)

mentioned in issue #215814 (closed)

mentioned in issue #216409 (closed)

mentioned in issue #217268 (closed)

I believe this comes from using the action for fetching notes in multiple scenarios: first load and polling, at least.

That error message is on a very generically named fetchData action and poll.

I can't reproduce it with Firefox 76. Screenshot

I wonder if we should make failures on polling more silent or at least distinguish them from first load (which would cause the comments not showing in the first place9?... Paging @pedroms.

I'm also on Firefox 76 now and cannot reproduce anymore with the steps above.

Hmm, I can't seem to reproduce anymore either on Chrome 81. Fixed by accident ?

I wonder if we should make failures on polling more silent or at least distinguish them from first load (which would cause the comments not showing in the first place9?

@andr3 agreed. And the alert should be at the top of the comments area, not affecting the whole MR page in my opinion.

added priority3 severity4 labels

changed milestone to %13.3

added [deprecated] Accepting merge requests label

Am seeing this again for !32393 (merged). Chrome 81

I can't reproduce on your MR, but now I can reproduce with an MR of mine: !32413 (merged). Same steps above.

Does this only happen with MRs with running pipelines?

Hmm, I can't reproduce now on !32393 (merged) even when I run a new pipeline. Hard to say what's going on!

Yeah, I can't reproduce there either. Can you try !32282 (merged)?

I just started a new pipeline here and then the problem started happening. I noticed that it only works if I switch back to the tab after the favicon changes to the pipeline running icon.

Also, no related errors in the console tab, and the network tab seems okay too:

So I'm not sure where the error is coming from.

Can you try !32282 (merged)?

Nope, no warning for me there...

That's weird, here's me doing it again:

Screen_Recording_2020-05-18_at_6.19.41_PM

This usually happens to me when my laptop wakes from sleep and I have an issue or MR page open. It's very annoying…

It must be one of these two locations:

$ gg -n "Something went wrong while fetching latest comments."
app/assets/javascripts/notes/stores/actions.js:408:    .catch(() => Flash(__('Something went wrong while fetching latest comments.')));
app/assets/javascripts/notes/stores/actions.js:421:    errorCallback: () => Flash(__('Something went wrong while fetching latest comments.')),

Interesting that we actually have code that starts / stops polling depending on tab visibility. I didn't know we did this:

https://gitlab.com/gitlab-org/gitlab/blob/57f74d464d2e3267ba0cd035fce3a7afeb662001/app/assets/javascripts/notes/stores/actions.js#L430-436

  Visibility.change(() => {
    if (!Visibility.hidden()) {
      eTagPoll.restart();
    } else {
      eTagPoll.stop();
    }
  });

Then that poller has an error callback that sets this message: https://gitlab.com/gitlab-org/gitlab/blob/57f74d464d2e3267ba0cd035fce3a7afeb662001/app/assets/javascripts/notes/stores/actions.js#L421

@engwan I think I've seen this appear on issues as well - can you confirm? Also, do we have any leads on a potential fix?

@danielgruesso @jramsay Considering this happens to a significant number (all?) of users, can we please bump up the P/S labels?

I think I've seen this appear on issues as well - can you confirm? Also, do we have any leads on a potential fix?

I see this in issues sometimes but I cannot reproduce. For MRs, I can reproduce sometimes for a specific MR, but after a while it doesn't happen anymore. Like if I did the same thing to the MRs I linked above, I don't see the problem anymore.

Since this is related to visibility changes, this is very likely to be related to the code in https://gitlab.com/gitlab-org/gitlab/blob/5cb62a09910a6e0434d6f4ae496cee633add01e9/app/assets/javascripts/notes/stores/actions.js#L436-462

I'm not sure how the error callback ended up being called there even without any requests with errors but there are only 2 places where this specific error message is used and both of them are triggered from this block of code so this must be where the problem is.

I also saw a commit around the same time that this started happening: 65c4eabf from !25867 (merged)

This was deployed to production on March 10 and I created this issue on March 13.

In that MR, the requests here were changed to use Axios. But I'm not sure how that would cause the problem.

Maybe @kushalpandya might know more?

I have seen this happen intermittently on some MRs and my suspicion is that it is aggressive tab suspension which kicks in the moment you switch to different tab before previous tab is done loading fully. Right now the logic behind showing banner is generic and will execute for any reason that caused request to fail/cancel. I think an ideal fix for this is to determine whether tab is in foreground before showing any errors regardless of how request was treated.

@engwan @kushalpandya I just had this happen to me right now when I loaded an issue. In this case I opened the issue by middle clicking the back button of my Chrome browser to get back to this issue in a new tab.

@kushalpandya maybe also adding a retry of some kind?

Also, why not handle the comments error and show it in the discussion area with a Retry button? That way it can be styled with a visual attention level proportional to the error.

@andr3 created #217945 for that.

mentioned in issue #212811 (closed)

Closing this as a duplicate. Root cause was found in #212811 (comment 374107184)

marked this issue as a duplicate of #212811 (closed)

closed

mentioned in issue gitlab-com/Product#1442 (closed)