Merge branch 'fix-job-trace-polling-bug' into 'master'

Fix job trace polling bug

See merge request gitlab-org/gitlab!48516

app/assets/javascripts/jobs/store/actions.js

@@ -20,8 +20,7 @@ export const init = ({ dispatch }, { endpoint, logState, pagePath }) => {
     logState,
     pagePath,
   });
-
-  return Promise.all([dispatch('fetchJob'), dispatch('fetchTrace')]);
+  dispatch('fetchJob');
 };
 
 export const setJobEndpoint = ({ commit }, endpoint) => commit(types.SET_JOB_ENDPOINT, endpoint);
@@ -39,6 +38,7 @@ export const toggleSidebar = ({ dispatch, state }) => {
 };
 
 let eTagPoll;
+let isTraceReadyForRender;
 
 export const clearEtagPoll = () => {
   eTagPoll = null;
@@ -70,7 +70,14 @@ export const fetchJob = ({ state, dispatch }) => {
   });
 
   if (!Visibility.hidden()) {
-    eTagPoll.makeRequest();
+    // eslint-disable-next-line promise/catch-or-return
+    eTagPoll.makeRequest().then(() => {
+      // if a job is canceled we still need to dispatch
+      // fetchTrace to get the trace so we check for has_trace
+      if (state.job.started || state.job.has_trace) {
+        dispatch('fetchTrace');
+      }
+    });
   } else {
     axios
       .get(state.jobEndpoint)
@@ -80,9 +87,15 @@ export const fetchJob = ({ state, dispatch }) => {
 
   Visibility.change(() => {
     if (!Visibility.hidden()) {
+      // This check is needed to ensure the loading icon
+      // is not shown for a finished job during a visibility change
+      if (!isTraceReadyForRender) {
+        dispatch('startPollingTrace');
+      }
       dispatch('restartPolling');
     } else {
       dispatch('stopPolling');
+      dispatch('stopPollingTrace');
     }
   });
 };
@@ -163,6 +176,8 @@ export const fetchTrace = ({ dispatch, state }) =>
       params: { state: state.traceState },
     })
     .then(({ data }) => {
+      isTraceReadyForRender = data.complete;
+
       dispatch('toggleScrollisInBottom', isScrolledToBottom());
       dispatch('receiveTraceSuccess', data);
 

app/assets/javascripts/jobs/store/mutations.js

@@ -49,6 +49,7 @@ export default {
 
   [types.SET_TRACE_TIMEOUT](state, id) {
     state.traceTimeout = id;
+    state.isTraceComplete = false;
   },
 
   /**

changelogs/unreleased/fix-job-trace-polling-bug.yml

+---
+title: Ensure job trace endpoint is not called if the current job has not started or the browser is not visible
+merge_request: 48516
+author:
+type: fixed

spec/frontend/jobs/store/actions_spec.js

@@ -158,6 +158,32 @@ describe('Job State actions', () => {
         );
       });
     });
+
+    it('fetchTrace is called only if the job has started or has a trace', done => {
+      mock.onGet(`${TEST_HOST}/endpoint.json`).replyOnce(200, { id: 121212, name: 'karma' });
+
+      mockedState.job.started = true;
+
+      testAction(
+        fetchJob,
+        null,
+        mockedState,
+        [],
+        [
+          {
+            type: 'requestJob',
+          },
+          {
+            payload: { id: 121212, name: 'karma' },
+            type: 'receiveJobSuccess',
+          },
+          {
+            type: 'fetchTrace',
+          },
+        ],
+        done,
+      );
+    });
   });
 
   describe('receiveJobSuccess', () => {

spec/frontend/jobs/store/mutations_spec.js

@@ -153,6 +153,7 @@ describe('Jobs Store Mutations', () => {
       mutations[types.SET_TRACE_TIMEOUT](stateCopy, id);
 
       expect(stateCopy.traceTimeout).toEqual(id);
+      expect(stateCopy.isTraceComplete).toBe(false);
     });
   });