Merge branch '334116-calm-down-the-code-quality-diff-error-messages' into 'master'

Reduce noisy code quality diff errors on MR page

See merge request gitlab-org/gitlab!64661

app/controllers/projects/merge_requests_controller.rb

@@ -220,7 +220,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   end
 
   def codequality_mr_diff_reports
-    reports_response(@merge_request.find_codequality_mr_diff_reports)
+    reports_response(@merge_request.find_codequality_mr_diff_reports, head_pipeline)
   end
 
   def codequality_reports

ee/app/assets/javascripts/diffs/store/actions.js

@@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils';
 import httpStatusCodes from '~/lib/utils/http_status';
 import Poll from '~/lib/utils/poll';
 import { __ } from '~/locale';
+import { MAX_RETRIES, RETRY_DELAY } from './constants';
 
 import * as types from './mutation_types';
 
@@ -28,6 +29,8 @@ export const restartCodequalityPolling = () => {
 };
 
 export const fetchCodequality = ({ commit, state, dispatch }) => {
+  let retryCount = 0;
+
   codequalityPoll = new Poll({
     resource: {
       getCodequalityDiffReports: (endpoint) => axios.get(endpoint),
@@ -35,16 +38,32 @@ export const fetchCodequality = ({ commit, state, dispatch }) => {
     data: state.endpointCodequality,
     method: 'getCodequalityDiffReports',
     successCallback: ({ status, data }) => {
+      retryCount = 0;
       if (status === httpStatusCodes.OK) {
         commit(types.SET_CODEQUALITY_DATA, data);
 
         dispatch('stopCodequalityPolling');
       }
     },
-    errorCallback: () =>
-      createFlash({
-        message: __('Something went wrong on our end while loading the code quality diff.'),
-      }),
+    errorCallback: ({ response }) => {
+      if (response.status === httpStatusCodes.BAD_REQUEST) {
+        // we could get a 400 status response temporarily during report processing
+        // so we retry up to MAX_RETRIES times in case the reports are on their way
+        // and stop polling if we get 400s consistently
+        if (retryCount < MAX_RETRIES) {
+          codequalityPoll.makeDelayedRequest(RETRY_DELAY);
+          retryCount += 1;
+        } else {
+          codequalityPoll.stop();
+        }
+      } else {
+        retryCount = 0;
+        dispatch('stopCodequalityPolling');
+        createFlash({
+          message: __('An unexpected error occurred while loading the code quality diff.'),
+        });
+      }
+    },
   });
 
   if (!Visibility.hidden()) {

ee/app/assets/javascripts/diffs/store/constants.js

+export const MAX_RETRIES = 5;
+export const RETRY_DELAY = 3000; // milliseconds

ee/spec/frontend/diffs/store/actions_spec.js

@@ -5,10 +5,13 @@ import {
   stopCodequalityPolling,
   fetchCodequality,
 } from 'ee/diffs/store/actions';
+import { RETRY_DELAY } from 'ee/diffs/store/constants';
 import * as types from 'ee/diffs/store/mutation_types';
 import testAction from 'helpers/vuex_action_helper';
+import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
 import axios from '~/lib/utils/axios_utils';
+import Poll from '~/lib/utils/poll';
 
 jest.mock('~/flash');
 
@@ -30,7 +33,7 @@ describe('EE DiffsStoreActions', () => {
 
   describe('fetchCodequality', () => {
     let mock;
-    const endpoint = '/codequality_mr_diff.json';
+    const endpointCodequality = '/codequality_mr_diff.json';
 
     beforeEach(() => {
       mock = new MockAdapter(axios);
@@ -46,28 +49,86 @@ describe('EE DiffsStoreActions', () => {
         files: { 'app.js': [{ line: 1, description: 'Unexpected alert.', severity: 'minor' }] },
       };
 
-      mock.onGet(endpoint).reply(200, { data });
+      mock.onGet(endpointCodequality).reply(200, { data });
 
       testAction(
         fetchCodequality,
         {},
-        { endpointCodequality: endpoint },
+        { endpointCodequality },
         [{ type: types.SET_CODEQUALITY_DATA, payload: { data } }],
         [{ type: 'stopCodequalityPolling' }],
         done,
       );
     });
 
-    it('should show flash on API error', (done) => {
-      mock.onGet(endpoint).reply(400);
+    describe('with 400 status response', () => {
+      let pollDelayedRequest;
+      let pollStop;
 
-      testAction(fetchCodequality, {}, { endpoint }, [], [], () => {
-        expect(createFlash).toHaveBeenCalledTimes(1);
-        expect(createFlash).toHaveBeenCalledWith({
-          message: 'Something went wrong on our end while loading the code quality diff.',
+      beforeEach(() => {
+        pollDelayedRequest = jest.spyOn(Poll.prototype, 'makeDelayedRequest');
+        pollStop = jest.spyOn(Poll.prototype, 'stop');
+
+        mock.onGet(endpointCodequality).reply(400);
+      });
+
+      it('should not show a flash message', (done) => {
+        testAction(fetchCodequality, {}, { endpointCodequality }, [], [], () => {
+          expect(createFlash).not.toHaveBeenCalled();
+          done();
+        });
+      });
+
+      it('should retry five times with a delay, then stop polling', (done) => {
+        testAction(fetchCodequality, {}, { endpointCodequality }, [], [], () => {
+          expect(pollDelayedRequest).toHaveBeenCalledTimes(1);
+          expect(pollStop).toHaveBeenCalledTimes(0);
+
+          jest.advanceTimersByTime(RETRY_DELAY);
+
+          waitForPromises()
+            .then(() => {
+              expect(pollDelayedRequest).toHaveBeenCalledTimes(2);
+
+              jest.advanceTimersByTime(RETRY_DELAY);
+            })
+            .then(() => waitForPromises())
+            .then(() => jest.advanceTimersByTime(RETRY_DELAY))
+            .then(() => waitForPromises())
+            .then(() => jest.advanceTimersByTime(RETRY_DELAY))
+            .then(() => waitForPromises())
+            .then(() => {
+              expect(pollDelayedRequest).toHaveBeenCalledTimes(5);
+
+              jest.advanceTimersByTime(RETRY_DELAY);
+            })
+            .then(() => waitForPromises())
+            .then(() => {
+              expect(pollStop).toHaveBeenCalledTimes(1);
+            })
+            .then(done)
+            .catch(done.fail);
         });
-        done();
       });
     });
+
+    it('with unexpected error should stop polling and show a flash message', (done) => {
+      mock.onGet(endpointCodequality).reply(500);
+
+      testAction(
+        fetchCodequality,
+        {},
+        { endpointCodequality },
+        [],
+        [{ type: 'stopCodequalityPolling' }],
+        () => {
+          expect(createFlash).toHaveBeenCalledTimes(1);
+          expect(createFlash).toHaveBeenCalledWith({
+            message: 'An unexpected error occurred while loading the code quality diff.',
+          });
+          done();
+        },
+      );
+    });
   });
 });

locale/gitlab.pot

@@ -3852,6 +3852,9 @@ msgstr ""
 msgid "An unexpected error occurred while communicating with the Web Terminal."
 msgstr ""
 
+msgid "An unexpected error occurred while loading the code quality diff."
+msgstr ""
+
 msgid "An unexpected error occurred while starting the Web Terminal."
 msgstr ""
 
@@ -30297,9 +30300,6 @@ msgstr ""
 msgid "Something went wrong on our end"
 msgstr ""
 
-msgid "Something went wrong on our end while loading the code quality diff."
-msgstr ""
-
 msgid "Something went wrong on our end."
 msgstr ""