fix(create-recordings): add error-views and refactor target listener (#…

…687)

* fix(custom-recordings): add error-view & refactor target selection listener

* fix(snapshot-recording): add error view

* fix(create-recording): use forkJoin

src/app/CreateRecording/CustomRecordingForm.tsx

@@ -60,7 +60,7 @@ import {
 } from '@patternfly/react-core';
 import { HelpIcon } from '@patternfly/react-icons';
 import { useHistory } from 'react-router-dom';
-import { concatMap, first } from 'rxjs/operators';
+import { concatMap, first, filter } from 'rxjs/operators';
 import { EventTemplate } from './CreateRecording';
 import { RecordingOptions, RecordingAttributes } from '@app/Shared/Services/Api.service';
 import { DurationPicker } from '@app/DurationPicker/DurationPicker';
@@ -70,6 +70,9 @@ import { RecordingLabelFields } from '@app/RecordingMetadata/RecordingLabelField
 import { useSubscriptions } from '@app/utils/useSubscriptions';
 import { LoadingPropsType } from '@app/Shared/ProgressIndicator';
 import { TemplateType } from '@app/Shared/Services/Api.service';
+import { authFailMessage, ErrorView, isAuthFail } from '@app/ErrorView/ErrorView';
+import { NO_TARGET } from '@app/Shared/Services/Target.service';
+import { forkJoin } from 'rxjs';
 
 export interface CustomRecordingFormProps {
   prefilled?: {
@@ -105,6 +108,7 @@ export const CustomRecordingForm: React.FunctionComponent<CustomRecordingFormPro
   const [labels, setLabels] = React.useState([] as RecordingLabel[]);
   const [labelsValid, setLabelsValid] = React.useState(ValidatedOptions.default);
   const [loading, setLoading] = React.useState(false);
+  const [errorMessage, setErrorMessage] = React.useState('');
 
   const handleCreateRecording = React.useCallback(
     (recordingAttributes: RecordingAttributes) => {
@@ -277,31 +281,52 @@ export const CustomRecordingForm: React.FunctionComponent<CustomRecordingFormPro
     handleCreateRecording,
   ]);
 
-  React.useEffect(() => {
+  const refeshFormOptions = React.useCallback(() => {
     addSubscription(
       context.target
         .target()
         .pipe(
+          filter((target) => target !== NO_TARGET),
+          first(),
           concatMap((target) =>
-            context.api.doGet<EventTemplate[]>(`targets/${encodeURIComponent(target.connectUrl)}/templates`)
+            forkJoin({
+              templates: context.api.doGet<EventTemplate[]>(
+                `targets/${encodeURIComponent(target.connectUrl)}/templates`
+              ),
+              recordingOptions: context.api.doGet<RecordingOptions>(
+                `targets/${encodeURIComponent(target.connectUrl)}/recordingOptions`
+              ),
+            })
           )
         )
-        .subscribe(setTemplates)
+        .subscribe({
+          next: (formOptions) => {
+            setErrorMessage('');
+            setTemplates(formOptions.templates);
+            setRecordingOptions(formOptions.recordingOptions);
+          },
+          error: (error) => {
+            setErrorMessage(error.message); // If both throw, first error will be shown
+            setTemplates([]);
+            setRecordingOptions({});
+          },
+        })
     );
-  }, [addSubscription, context.target, context.api, setTemplates]);
+  }, [addSubscription, context.target, context.api, setTemplates, setRecordingOptions, setErrorMessage]);
 
   React.useEffect(() => {
     addSubscription(
-      context.target
-        .target()
-        .pipe(
-          concatMap((target) =>
-            context.api.doGet<RecordingOptions>(`targets/${encodeURIComponent(target.connectUrl)}/recordingOptions`)
-          )
-        )
-        .subscribe(setRecordingOptions)
+      context.target.authFailure().subscribe(() => {
+        setErrorMessage(authFailMessage);
+        setTemplates([]);
+        setRecordingOptions({});
+      })
     );
-  }, [addSubscription, context.api, context.target, setRecordingOptions]);
+  }, [context.target, setErrorMessage, addSubscription, setTemplates, setRecordingOptions]);
+
+  React.useEffect(() => {
+    addSubscription(context.target.target().subscribe(refeshFormOptions));
+  }, [addSubscription, context.target, refeshFormOptions]);
 
   const isFormInvalid: boolean = React.useMemo(() => {
     return (
@@ -335,6 +360,19 @@ export const CustomRecordingForm: React.FunctionComponent<CustomRecordingFormPro
     return '';
   }, [templateName, templateType]);
 
+  const authRetry = React.useCallback(() => {
+    context.target.setAuthRetry();
+  }, [context.target]);
+
+  if (errorMessage != '') {
+    return (
+      <ErrorView
+        title={'Error displaying recording creation form'}
+        message={errorMessage}
+        retry={isAuthFail(errorMessage) ? authRetry : undefined}
+      />
+    );
+  }
   return (
     <>
       <Text component={TextVariants.small}>

src/app/CreateRecording/SnapshotRecordingForm.tsx

@@ -42,6 +42,7 @@ import { useSubscriptions } from '@app/utils/useSubscriptions';
 import { ServiceContext } from '@app/Shared/Services/Services';
 import { first } from 'rxjs';
 import { LoadingPropsType } from '@app/Shared/ProgressIndicator';
+import { authFailMessage, ErrorView, isAuthFail, missingSSLMessage } from '@app/ErrorView/ErrorView';
 
 export interface SnapshotRecordingFormProps {}
 
@@ -50,6 +51,7 @@ export const SnapshotRecordingForm: React.FunctionComponent<SnapshotRecordingFor
   const addSubscription = useSubscriptions();
   const context = React.useContext(ServiceContext);
   const [loading, setLoading] = React.useState(false);
+  const [errorMessage, setErrorMessage] = React.useState('');
 
   const handleCreateSnapshot = React.useCallback(() => {
     setLoading(true);
@@ -76,6 +78,53 @@ export const SnapshotRecordingForm: React.FunctionComponent<SnapshotRecordingFor
     [loading]
   );
 
+  React.useEffect(() => {
+    addSubscription(
+      context.target.sslFailure().subscribe(() => {
+        // also triggered if api calls in Custom Recording form fail
+        setErrorMessage(missingSSLMessage);
+      })
+    );
+  }, [context.target, setErrorMessage, addSubscription]);
+
+  React.useEffect(() => {
+    addSubscription(
+      context.target.authRetry().subscribe(() => {
+        setErrorMessage(''); // Reset on retry
+      })
+    );
+  }, [context.target, setErrorMessage, addSubscription]);
+
+  React.useEffect(() => {
+    addSubscription(
+      context.target.authFailure().subscribe(() => {
+        // also triggered if api calls in Custom Recording form fail
+        setErrorMessage(authFailMessage);
+      })
+    );
+  }, [context.target, setErrorMessage, addSubscription]);
+
+  React.useEffect(() => {
+    addSubscription(
+      context.target.target().subscribe(() => {
+        setErrorMessage(''); // Reset on change
+      })
+    );
+  }, [context.target, setErrorMessage, addSubscription]);
+
+  const authRetry = React.useCallback(() => {
+    context.target.setAuthRetry();
+  }, [context.target]);
+
+  if (errorMessage != '') {
+    return (
+      <ErrorView
+        title={'Error displaying recording creation form'}
+        message={errorMessage}
+        retry={isAuthFail(errorMessage) ? authRetry : undefined}
+      />
+    );
+  }
   return (
     <>
       <Form isHorizontal>

src/app/ErrorView/ErrorView.tsx

@@ -49,6 +49,9 @@ import {
 import { ExclamationCircleIcon } from '@patternfly/react-icons';
 
 export const authFailMessage = 'Auth failure';
+
+export const missingSSLMessage = 'Bad Gateway';
+
 export const isAuthFail = (message: string) => message === authFailMessage;
 export interface ErrorViewProps {
   title: string | React.ReactNode;

src/test/CreateRecording/CustomRecordingForm.test.tsx

@@ -38,12 +38,12 @@
 
 import * as React from 'react';
 import { createMemoryHistory } from 'history';
-import { screen, cleanup, waitFor } from '@testing-library/react';
+import { screen, cleanup, act as doAct } from '@testing-library/react';
 import renderer, { act } from 'react-test-renderer';
-import { ServiceContext } from '@app/Shared/Services/Services';
+import { ServiceContext, Services } from '@app/Shared/Services/Services';
 import { defaultServices } from '@app/Shared/Services/Services';
 import { EventTemplate, RecordingAttributes, RecordingOptions } from '@app/Shared/Services/Api.service';
-import { of } from 'rxjs';
+import { of, Subject } from 'rxjs';
 import { renderWithServiceContext } from '../Common';
 
 jest.mock('@patternfly/react-core', () => ({
@@ -54,6 +54,7 @@ jest.mock('@patternfly/react-core', () => ({
 
 import { CustomRecordingForm } from '@app/CreateRecording/CustomRecordingForm';
 import { NotificationsContext, NotificationsInstance } from '@app/Notifications/Notifications';
+import { TargetService } from '@app/Shared/Services/Target.service';
 
 const mockConnectUrl = 'service:jmx:rmi://someUrl';
 const mockTarget = { connectUrl: mockConnectUrl, alias: 'fooTarget' };
@@ -83,6 +84,8 @@ jest
   .mockReturnValueOnce(of([mockCustomEventTemplate])) // should show correct helper texts in metadata label editor
   .mockReturnValueOnce(of(mockRecordingOptions));
 
+jest.spyOn(defaultServices.target, 'authFailure').mockReturnValue(of());
+
 const history = createMemoryHistory({ initialEntries: ['/recordings/create'] });
 
 jest.mock('react-router-dom', () => ({
@@ -162,4 +165,31 @@ describe('<CustomRecordingForm />', () => {
     expect(helperText).toBeInTheDocument();
     expect(helperText).toBeVisible();
   });
+
+  it('should show error view if failing to retrieve templates or recording options', async () => {
+    const subj = new Subject<void>();
+    const mockTargetSvc = {
+      target: () => of(mockTarget),
+      authFailure: () => subj.asObservable(),
+    } as TargetService;
+    const services: Services = {
+      ...defaultServices,
+      target: mockTargetSvc,
+    };
+    renderWithServiceContext(<CustomRecordingForm />, { services: services });
+
+    await doAct(async () => subj.next());
+
+    const failTitle = screen.getByText('Error displaying recording creation form');
+    expect(failTitle).toBeInTheDocument();
+    expect(failTitle).toBeVisible();
+
+    const authFailText = screen.getByText('Auth failure');
+    expect(authFailText).toBeInTheDocument();
+    expect(authFailText).toBeVisible();
+
+    const retryButton = screen.getByText('Retry');
+    expect(retryButton).toBeInTheDocument();
+    expect(retryButton).toBeVisible();
+  });
 });

src/test/CreateRecording/SnapshotRecordingForm.test.tsx

@@ -0,0 +1,128 @@
+/*
+ * Copyright The Cryostat Authors
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or data
+ * (collectively the "Software"), free of charge and under any and all copyright
+ * rights in the Software, and any and all patent rights owned or freely
+ * licensable by each licensor hereunder covering either (i) the unmodified
+ * Software as contributed to or provided by such licensor, or (ii) the Larger
+ * Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software (each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ * The above copyright notice and either this complete permission notice or at
+ * a minimum a reference to the UPL must be included in all copies or
+ * substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+import * as React from 'react';
+import { createMemoryHistory } from 'history';
+import { screen, cleanup, act as doAct } from '@testing-library/react';
+import renderer, { act } from 'react-test-renderer';
+import { ServiceContext, Services } from '@app/Shared/Services/Services';
+import { defaultServices } from '@app/Shared/Services/Services';
+import { of, Subject } from 'rxjs';
+import { renderWithServiceContext } from '../Common';
+import { NotificationsContext, NotificationsInstance } from '@app/Notifications/Notifications';
+import { TargetService } from '@app/Shared/Services/Target.service';
+import { SnapshotRecordingForm } from '@app/CreateRecording/SnapshotRecordingForm';
+
+const mockConnectUrl = 'service:jmx:rmi://someUrl';
+const mockTarget = { connectUrl: mockConnectUrl, alias: 'fooTarget' };
+
+const history = createMemoryHistory({ initialEntries: ['/recordings/create'] });
+
+jest.mock('react-router-dom', () => ({
+  ...jest.requireActual('react-router-dom'),
+  useRouteMatch: () => ({ url: history.location.pathname }),
+  useHistory: () => history,
+}));
+
+jest.spyOn(defaultServices.target, 'authFailure').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'target').mockReturnValue(of(mockTarget));
+jest.spyOn(defaultServices.target, 'sslFailure').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'authRetry').mockReturnValue(of());
+
+describe('<SnapshotRecordingForm />', () => {
+  beforeEach(() => {
+    history.go(-history.length);
+  });
+
+  afterEach(cleanup);
+
+  it('renders correctly', async () => {
+    let tree;
+    await act(async () => {
+      tree = renderer.create(
+        <ServiceContext.Provider value={defaultServices}>
+          <NotificationsContext.Provider value={NotificationsInstance}>
+            <SnapshotRecordingForm />
+          </NotificationsContext.Provider>
+        </ServiceContext.Provider>
+      );
+    });
+    expect(tree.toJSON()).toMatchSnapshot();
+  });
+
+  it('should create recording when create is clicked', async () => {
+    const onCreateSpy = jest.spyOn(defaultServices.api, 'createSnapshot').mockReturnValue(of(true));
+    const { user } = renderWithServiceContext(<SnapshotRecordingForm />);
+
+    const createButton = screen.getByText('Create');
+    expect(createButton).toBeInTheDocument();
+    expect(createButton).toBeVisible();
+
+    await user.click(createButton);
+
+    expect(onCreateSpy).toHaveBeenCalledTimes(1);
+    expect(history.entries.map((entry) => entry.pathname)).toStrictEqual(['/recordings/create', '/recordings']);
+  });
+
+  it('should show error view if failing to retrieve templates or recording options', async () => {
+    const authSubj = new Subject<void>();
+    const mockTargetSvc = {
+      ...defaultServices.target,
+      authFailure: () => authSubj.asObservable(),
+    } as TargetService;
+    const services: Services = {
+      ...defaultServices,
+      target: mockTargetSvc,
+    };
+    renderWithServiceContext(<SnapshotRecordingForm />, { services: services });
+
+    await doAct(async () => authSubj.next());
+
+    const failTitle = screen.getByText('Error displaying recording creation form');
+    expect(failTitle).toBeInTheDocument();
+    expect(failTitle).toBeVisible();
+
+    const authFailText = screen.getByText('Auth failure');
+    expect(authFailText).toBeInTheDocument();
+    expect(authFailText).toBeVisible();
+
+    const retryButton = screen.getByText('Retry');
+    expect(retryButton).toBeInTheDocument();
+    expect(retryButton).toBeVisible();
+  });
+});