feat(graphql): improve error handling (cryostatio#1295)

Co-authored-by: Andrew Azores <[email protected]>
tthvo · Aug 7, 2024 · 9dafc5b · 9dafc5b
1 parent 508d03e
commit 9dafc5b
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 40 deletions.
diff --git a/src/app/Dashboard/AutomatedAnalysis/AutomatedAnalysisCard.tsx b/src/app/Dashboard/AutomatedAnalysis/AutomatedAnalysisCard.tsx
@@ -15,7 +15,7 @@
  */
 
 import { ErrorView } from '@app/ErrorView/ErrorView';
-import { authFailMessage, isAuthFail } from '@app/ErrorView/types';
+import { authFailMessage, isAuthFail, missingSSLMessage } from '@app/ErrorView/types';
 import { LoadingView } from '@app/Shared/Components/LoadingView';
 import {
   emptyAutomatedAnalysisFilters,
@@ -45,7 +45,7 @@ import {
   AutomatedAnalysisScore,
   AnalysisResult,
 } from '@app/Shared/Services/api.types';
-import { isGraphQLAuthError } from '@app/Shared/Services/api.utils';
+import { isGraphQLAuthError, isGraphQLError, isGraphQLSSLError } from '@app/Shared/Services/api.utils';
 import { FeatureLevel } from '@app/Shared/Services/service.types';
 import { automatedAnalysisConfigToRecordingAttributes } from '@app/Shared/Services/service.utils';
 import { ServiceContext } from '@app/Shared/Services/Services';
@@ -359,10 +359,13 @@ export const AutomatedAnalysisCard: DashboardCardFC<AutomatedAnalysisCardProps>
               .pipe(
                 first(),
                 tap((resp) => {
-                  if (resp.data == undefined) {
+                  if (isGraphQLError(resp)) {
                     if (isGraphQLAuthError(resp)) {
                       context.target.setAuthFailure();
                       throw new Error(authFailMessage);
+                    } else if (isGraphQLSSLError(resp)) {
+                      context.target.setSslFailure();
+                      throw new Error(missingSSLMessage);
                     } else {
                       throw new Error(resp.errors[0].message);
                     }

diff --git a/src/app/Dashboard/Charts/mbean/MBeanMetricsChartCard.tsx b/src/app/Dashboard/Charts/mbean/MBeanMetricsChartCard.tsx
@@ -20,6 +20,8 @@ import {
   DashboardCardSizes,
   DashboardCardDescriptor,
 } from '@app/Dashboard/types';
+import { ErrorView } from '@app/ErrorView/ErrorView';
+import { missingSSLMessage, authFailMessage, isAuthFail } from '@app/ErrorView/types';
 import { ThemeType, ThemeSetting } from '@app/Settings/types';
 import { MBeanMetrics } from '@app/Shared/Services/api.types';
 import { FeatureLevel } from '@app/Shared/Services/service.types';
@@ -370,6 +372,8 @@ export const MBeanMetricsChartCard: DashboardCardFC<MBeanMetricsChartCardProps>
   const addSubscription = useSubscriptions();
   const [samples, setSamples] = React.useState([] as Sample[]);
   const [isLoading, setLoading] = React.useState(true);
+  const [errorMessage, setErrorMessage] = React.useState('');
+  const isError = React.useMemo(() => errorMessage != '', [errorMessage]);
 
   const resizeObserver = React.useRef((): void => undefined);
   const [cardWidth, setCardWidth] = React.useState(0);
@@ -411,9 +415,34 @@ export const MBeanMetricsChartCard: DashboardCardFC<MBeanMetricsChartCardProps>
     addSubscription(
       serviceContext.target.target().subscribe((_) => {
         setSamples([]);
+        setErrorMessage(''); // Reset on change
       }),
     );
-  }, [addSubscription, serviceContext, setSamples, refresh]);
+  }, [addSubscription, serviceContext, setSamples, setErrorMessage, refresh]);
+
+  React.useEffect(() => {
+    addSubscription(
+      serviceContext.target.authRetry().subscribe(() => {
+        setErrorMessage(''); // Reset on retry
+      }),
+    );
+  }, [addSubscription, serviceContext.target, setErrorMessage]);
+
+  React.useEffect(() => {
+    addSubscription(
+      serviceContext.target.sslFailure().subscribe(() => {
+        setErrorMessage(missingSSLMessage);
+      }),
+    );
+  }, [addSubscription, serviceContext.target, setErrorMessage]);
+
+  React.useEffect(() => {
+    addSubscription(
+      serviceContext.target.authFailure().subscribe(() => {
+        setErrorMessage(authFailMessage);
+      }),
+    );
+  }, [addSubscription, serviceContext.target, setErrorMessage]);
 
   React.useEffect(() => {
     refresh();
@@ -424,6 +453,10 @@ export const MBeanMetricsChartCard: DashboardCardFC<MBeanMetricsChartCardProps>
     addSubscription(controllerContext.mbeanController.loading().subscribe(setLoading));
   }, [addSubscription, controllerContext, setLoading]);
 
+  const authRetry = React.useCallback(() => {
+    serviceContext.target.setAuthRetry();
+  }, [serviceContext.target]);
+
   const refreshButton = React.useMemo(
     () => (
       <Button
@@ -466,7 +499,13 @@ export const MBeanMetricsChartCard: DashboardCardFC<MBeanMetricsChartCardProps>
     [theme, containerRef, props.themeColor, props.isFullHeight, chartKind, cardWidth, samples],
   );
 
-  return (
+  return isError ? (
+    <ErrorView
+      title={'Error retrieving metrics'}
+      message={errorMessage}
+      retry={isAuthFail(errorMessage) ? authRetry : undefined}
+    />
+  ) : (
     <DashboardCard
       id={props.chartKind + '-chart-card'}
       dashboardId={props.dashboardId}

diff --git a/src/app/Shared/Services/Api.service.tsx b/src/app/Shared/Services/Api.service.tsx
@@ -59,7 +59,6 @@ import {
   ActiveRecordingsFilterInput,
   RecordingCountResponse,
   MBeanMetrics,
-  MBeanMetricsResponse,
   EventType,
   NotificationCategory,
   HttpError,
@@ -73,8 +72,18 @@ import {
   Metadata,
   TargetMetadata,
   isTargetMetadata,
+  MBeanMetricsResponse,
 } from './api.types';
-import { isHttpError, includesTarget, isHttpOk, isXMLHttpError } from './api.utils';
+import {
+  isHttpError,
+  includesTarget,
+  isHttpOk,
+  isXMLHttpError,
+  isGraphQLAuthError,
+  isGraphQLSSLError,
+  isGraphQLError,
+  GraphQLError,
+} from './api.utils';
 import { LoginService } from './Login.service';
 import { NotificationService } from './Notifications.service';
 import { TargetService } from './Target.service';
@@ -807,33 +816,32 @@ export class ApiService {
   ): Observable<T> {
     const headers = new Headers();
     headers.set('Content-Type', 'application/json');
-    return this.sendRequest(
-      'v2.2',
-      'graphql',
-      {
-        method: 'POST',
-        body: JSON.stringify({
-          query: query.replace(/[\s]+/g, ' '),
-          variables,
+    const req = () =>
+      this.sendRequest(
+        'v2.2',
+        'graphql',
+        {
+          method: 'POST',
+          body: JSON.stringify({
+            query: query.replace(/[\s]+/g, ' '),
+            variables,
+          }),
+          headers,
+        },
+        undefined,
+        suppressNotifications,
+        skipStatusCheck,
+      ).pipe(
+        map((resp) => resp.json()),
+        concatMap(from),
+        tap((resp) => {
+          if (isGraphQLError(resp)) {
+            this.handleError(new GraphQLError(resp.errors), req);
+          }
         }),
-        headers,
-      },
-      undefined,
-      suppressNotifications,
-      skipStatusCheck,
-    ).pipe(
-      map((resp) => resp.json()),
-      concatMap(from),
-      tap((resp) => {
-        if (suppressNotifications || !resp?.errors?.length) {
-          return;
-        }
-        resp.errors.forEach((err) =>
-          this.notifications.danger(`Request failed (${err.extensions.classification})`, err.message),
-        );
-      }),
-      first(),
-    );
+        first(),
+      );
+    return req();
   }
 
   downloadRecording(recording: Recording): void {
@@ -1452,6 +1460,20 @@ export class ApiService {
         });
       }
       throw error;
+    } else if (isGraphQLError(error)) {
+      if (isGraphQLAuthError(error)) {
+        this.target.setAuthFailure();
+        return this.target.authRetry().pipe(mergeMap(() => retry()));
+      } else if (isGraphQLSSLError(error)) {
+        this.target.setSslFailure();
+      } else {
+        if (!suppressNotifications) {
+          error.errors.forEach((err) =>
+            this.notifications.danger(`Request failed (${err.extensions.classification})`, err.message),
+          );
+        }
+      }
+      throw error;
     }
     if (!suppressNotifications) {
       this.notifications.danger(`Request failed`, error.message);

diff --git a/src/app/Shared/Services/api.utils.ts b/src/app/Shared/Services/api.utils.ts
@@ -59,14 +59,29 @@ export const isActiveRecording = (toCheck: Recording): toCheck is ActiveRecordin
   return (toCheck as ActiveRecording).state !== undefined;
 };
 
-/* eslint @typescript-eslint/no-explicit-any: 0 */
-export const isGraphQLAuthError = (resp: any): boolean => {
-  if (resp.errors !== undefined) {
-    if (resp.errors[0].message.includes('Authentication failed!')) {
-      return true;
-    }
+// ======================================
+// GraphQL Error Handling utils
+// ======================================
+
+export class GraphQLError extends Error {
+  constructor(readonly errors: any[]) {
+    super();
   }
-  return false;
+}
+
+/* eslint @typescript-eslint/no-explicit-any: 0 */
+export const isGraphQLError = (resp: any): resp is GraphQLError => {
+  return Array.isArray(resp?.errors) && resp.errors.length > 0;
+};
+
+/* eslint @typescript-eslint/no-explicit-any: 0 */
+export const isGraphQLAuthError = (resp: GraphQLError): boolean => {
+  return isGraphQLError(resp) && resp.errors.some((v) => v.message.includes('Client Error (427)'));
+};
+
+/* eslint @typescript-eslint/no-explicit-any: 0 */
+export const isGraphQLSSLError = (resp: GraphQLError): boolean => {
+  return isGraphQLError(resp) && resp.errors.some((v) => v.message.includes('Bad Gateway'));
 };
 
 // ======================================

diff --git a/src/test/Dashboard/Charts/mbean/MBeanMetricsChartCard.test.tsx b/src/test/Dashboard/Charts/mbean/MBeanMetricsChartCard.test.tsx
@@ -33,6 +33,9 @@ import { from, of } from 'rxjs';
 jest.spyOn(defaultServices.settings, 'datetimeFormat').mockReturnValue(of(defaultDatetimeFormat));
 jest.spyOn(defaultServices.settings, 'themeSetting').mockReturnValue(of(ThemeSetting.DARK));
 jest.spyOn(defaultServices.settings, 'media').mockReturnValue(of(mockMediaQueryList));
+jest.spyOn(defaultServices.target, 'authFailure').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'authRetry').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'sslFailure').mockReturnValue(of());
 
 const mockTarget = {
   connectUrl: 'service:jmx:rmi://someUrl',

diff --git a/src/test/Dashboard/Dashboard.test.tsx b/src/test/Dashboard/Dashboard.test.tsx
@@ -56,6 +56,9 @@ jest.spyOn(defaultServices.settings, 'themeSetting').mockReturnValue(of(ThemeSet
 jest.spyOn(defaultServices.settings, 'media').mockReturnValue(of());
 jest.spyOn(defaultServices.settings, 'chartControllerConfig').mockReturnValue(defaultChartControllerConfig);
 jest.spyOn(defaultServices.api, 'getTargetMBeanMetrics').mockReturnValue(of({}));
+jest.spyOn(defaultServices.target, 'authFailure').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'authRetry').mockReturnValue(of());
+jest.spyOn(defaultServices.target, 'sslFailure').mockReturnValue(of());
 
 describe('<Dashboard />', () => {
   it('renders correctly', async () => {