[UI] Stops experiment list from leaking previous error message (#3350)

* [UI] Stops experiment list from leaking previous error message

* Move the fix to Page component so it's more generic

* Update ExperimentList.test.tsx

frontend/src/pages/ExperimentList.test.tsx

@@ -29,7 +29,8 @@ import { RoutePage, QUERY_PARAMS } from '../components/Router';
 import { range } from 'lodash';
 import { ButtonKeys } from '../lib/Buttons';
 import { NamespaceContext } from 'src/lib/KubeflowClient';
-import { render } from '@testing-library/react';
+import { render, act } from '@testing-library/react';
+import { MemoryRouter } from 'react-router-dom';
 
 // Default arguments for Apis.experimentServiceApi.listExperiment.
 const LIST_EXPERIMENT_DEFAULTS = [
@@ -71,17 +72,22 @@ describe('ExperimentList', () => {
     );
   }
 
+  function mockListNExpperiments(n: number = 1) {
+    return () =>
+      Promise.resolve({
+        experiments: range(n).map(i => ({
+          id: 'test-experiment-id' + i,
+          name: 'test experiment name' + i,
+        })),
+      });
+  }
+
   async function mountWithNExperiments(
     n: number,
     nRuns: number,
     { namespace }: { namespace?: string } = {},
   ): Promise<void> {
-    listExperimentsSpy.mockImplementation(() => ({
-      experiments: range(n).map(i => ({
-        id: 'test-experiment-id' + i,
-        name: 'test experiment name' + i,
-      })),
-    }));
+    listExperimentsSpy.mockImplementation(mockListNExpperiments(n));
     listRunsSpy.mockImplementation(() => ({
       runs: range(nRuns).map(i => ({ id: 'test-run-id' + i, name: 'test run name' + i })),
     }));
@@ -473,5 +479,29 @@ describe('ExperimentList', () => {
         'test-ns-2',
       );
     });
+
+    it("doesn't keep error message for request from previous namespace", async () => {
+      listExperimentsSpy.mockImplementation(() => Promise.reject('namespace cannot be empty'));
+      const { rerender } = render(
+        <MemoryRouter>
+          <NamespaceContext.Provider value={undefined}>
+            <EnhancedExperimentList {...generateProps()} />
+          </NamespaceContext.Provider>
+        </MemoryRouter>,
+      );
+
+      listExperimentsSpy.mockImplementation(mockListNExpperiments());
+      rerender(
+        <MemoryRouter>
+          <NamespaceContext.Provider value={'test-ns'}>
+            <EnhancedExperimentList {...generateProps()} />
+          </NamespaceContext.Provider>
+        </MemoryRouter>,
+      );
+      await act(TestUtils.flushPromises);
+      expect(updateBannerSpy).toHaveBeenLastCalledWith(
+        {}, // Empty object means banner has no error message
+      );
+    });
   });
 });

frontend/src/pages/ExperimentList.tsx

@@ -227,7 +227,7 @@ export class ExperimentList extends Page<{ namespace?: string }, ExperimentListS
       }),
     );
 
-    this.setState({ displayExperiments });
+    this.setStateSafe({ displayExperiments });
     return response.next_page_token || '';
   }
 

frontend/src/pages/Page.tsx

@@ -54,6 +54,9 @@ export abstract class Page<P, S> extends React.Component<P & PageProps, S> {
   }
 
   public clearBanner(): void {
+    if (!this._isMounted) {
+      return;
+    }
     this.props.updateBanner({});
   }
 
@@ -63,6 +66,9 @@ export abstract class Page<P, S> extends React.Component<P & PageProps, S> {
     mode?: 'error' | 'warning',
   ): Promise<void> {
     const errorMessage = await errorToMessage(error);
+    if (!this._isMounted) {
+      return;
+    }
     this.props.updateBanner({
       additionalInfo: errorMessage ? errorMessage : undefined,
       message: message + (errorMessage ? ' Click Details for more information.' : ''),
@@ -72,6 +78,9 @@ export abstract class Page<P, S> extends React.Component<P & PageProps, S> {
   }
 
   public showErrorDialog(title: string, content: string): void {
+    if (!this._isMounted) {
+      return;
+    }
     this.props.updateDialog({
       buttons: [{ text: 'Dismiss' }],
       content,