[MR Widget Eng] - Brainstorm how to Handle successful status code like 204 - No Content - Causes Exception otherwise

Context

The MR widget extension has the ability to poll when setting enablePolling. It will poll until the underlying AJAX request returns a successful status code.

Problem

There are many successful status codes. 204 - No Content is one of them. It is used for many of the ~"group::secure" MR widgets. The following widgets/reports are affected by this behavior license scanning, container scanning, dependency scanning, dast, metrics, coverage fuzzing, api fuzzing. Please see #360307 (comment 925095688) for technical details.

Workflow not covered by the current MR widget extension

Initial Request -> 204 No Content Response -> Continue To Poll -> 200 OK Response -> Stop Polling/Show Data

What happens now

Initial Request -> 204 No Content Response -> Captured Exception that shows a widget Error

Possible Solutions (Proposed Patch):

diff --git a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
index efd277c1756..ca24f704850 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
+++ b/app/assets/javascripts/vue_merge_request_widget/components/extensions/base.vue
@@ -153,10 +153,14 @@ export default {
           fetchData: () => this.fetchCollapsedData(this.$props),
         },
         method: 'fetchData',
-        successCallback: ({ data }) => {
+        successCallback: ({ data = {} }) => {
+          //Would cause an exception when data was undefined when we got a 204 - No Content
           if (Object.keys(data).length > 0) {
             poll.stop();
             this.setCollapsedData(data);
+          } else {
+            // Restart polling if we have a successful status code that returns no data. 204 - No Content
+            poll.restart();
           }
         },
         errorCallback: (e) => {

Challenges when patch was applied

• Some unit tests would hang and cause the build pipeline to time out because existing unit tests that test the Loading state were not mocked out to resolve being returned empty data.

Unit tests that timed out with patch

spec/frontend/vue_mr_widget/mr_widget_options_spec.js

ee/spec/frontend/vue_mr_widget/extensions/license_compliance/index_spec.js

ee/spec/frontend/vue_mr_widget/extensions/metrics/index_spec.js

Unit test solution for metrics

diff --git a/ee/spec/frontend/vue_mr_widget/extensions/metrics/index_spec.js b/ee/spec/frontend/vue_mr_widget/extensions/metrics/index_spec.js
index b53e991b881..8cf0d2475c2 100644
--- a/ee/spec/frontend/vue_mr_widget/extensions/metrics/index_spec.js
+++ b/ee/spec/frontend/vue_mr_widget/extensions/metrics/index_spec.js
@@ -61,6 +61,10 @@ describe('Metrics extension', () => {
       createComponent();
 
       expect(wrapper.text()).toBe('Metrics reports are loading');
+
+      // base.vue polls until we have data to display
+      mock.restore();
+      mockApi(httpStatusCodes.OK, { existing_metrics: [changedMetric, changedMetric] });
     });
 
     it('displays failed loading text', async () => {

Regression testing needed for all widgets that use enablePolling

• app/assets/javascripts/vue_merge_request_widget/components/extensions/index.js
• app/assets/javascripts/vue_merge_request_widget/extensions/accessibility/index.js
• app/assets/javascripts/vue_merge_request_widget/extensions/terraform/index.js
• app/assets/javascripts/vue_merge_request_widget/extensions/test_report/index.js
• ee/app/assets/javascripts/vue_merge_request_widget/extensions/metrics/index.js
• ee/app/assets/javascripts/vue_merge_request_widget/extensions/license_compliance/index.js

---

Additional UI/UX States Not covered by MR widget Extension

Needs UX feedback

The current assumption is that the MR widgets have 3 states. Loading and Loaded w/ data and Error. The ~"group::secure" widgets actually have 4 states. Loading, Loading - Parsing Report and Loaded w/ data and Error

It's subtle but there is a difference. Loading, Loading - Parsing Report

Loading

The default state we show when the page loads, because we have no idea what the server will say. We may have the data available after initial load, we may error out, or we may still be waiting for results. We don't know at this point.

Example:

Loaded - Parsing Report

This state exists AFTER a pipeline is successful, but BEFORE the reports are parsed. ~"group::secure" is unique in that we have post pipeline background jobs that run to parse pipeline artifacts to generate our vulnerabilities and and other Security related data for the UI.

Example:

Non existent. What we do for the original Security widgets is show a very general loading message "Loading Report". This will show during the entire duration of the pipeline run, and even after a pipeline completes. This has caused users to think the widget is broken in some cases. See: License Compliance Does Not work

Why we need a Loading - Doing something else state

If we implement the patch as is to get around the widget failing we still end up with a broken UX flow. This is what we get.

Loading -> No Licenses Detected ->(1 second polling delay) Licenses detected

This makes the status jump from No licenses to Licenses detected after the initial widget load.

Loading

Note: Even though the description seems pretty clear, this is our Loading message. We don't distinguish between initial Load and Loaded - But still waiting for pipeline completion and parsing. Right now if a pipeline took 2 hours, it would show loading for 2 hours.

No Licenses Detected

Licenses detected (~1 Second Later after polling again)