Error: Failed to fetch

This error sometimes floods our Sentry, bringing it down

https://sentry.gitlab.net/gitlab/gitlabcom-clientside/issues/3390527/?referrer=gitlab_plugin

Error: Failed to fetch
  at new t (/assets/webpack/graphql.7a7c3f9d.chunk.js:1:93432)
  at None (/assets/webpack/graphql.7a7c3f9d.chunk.js:1:196768)
  at r (/assets/webpack/graphql.7a7c3f9d.chunk.js:1:172443)
  at None (/assets/webpack/graphql.7a7c3f9d.chunk.js:1:172368)
  at new Promise (<anonymous>)
...
(37 additional frame(s) were not displayed)

  1 day ago
@ntepluhina
Error sending the query 'state' TypeError: Failed to fetch Do we have any query named state?
  1 day ago
@lduncalfe
we do but it’s @client one :thinking_face:
  1 day ago
ee/app/assets/javascripts/subscriptions/graphql/queries/state.query.graphql (edited) 
  1 day ago
@ntepluhina
Tracked the stacktrace down to: https://github.com/apollographql/apollo-client/blob/main/src/link/batch-http/batchHttpLink.ts#L146-L186
  1 day ago
Traveling up the stacktrace again to see if it actually has the “source” of the error.
  1 day ago
@ntepluhina
I think it is not that client query. The error is triggered on the MR/diff page it seems.
  1 day ago
@leipert
that was only an answer about state query, not the answer about the reason of the actual error
  1 day ago
This error for event for example: https://sentry.gitlab.net/gitlab/gitlabcom-clientside/issues/3390527/events/122427870 was triggered by this user: https://gitlab.com/clefelhocz1 :smile: On one of our MRs: https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_requests/115006/diffsLooking at the sentry log, it seems like multiple graphql queries went fine and then after some time, some failed. Seems maybe related to some polling? (edited) 
  1 day ago
We also seem to poll a query getState
  1 day ago
this one? app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql
  1 day ago
I’ll look into commit history to mr_widget
  1 day ago
The error was seen the first time 4 months ago
  1 day ago
My gut feeling is that the query errors for some reason (obv I know) (maybe browser tab in background, or something else) and that error is not handled well. (edited) 
  1 day ago
@leipert
ideally yes, we would have a source maps, https://docs.sentry.io/platforms/javascript/sourcemaps/
  1 day ago
Maybe it is worthwhile for someone in Chrome to leave a tab open for some time, with the dev tools and we can catch when that error happened. I have it open in FF, but maybe Chrome does something “new” since ~4 months ago.
  1 day ago
Seems like you must click away from the diffs tab and to the overview tab in order to trigger the polling
  1 day ago
Seems like we have another issue with ~17M that is similar but on Safari: https://sentry.gitlab.net/gitlab/gitlabcom-clientside/issues/3390921/
  1 day ago
@ntepluhina
I can reproduce
  1 day ago
Whats also weird, that after some time the readyToMergeEE query goes nuts and happens like every second.
  1 day ago
Okay my findings:

    app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue can start multiple pollings in parallel (initPolling doesn’t check if there is already a polling started)
    Apollo does not seem to catch certain errors properly. This can be reproduced with:

    Going to any MR page. Make sure the polling starts
    Block the /graphql url in the network tab
    See JS errors piling up in the console.

(I don’t know if 2. is an apollo problem or a priblem of the ready_to_merge vue component) (edited) 
  1 day ago
(Hope this information helps.
@ntepluhina
Are you going to dig into it? Or should we move it to an issue? I have to prep some Pajamas Migration day things for this Friday)
  1 day ago
fyi we had a discussion about polling when offline at https://gitlab.slack.com/archives/C0GQHHPGW/p1669629537050279?thread_ts=1669361359.167999&cid=C0GQHHPGW
  1 day ago
app/assets/javascripts/smart_interval.js is supposed to at least adhere to visibility stuff. Network connectivity might be another great feature (maybe smarter_interval.js :laughing:)
  1 day ago

> opens smart_interval.js in a new tab
> sees first line is a jquery import
> proceeds to close the tab

(edited)
  1 day ago
@Miguel
Doing it wrong:

> opens smart_interval.js in a new tab
> sees first line is a jquery import
> opens MR to remove jQuery usage

  1 day ago
actually that jquery removal is super easy
  1 day ago
https://xkcd.com/356/
xkcdxkcd
Nerd Sniping
[Title text] "I first saw this problem on the Google Labs Aptitude Test.  A professor and I filled a blackboard without getting anywhere.  Have fun." (103 kB)
https://xkcd.com/356/
  1 day ago
@leipert
I’m on the meeting right now but will take a look after it
  1 day ago
@leipert
for 2: confirmed it’s Apollo issue (please see this handy sandbox for reproduction). I wouldn’t say though that it’s not “caught” by Apollo, I can see it triggers the query error hook
  1 day ago
Second interesting finding: if we get rid of our refetch  logic and start using Apollo’s own pollInterval, the error hook intercepts the error completely
Here goes the second handy sandbox: https://codesandbox.io/s/vigilant-matan-68etwf?file=/src/App.vue (edited) 
  1 day ago
So the general recommendation to fix both crazy polling AND Failed to fetch error on Sentry is to refactor component to use pollInterval instead of refetch. pollInterval accepts dynamic properties so we could implement the same complex behaviour smartInterval does for us, and unlike refetch, pollInterval will check on polling status/cc
@iamphill
  1 day ago
FYI smart_interval is buggy: We don’t remove the event listeners correctly because we do:

// init:
    window.addEventListener('blur', this.onWindowVisibilityChange.bind(this));
// destroy
    window.removeEventListener('blur', this.onWindowVisibilityChange);

the listeners don’t match because of the .bind(this) and they don’t get removed :sad:

Edited Dec 01, 2022 by Thong Kuah