fixup! fixup! fixup! fix: 'Backend is not available...' error message

eclipse-che · Oct 5, 2023 · c498f0f · c498f0f
1 parent 74206bf
commit c498f0f
Show file tree

Hide file tree

Showing 19 changed files with 206 additions and 124 deletions.
diff --git a/packages/common/package.json b/packages/common/package.json
@@ -28,7 +28,7 @@
     "eslint-config-prettier": "^9.0.0",
     "eslint-plugin-notice": "^0.9.10",
     "eslint-plugin-prettier": "^5.0.0",
-    "jest": "^29.6.2",
+    "jest": "^29.7.0",
     "prettier": "^3.0.1",
     "rimraf": "^5.0.1",
     "ts-jest": "^29.1.1",

diff --git a/packages/dashboard-backend/package.json b/packages/dashboard-backend/package.json
@@ -65,7 +65,7 @@
     "eslint-plugin-prettier": "^5.0.0",
     "eslint-webpack-plugin": "^4.0.1",
     "file-loader": "^6.2.0",
-    "jest": "^29.6.2",
+    "jest": "^29.7.0",
     "json-schema": "^0.4.0",
     "nodemon": "^3.0.1",
     "prettier": "^3.0.2",

diff --git a/packages/dashboard-frontend/src/Layout/ErrorBoundary/__tests__/index.spec.tsx b/packages/dashboard-frontend/src/Layout/ErrorBoundary/__tests__/index.spec.tsx
@@ -37,8 +37,9 @@ export class NoResourceComponent extends React.Component {
 // mute the outputs
 console.error = jest.fn();
 
+const mockTestBackends = jest.fn();
 function wrapComponent(componentToWrap: React.ReactNode) {
-  return <ErrorBoundary>{componentToWrap}</ErrorBoundary>;
+  return <ErrorBoundary testBackends={mockTestBackends}>{componentToWrap}</ErrorBoundary>;
 }
 
 describe('Error boundary', () => {
@@ -66,6 +67,7 @@ describe('Error boundary', () => {
       const showDetailsAction = screen.getByRole('button', { name: 'View stack' });
       userEvent.click(showDetailsAction);
 
+      expect(mockTestBackends).not.toHaveBeenCalled();
       expect(screen.queryByText('in BadComponent', { exact: false })).toBeTruthy();
       expect(screen.queryByText('in ErrorBoundary', { exact: false })).toBeTruthy();
 
@@ -98,6 +100,7 @@ describe('Error boundary', () => {
       const errorBoundary = wrapComponent(<NoResourceComponent />);
       render(errorBoundary);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(
         screen.queryByText('The application has been likely updated on the server.', {
           exact: false,
@@ -111,6 +114,7 @@ describe('Error boundary', () => {
       const errorBoundary = wrapComponent(<NoResourceComponent />);
       render(errorBoundary);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(
         screen.queryByText('Refreshing a page to get newer resources in', { exact: false }),
       ).toBeTruthy();
@@ -127,6 +131,7 @@ describe('Error boundary', () => {
       const stopCountdownAction = screen.getByRole('button', { name: 'Stop countdown' });
       userEvent.click(stopCountdownAction);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(
         screen.queryByText('Refreshing a page to get newer resources in', { exact: false }),
       ).toBeFalsy();
@@ -148,6 +153,7 @@ describe('Error boundary', () => {
 
       jest.advanceTimersByTime(35000);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(window.location.reload).toHaveBeenCalled();
       expect(window.location.reload).toHaveBeenCalledTimes(1);
     });
@@ -161,6 +167,7 @@ describe('Error boundary', () => {
       userEvent.click(reloadNowAction);
       userEvent.click(reloadNowAction);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(window.location.reload).toHaveBeenCalled();
       expect(window.location.reload).toHaveBeenCalledTimes(3);
     });
@@ -185,6 +192,7 @@ describe('Error boundary', () => {
       window.dispatchEvent(new Event('beforeunload'));
       render(errorBoundary);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(
         screen.queryByText(
           'Contact an administrator if refreshing continues after the next load.',
@@ -225,6 +233,7 @@ describe('Error boundary', () => {
       window.dispatchEvent(new Event('beforeunload'));
       render(goodComponent);
 
+      expect(mockTestBackends).toHaveBeenCalledWith('Loading chunk 23 failed.');
       expect(sessionStorage.getItem(STORAGE_KEY_RELOAD_NUMBER)).toBeNull();
     });
   });

diff --git a/packages/dashboard-frontend/src/Layout/ErrorBoundary/index.tsx b/packages/dashboard-frontend/src/Layout/ErrorBoundary/index.tsx
@@ -28,7 +28,9 @@ export const STORAGE_KEY_RELOAD_NUMBER = 'UD:ErrorBoundary:reloaded';
 const RELOAD_TIMEOUT_SEC = 30;
 const RELOADS_FOR_EXTENDED_MESSAGE = 2;
 
-type Props = PropsWithChildren;
+type Props = PropsWithChildren & {
+  testBackends: (error?: string) => void;
+};
 type State = {
   hasError: boolean;
   error?: Error;
@@ -43,7 +45,6 @@ type State = {
 
 export class ErrorBoundary extends React.PureComponent<Props, State> {
   private readonly toDispose = new DisposableCollection();
-  private notFoundCallback: (error?: Error) => void;
 
   constructor(props: Props) {
     super(props);
@@ -75,9 +76,7 @@ export class ErrorBoundary extends React.PureComponent<Props, State> {
     });
 
     if (this.testResourceNotFound(error)) {
-      if (this.notFoundCallback) {
-        this.notFoundCallback(error);
-      }
+      this.props.testBackends(error.message);
       this.setState({
         shouldReload: true,
       });
@@ -99,13 +98,6 @@ export class ErrorBoundary extends React.PureComponent<Props, State> {
     this.toDispose.dispose();
   }
 
-  /**
-   * This method is used from parent component by reference.
-   */
-  public setCallback(notFoundCallback: (error?: Error) => void): void {
-    this.notFoundCallback = notFoundCallback;
-  }
-
   private testResourceNotFound(error: Error): boolean {
     return /loading chunk [\d]+ failed/i.test(error.message);
   }

diff --git a/packages/dashboard-frontend/src/Layout/index.tsx b/packages/dashboard-frontend/src/Layout/index.tsx
@@ -31,6 +31,7 @@ import { ToggleBarsContext } from '../contexts/ToggleBars';
 import { signOut } from '../services/helpers/login';
 import { selectDashboardLogo } from '../store/ServerConfig/selectors';
 import * as SanityCheckStore from '../store/SanityCheck';
+import { selectSanityCheckError } from '../store/SanityCheck/selectors';
 
 const IS_MANAGED_SIDEBAR = false;
 
@@ -46,13 +47,10 @@ type State = {
 export class Layout extends React.PureComponent<Props, State> {
   @lazyInject(IssuesReporterService)
   private readonly issuesReporterService: IssuesReporterService;
-  private readonly errorBoundaryRef: React.RefObject<ErrorBoundary>;
 
   constructor(props: Props) {
     super(props);
 
-    this.errorBoundaryRef = React.createRef<ErrorBoundary>();
-
     this.state = {
       isHeaderVisible: true,
       isSidebarVisible: true,
@@ -89,11 +87,13 @@ export class Layout extends React.PureComponent<Props, State> {
     if (matchFactoryLoaderPath !== null || matchIdeLoaderPath !== null) {
       this.hideAllBars();
     }
-    this.errorBoundaryRef.current?.setCallback((error?: Error) => {
-      if (error?.message) {
-        console.error(error.message);
+  }
+  private testBackends(error?: string): void {
+    this.props.testBackends().catch(() => {
+      if (error) {
+        console.error(error);
       }
-      this.props.testBackends();
+      console.error('Error testing backends:', this.props.sanityCheckError);
     });
   }
 
@@ -145,7 +145,7 @@ export class Layout extends React.PureComponent<Props, State> {
           }
           isManagedSidebar={IS_MANAGED_SIDEBAR}
         >
-          <ErrorBoundary ref={this.errorBoundaryRef}>
+          <ErrorBoundary testBackends={error => this.testBackends(error)}>
             <StoreErrorsAlert />
             <BannerAlert />
             {this.props.children}
@@ -159,6 +159,7 @@ export class Layout extends React.PureComponent<Props, State> {
 const mapStateToProps = (state: AppState) => ({
   branding: selectBranding(state),
   dashboardLogo: selectDashboardLogo(state),
+  sanityCheckError: selectSanityCheckError(state),
 });
 
 const connector = connect(mapStateToProps, SanityCheckStore.actionCreators);

diff --git a/packages/dashboard-frontend/src/services/helpers/types.ts b/packages/dashboard-frontend/src/services/helpers/types.ts
@@ -13,6 +13,7 @@
 import { AlertVariant } from '@patternfly/react-core';
 import * as React from 'react';
 import devfileApi from '../devfileApi';
+import * as cheApi from '@eclipse-che/api';
 
 export type ActionCallback = {
   title: string;
@@ -35,7 +36,7 @@ export interface FactoryResolver {
   devfile: devfileApi.Devfile | che.WorkspaceDevfile;
   location?: string;
   scm_info?: FactoryResolverScmInfo;
-  links: api.che.core.rest.Link[];
+  links: cheApi.che.core.rest.Link[];
 }
 
 export type FactoryResolverScmInfo = {

diff --git a/packages/dashboard-frontend/src/services/workspace-client/__tests__/helpers.spec.ts b/packages/dashboard-frontend/src/services/workspace-client/__tests__/helpers.spec.ts
@@ -13,6 +13,7 @@
 import {
   getCustomEditor,
   hasLoginPage,
+  getErrorMessage,
   isForbidden,
   isInternalServerError,
   isUnauthorized,
@@ -24,6 +25,28 @@ import devfileApi from '../../devfileApi';
 import { FakeStoreBuilder } from '../../../store/__mocks__/storeBuilder';
 
 describe('Workspace-client helpers', () => {
+  describe('get an error message', () => {
+    it('should return the default error message', () => {
+      expect(getErrorMessage(undefined)).toEqual('Check the browser logs message.');
+    });
+    it('should return the unknown error message', () => {
+      expect(getErrorMessage({})).toEqual('Unexpected error type. Please report a bug.');
+    });
+    it('should return unknown an error message', () => {
+      expect(
+        getErrorMessage({
+          response: {
+            status: 401,
+          },
+          request: {
+            responseURL: 'http://dummyurl.com',
+          },
+        }),
+      ).toEqual(
+        'HTTP Error code 401. Endpoint which throws an error http://dummyurl.com. Check the browser logs message.',
+      );
+    });
+  });
   describe('checks for HTML login page in response data', () => {
     it('should return false without  HTML login page', () => {
       expect(

diff --git a/packages/dashboard-frontend/src/services/workspace-client/helpers.ts b/packages/dashboard-frontend/src/services/workspace-client/helpers.ts
@@ -31,7 +31,11 @@ export function getErrorMessage(error: unknown): string {
     const code = response?.status ? response?.status : response?.request?.status;
     const endpoint = request?.responseURL ? request?.responseURL : response?.request?.responseURL;
 
-    errorMessage = `HTTP Error code ${code}. Endpoint which throw an error ${endpoint}. ${errorMessage}`;
+    if (!code || !endpoint) {
+      return 'Unexpected error type. Please report a bug.';
+    }
+
+    errorMessage = `HTTP Error code ${code}. Endpoint which throws an error ${endpoint}. ${errorMessage}`;
   }
 
   if (isUnauthorized(error) || isForbidden(error)) {

diff --git a/packages/dashboard-frontend/src/store/DevfileRegistries/__tests__/index.spec.ts b/packages/dashboard-frontend/src/store/DevfileRegistries/__tests__/index.spec.ts
@@ -58,6 +58,14 @@ describe('Devfile registries', () => {
       const actions = store.getActions();
 
       const expectedActions: devfileRegistriesStore.KnownAction[] = [
+        {
+          type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
+          check: AUTHORIZED,
+        },
+        {
+          type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
+          check: AUTHORIZED,
+        },
         {
           type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
           check: AUTHORIZED,
@@ -116,6 +124,14 @@ describe('Devfile registries', () => {
       const actions = store.getActions();
 
       const expectedActions: devfileRegistriesStore.KnownAction[] = [
+        {
+          type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
+          check: AUTHORIZED,
+        },
+        {
+          type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
+          check: AUTHORIZED,
+        },
         {
           type: devfileRegistriesStore.Type.REQUEST_REGISTRY_METADATA,
           check: AUTHORIZED,

diff --git a/packages/dashboard-frontend/src/store/DevfileRegistries/index.ts b/packages/dashboard-frontend/src/store/DevfileRegistries/index.ts
@@ -19,6 +19,7 @@ import fetchAndUpdateDevfileSchema from './fetchAndUpdateDevfileSchema';
 import devfileApi from '../../services/devfileApi';
 import { fetchResources, loadResourcesContent } from '../../services/registry/resources';
 import { AUTHORIZED, SanityCheckAction } from '../sanityCheckMiddleware';
+import { selectAsyncIsAuthorized, selectSanityCheckError } from '../SanityCheck/selectors';
 
 export const DEFAULT_REGISTRY = '/dashboard/devfile-registry/';
 
@@ -170,12 +171,15 @@ export const actionCreators: ActionCreators = {
    */
   requestRegistriesMetadata:
     (registryUrls: string, isExternal: boolean): AppThunk<KnownAction, Promise<void>> =>
-    async (dispatch): Promise<void> => {
-      await dispatch({ type: Type.REQUEST_REGISTRY_METADATA, check: AUTHORIZED });
-
+    async (dispatch, getState): Promise<void> => {
       const registries: string[] = registryUrls.split(' ');
       const promises = registries.map(async url => {
         try {
+          await dispatch({ type: Type.REQUEST_REGISTRY_METADATA, check: AUTHORIZED });
+          if (!(await selectAsyncIsAuthorized(getState()))) {
+            const error = selectSanityCheckError(getState());
+            throw new Error(error);
+          }
           const metadata: che.DevfileMetaData[] = await fetchRegistryMetadata(url, isExternal);
           if (!Array.isArray(metadata) || metadata.length === 0) {
             return;
@@ -205,9 +209,13 @@ export const actionCreators: ActionCreators = {
 
   requestDevfile:
     (url: string): AppThunk<KnownAction, Promise<string>> =>
-    async (dispatch): Promise<string> => {
-      await dispatch({ type: Type.REQUEST_DEVFILE, check: AUTHORIZED });
+    async (dispatch, getState): Promise<string> => {
       try {
+        await dispatch({ type: Type.REQUEST_DEVFILE, check: AUTHORIZED });
+        if (!(await selectAsyncIsAuthorized(getState()))) {
+          const error = selectSanityCheckError(getState());
+          throw new Error(error);
+        }
         const devfile = await fetchDevfile(url);
         dispatch({ type: Type.RECEIVE_DEVFILE, devfile, url });
         return devfile;
@@ -218,10 +226,13 @@ export const actionCreators: ActionCreators = {
 
   requestResources:
     (resourcesUrl: string): AppThunk<KnownAction, Promise<void>> =>
-    async (dispatch): Promise<void> => {
-      await dispatch({ type: Type.REQUEST_RESOURCES, check: AUTHORIZED });
-
+    async (dispatch, getState): Promise<void> => {
       try {
+        await dispatch({ type: Type.REQUEST_RESOURCES, check: AUTHORIZED });
+        if (!(await selectAsyncIsAuthorized(getState()))) {
+          const error = selectSanityCheckError(getState());
+          throw new Error(error);
+        }
         const resourcesContent = await fetchResources(resourcesUrl);
         const resources = loadResourcesContent(resourcesContent);
 
@@ -259,9 +270,13 @@ export const actionCreators: ActionCreators = {
 
   requestJsonSchema:
     (): AppThunk<KnownAction, any> =>
-    async (dispatch): Promise<any> => {
-      await dispatch({ type: Type.REQUEST_SCHEMA, check: AUTHORIZED });
+    async (dispatch, getState): Promise<any> => {
       try {
+        await dispatch({ type: Type.REQUEST_SCHEMA, check: AUTHORIZED });
+        if (!(await selectAsyncIsAuthorized(getState()))) {
+          const error = selectSanityCheckError(getState());
+          throw new Error(error);
+        }
         const schemav200 = await fetchAndUpdateDevfileSchema('2.0.0');
         const schemav210 = await fetchAndUpdateDevfileSchema('2.1.0');
         const schemav220 = await fetchAndUpdateDevfileSchema('2.2.0');