From dca5d1af03c2348f592f9a0abd6d53e563ded610 Mon Sep 17 00:00:00 2001 From: Oleksii Orel Date: Fri, 21 Oct 2022 19:44:44 +0300 Subject: [PATCH 1/2] feat: handle gracefully when an invalid devfile is found in a git repository Signed-off-by: Oleksii Orel --- .deps/prod.md | 10 +- packages/dashboard-frontend/package.json | 2 +- .../__snapshots__/index.spec.tsx.snap | 94 +++++++++++ .../__tests__/index.spec.tsx | 62 +++++++ .../ExpandableWarning/index.module.css | 28 ++++ .../components/ExpandableWarning/index.tsx | 155 ++++++++++++++++++ .../Factory/Steps/Apply/Devfile/index.tsx | 117 +++++++++++-- .../Steps/CheckExistingWorkspaces/index.tsx | 12 +- .../Factory/Steps/Fetch/Devfile/index.tsx | 71 +++++++- .../GetStartedTab/SamplesListGallery.tsx | 6 +- .../src/services/bootstrap/index.ts | 12 +- .../devWorkspaceApi.ts | 6 +- .../src/store/DevfileRegistries/selectors.ts | 33 ++++ .../FactoryResolver/__tests__/index.spec.ts | 4 +- .../src/store/FactoryResolver/index.ts | 7 +- .../store/Workspaces/cheWorkspaces/index.ts | 4 +- .../store/Workspaces/devWorkspaces/index.ts | 4 +- yarn.lock | 4 +- 18 files changed, 580 insertions(+), 51 deletions(-) create mode 100644 packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/__snapshots__/index.spec.tsx.snap create mode 100644 packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/index.spec.tsx create mode 100644 packages/dashboard-frontend/src/components/ExpandableWarning/index.module.css create mode 100644 packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx diff --git a/.deps/prod.md b/.deps/prod.md index 2fdf10884..8851be1c6 100644 --- a/.deps/prod.md +++ b/.deps/prod.md @@ -7,11 +7,11 @@ | `@eclipse-che/api@7.44.0` | EPL-2.0 | ecd.che | | [`@eclipse-che/che-code-devworkspace-handler@1.64.0-dev-210b722`](git+https://github.com/che-incubator/che-code.git) | EPL-2.0 | ecd.che | | [`@eclipse-che/che-theia-devworkspace-handler@0.0.1-1642670698`](git+https://github.com/eclipse-che/che-theia.git) | EPL-2.0 | ecd.che | -| [`@eclipse-che/common@7.54.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che | -| [`@eclipse-che/dashboard-backend@7.54.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che | -| [`@eclipse-che/dashboard-frontend@7.54.0-next`](git://github.com/eclipse/che-dashboard.git) | EPL-2.0 | ecd.che | +| [`@eclipse-che/common@7.56.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che | +| [`@eclipse-che/dashboard-backend@7.56.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che | +| [`@eclipse-che/dashboard-frontend@7.56.0-next`](git://github.com/eclipse/che-dashboard.git) | EPL-2.0 | ecd.che | | [`@eclipse-che/devfile-converter@0.0.1-d624e3e`](git+https://github.com/che-incubator/devfile-converter.git) | EPL-2.0 | ecd.che | -| [`@eclipse-che/workspace-client@0.0.1-1632305737`](https://github.com/eclipse/che-workspace-client) | EPL-2.0 | ecd.che | +| [`@eclipse-che/workspace-client@0.0.1-1663851810`](https://github.com/eclipse/che-workspace-client) | EPL-2.0 | ecd.che | | [`@fastify/ajv-compiler@1.1.0`](git+https://github.com/fastify/ajv-compiler.git) | MIT | clearlydefined | | [`@fastify/cors@7.0.0`](git+https://github.com/fastify/fastify-cors.git) | MIT | clearlydefined | | [`@fastify/error@3.0.0`](git+https://github.com/fastify/fastify-error.git) | MIT | clearlydefined | @@ -324,7 +324,7 @@ | [`pino-std-serializers@3.2.0`](git+ssh://git@github.com/pinojs/pino-std-serializers.git) | MIT | clearlydefined | | [`pino@6.14.0`](git+https://github.com/pinojs/pino.git) | MIT | clearlydefined | | [`popper.js@1.16.1`](git+https://github.com/FezVrasta/popper.js.git) | MIT | [CQ22353](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22353) | -| [`postcss@8.4.14`](https://github.com/postcss/postcss.git) | MIT | clearlydefined | +| [`postcss@8.4.14`](https://github.com/postcss/postcss.git) | MIT | #3545 | | [`prettier@2.0.5`](https://github.com/prettier/prettier.git) | MIT | #1523 | | [`private@0.1.8`](git://github.com/benjamn/private.git) | MIT | clearlydefined | | [`process-warning@1.0.0`](git+https://github.com/fastify/processs-warning.git) | MIT | clearlydefined | diff --git a/packages/dashboard-frontend/package.json b/packages/dashboard-frontend/package.json index 31a72902c..e78cfe0c7 100644 --- a/packages/dashboard-frontend/package.json +++ b/packages/dashboard-frontend/package.json @@ -59,7 +59,7 @@ "process": "^0.11.10", "qs": "^6.9.4", "react": "^16.14.0", - "react-copy-to-clipboard": "^5.0.2", + "react-copy-to-clipboard": "^5.1.0", "react-dom": "^16.12.0", "react-helmet": "^6.1.0", "react-pluralize": "^1.6.3", diff --git a/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/__snapshots__/index.spec.tsx.snap b/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/__snapshots__/index.spec.tsx.snap new file mode 100644 index 000000000..9c5124e49 --- /dev/null +++ b/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/__snapshots__/index.spec.tsx.snap @@ -0,0 +1,94 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Expandable warning items should correctly render the component 1`] = ` +
+ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi non sollicitudin lorem, a suscipit massa. Cras egestas ante vel est pulvinar, a elementum orci faucibus. Etiam in risus et augue sollicitudin facilisis. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Vivamus ligula arcu, imperdiet hendrerit nulla varius, molestie volutpat nunc. + + +
+
+
+
+ +
+
+
+
+ 
+          
+            
+              Ut venenatis, purus ut ultrices luctus, nisi leo rutrum tortor, id sagittis nisl augue ac risus. In hac habitasse platea dictumst. Curabitur vitae dui eu elit egestas consectetur ac sed lacus. Nam nisl arcu, mollis eget tempor consequat, egestas nec lacus. Sed elementum, nibh id suscipit accumsan, metus mauris ultrices nisi, ut gravida sapien ipsum at elit. Maecenas in ultrices ex, id efficitur ante. Nulla facilisi.
+            
+          
+
+
+
+ + + Sed iaculis dictum nibh nec varius. Pellentesque ac diam vestibulum nisl condimentum feugiat. Sed et est in dolor posuere pharetra. Aliquam sodales lorem eu velit efficitur vestibulum. Praesent ornare ut tellus nec cursus. Proin at hendrerit metus, sed placerat justo. Cras id hendrerit ante, et consequat orci. Ut ante ipsum, eleifend sit amet iaculis quis, scelerisque eget mi. Pellentesque aliquam porttitor neque ut consectetur. Vivamus euismod elit velit, eget suscipit sem euismod vitae. Quisque sagittis, felis ut rhoncus vestibulum, arcu dui tincidunt quam, sit amet congue ligula dolor id sapien. + +
+`; diff --git a/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/index.spec.tsx b/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/index.spec.tsx new file mode 100644 index 000000000..2ef2ab29c --- /dev/null +++ b/packages/dashboard-frontend/src/components/ExpandableWarning/__tests__/index.spec.tsx @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2018-2021 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ + +import React from 'react'; +import renderer from 'react-test-renderer'; + +import ExpandableWarningItems, { ERROR_MESSAGE_ID } from '../'; +import { render, RenderResult, screen } from '@testing-library/react'; + +describe('Expandable warning items', () => { + it('should correctly render the component', () => { + const component = getComponent( + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi non sollicitudin lorem, a suscipit massa. Cras egestas ante vel est pulvinar, a elementum orci faucibus. Etiam in risus et augue sollicitudin facilisis. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Vivamus ligula arcu, imperdiet hendrerit nulla varius, molestie volutpat nunc.', + 'Ut venenatis, purus ut ultrices luctus, nisi leo rutrum tortor, id sagittis nisl augue ac risus. In hac habitasse platea dictumst. Curabitur vitae dui eu elit egestas consectetur ac sed lacus. Nam nisl arcu, mollis eget tempor consequat, egestas nec lacus. Sed elementum, nibh id suscipit accumsan, metus mauris ultrices nisi, ut gravida sapien ipsum at elit. Maecenas in ultrices ex, id efficitur ante. Nulla facilisi.', + 'Sed iaculis dictum nibh nec varius. Pellentesque ac diam vestibulum nisl condimentum feugiat. Sed et est in dolor posuere pharetra. Aliquam sodales lorem eu velit efficitur vestibulum. Praesent ornare ut tellus nec cursus. Proin at hendrerit metus, sed placerat justo. Cras id hendrerit ante, et consequat orci. Ut ante ipsum, eleifend sit amet iaculis quis, scelerisque eget mi. Pellentesque aliquam porttitor neque ut consectetur. Vivamus euismod elit velit, eget suscipit sem euismod vitae. Quisque sagittis, felis ut rhoncus vestibulum, arcu dui tincidunt quam, sit amet congue ligula dolor id sapien.', + ); + const json = renderer.create(component).toJSON(); + + expect(json).toMatchSnapshot(); + }); + + it('should show a short error message and an expand button', () => { + const component = getComponent( + 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi non sollicitudin lorem, a suscipit massa. Cras egestas ante vel est pulvinar, a elementum orci faucibus. Etiam in risus et augue sollicitudin facilisis. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Vivamus ligula arcu, imperdiet hendrerit nulla varius, molestie volutpat nunc.', + 'Ut venenatis, purus ut ultrices luctus, nisi leo rutrum tortor, id sagittis nisl augue ac risus. In hac habitasse platea dictumst. Curabitur vitae dui eu elit egestas consectetur ac sed lacus. Nam nisl arcu, mollis eget tempor consequat, egestas nec lacus. Sed elementum, nibh id suscipit accumsan, metus mauris ultrices nisi, ut gravida sapien ipsum at elit. Maecenas in ultrices ex, id efficitur ante. Nulla facilisi.', + 'Sed iaculis dictum nibh nec varius. Pellentesque ac diam vestibulum nisl condimentum feugiat. Sed et est in dolor posuere pharetra. Aliquam sodales lorem eu velit efficitur vestibulum. Praesent ornare ut tellus nec cursus. Proin at hendrerit metus, sed placerat justo. Cras id hendrerit ante, et consequat orci. Ut ante ipsum, eleifend sit amet iaculis quis, scelerisque eget mi. Pellentesque aliquam porttitor neque ut consectetur. Vivamus euismod elit velit, eget suscipit sem euismod vitae. Quisque sagittis, felis ut rhoncus vestibulum, arcu dui tincidunt quam, sit amet congue ligula dolor id sapien.', + ); + + renderComponent(component); + + expect(screen.queryByTestId(ERROR_MESSAGE_ID)).toBeTruthy; + expect(screen.getByTestId(ERROR_MESSAGE_ID).className).toEqual('hideOverflow'); + expect(screen.queryByText('Show More')).toBeTruthy; + }); +}); + +function getComponent( + textBefore: string, + errorMessage: string, + textAfter: string, +): React.ReactElement { + return ( + + ); +} + +function renderComponent(component: React.ReactElement): RenderResult { + return render(component); +} diff --git a/packages/dashboard-frontend/src/components/ExpandableWarning/index.module.css b/packages/dashboard-frontend/src/components/ExpandableWarning/index.module.css new file mode 100644 index 000000000..be0255a47 --- /dev/null +++ b/packages/dashboard-frontend/src/components/ExpandableWarning/index.module.css @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2018-2021 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ + +.error { + border-top: 2px solid; + border-color: var(--pf-global--palette--red-100); + background-color: var(--pf-global--palette--red-50); +} + +.error small, +.error button { + font-size: small; +} + +.hideOverflow { + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; +} diff --git a/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx b/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx new file mode 100644 index 000000000..6320815db --- /dev/null +++ b/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2018-2021 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ + +import React from 'react'; +import { + Button, + CodeBlock, + CodeBlockAction, + CodeBlockCode, + ExpandableSectionToggle, + TextContent, + Text, + TextVariants, +} from '@patternfly/react-core'; +import CopyToClipboard from 'react-copy-to-clipboard'; +import { CopyIcon } from '@patternfly/react-icons/dist/js/icons'; +import styles from './index.module.css'; + +export const ERROR_MESSAGE_ID = 'expandable-warning-error-message'; + +type Props = { + textBefore: string; + errorMessage: string; + textAfter: string; +}; +type State = { + copied: boolean; + hasExpand: boolean; + isExpanded: boolean; +}; + +class ExpandableWarningItems extends React.Component { + private readonly checkOverflow: () => void; + private copiedTimer: number | undefined; + + constructor(props: Props) { + super(props); + + this.state = { + copied: false, + hasExpand: false, + isExpanded: false, + }; + + this.checkOverflow = () => { + if (this.state.isExpanded) { + this.setState({ + isExpanded: false, + }); + return; + } + const errorMessageElement = document.getElementById(ERROR_MESSAGE_ID); + if (errorMessageElement !== null) { + const { offsetWidth, scrollWidth } = errorMessageElement; + const showExpandToggle = scrollWidth > offsetWidth; + this.setState({ + hasExpand: showExpandToggle, + }); + } + }; + } + + public componentDidMount(): void { + window.addEventListener('resize', this.checkOverflow, false); + this.checkOverflow(); + } + + public componentDidUpdate(prevProps: Readonly, prevState: Readonly) { + if ( + prevProps.errorMessage !== this.props.errorMessage || + (prevState.isExpanded && !this.state.isExpanded) + ) { + this.checkOverflow(); + } + } + + public componentWillUnmount() { + window.removeEventListener('resize', this.checkOverflow); + } + + private onToggle(isExpanded: boolean) { + this.setState({ isExpanded }); + } + + private onCopyToClipboard() { + this.setState({ copied: true }); + if (this.copiedTimer) { + clearTimeout(this.copiedTimer); + } + this.copiedTimer = window.setTimeout(() => { + this.setState({ copied: false }); + }, 3000); + } + + render(): React.ReactElement { + const { errorMessage, textBefore, textAfter } = this.props; + const { hasExpand, isExpanded, copied } = this.state; + const copyIconTitle = copied ? 'Copied' : 'Copy to clipboard'; + const actions = ( + + this.onCopyToClipboard()}> + + + + ); + + const expandableSectionTitle = isExpanded ? 'Show Less' : 'Show More'; + const messageClassName = isExpanded ? undefined : styles.hideOverflow; + return ( + <> + + {textBefore} + + + + + {errorMessage} + + + {hasExpand && ( + this.onToggle(isExpanded)} + direction="up" + > + {expandableSectionTitle} + + )} + + + {textAfter} + + + ); + } +} + +export default ExpandableWarningItems; diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx index 8c77eeff5..8753fb210 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx @@ -14,7 +14,7 @@ import React from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { isEqual } from 'lodash'; import { AlertVariant } from '@patternfly/react-core'; -import { helpers } from '@eclipse-che/common'; +import common, { helpers } from '@eclipse-che/common'; import { AppState } from '../../../../../../store'; import * as WorkspacesStore from '../../../../../../store/Workspaces'; import { DisposableCollection } from '../../../../../../services/helpers/disposable'; @@ -36,6 +36,16 @@ import { FactoryParams } from '../../../types'; import buildFactoryParams from '../../../buildFactoryParams'; import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../../AbstractStep'; import { AlertItem } from '../../../../../../services/helpers/types'; +import { selectDefaultDevfile } from '../../../../../../store/DevfileRegistries/selectors'; +import { getProjectName } from '../../../../../../services/helpers/getProjectName'; +import ExpandableWarningItems from '../../../../../../components/ExpandableWarning'; + +export class CreateWorkspaceError extends Error { + constructor(message: string) { + super(message); + this.name = 'CreateWorkspaceError'; + } +} export type Props = MappedProps & LoaderStepProps & { @@ -126,12 +136,45 @@ class StepApplyDevfile extends AbstractLoaderStep { this.props.onRestart(); } + private updateCurrentDevfile(devfile: devfileApi.Devfile): void { + const { factoryResolver, allWorkspaces, defaultDevfile } = this.props; + const { factoryParams } = this.state; + const { factoryId, policiesCreate, storageType } = factoryParams; + // when using the default devfile instead of a user devfile + if (factoryResolver === undefined && isEqual(devfile, defaultDevfile)) { + if (devfile.projects === undefined) { + devfile.projects = []; + } + if (devfile.projects.length === 0) { + // adds a default project from the source URL + const sourceUrl = new URL(factoryParams.sourceUrl); + const name = getProjectName(factoryParams.sourceUrl); + const origin = sourceUrl.pathname.endsWith('.git') + ? `${sourceUrl.origin}${sourceUrl.pathname}` + : `${sourceUrl.origin}${sourceUrl.pathname}.git`; + devfile.projects[0] = { git: { remotes: { origin } }, name }; + // change default name + devfile.metadata.name = name; + devfile.metadata.generateName = name; + } + } + // test the devfile name to decide if we need to append a suffix to is + const nameConflict = allWorkspaces.some(w => devfile.metadata.name === w.name); + const appendSuffix = policiesCreate === 'perclick' || nameConflict; + + const updatedDevfile = prepareDevfile(devfile, factoryId, storageType, appendSuffix); + + this.setState({ + devfile: updatedDevfile, + newWorkspaceName: updatedDevfile.metadata.name, + }); + } + protected async runStep(): Promise { await delay(MIN_STEP_DURATION_MS); - const { factoryResolverConverted } = this.props; - const { shouldCreate, factoryParams, devfile } = this.state; - const { factoryId, policiesCreate, storageType } = factoryParams; + const { factoryResolverConverted, factoryResolver, defaultDevfile } = this.props; + const { shouldCreate, devfile } = this.state; const workspace = this.findTargetWorkspace(this.props, this.state); if (workspace !== undefined) { @@ -150,25 +193,28 @@ class StepApplyDevfile extends AbstractLoaderStep { } if (devfile === undefined) { + if (factoryResolver === undefined) { + const _devfile = defaultDevfile; + if (_devfile === undefined) { + throw new Error('Failed to resolve the default devfile.'); + } + this.updateCurrentDevfile(_devfile); + return false; + } const _devfile = factoryResolverConverted?.devfileV2; if (_devfile === undefined) { throw new Error('Failed to resolve the devfile.'); } - - // test the devfile name to decide if we need to append a suffix to is - const nameConflict = this.props.allWorkspaces.some(w => _devfile.metadata.name === w.name); - const appendSuffix = policiesCreate === 'perclick' || nameConflict; - - const updatedDevfile = prepareDevfile(_devfile, factoryId, storageType, appendSuffix); - - this.setState({ - devfile: updatedDevfile, - newWorkspaceName: updatedDevfile.metadata.name, - }); + this.updateCurrentDevfile(_devfile); return false; } - await this.createWorkspaceFromDevfile(devfile); + try { + await this.createWorkspaceFromDevfile(devfile); + } catch (e) { + const errorMessage = common.helpers.errors.getMessage(e); + throw new CreateWorkspaceError(errorMessage); + } // wait for the workspace creation to complete try { @@ -206,7 +252,45 @@ class StepApplyDevfile extends AbstractLoaderStep { ); } + private handleCreateWorkspaceError(): void { + const { defaultDevfile } = this.props; + const { devfile } = this.state; + const _devfile = defaultDevfile; + if (_devfile && devfile) { + _devfile.projects = devfile.projects; + _devfile.metadata.name = devfile.metadata.name; + _devfile.metadata.generateName = devfile.metadata.generateName; + this.updateCurrentDevfile(_devfile); + } + this.clearStepError(); + } + private getAlertItem(error: unknown): AlertItem | undefined { + if (error instanceof CreateWorkspaceError) { + return { + key: 'factory-loader-create-workspace-error', + title: 'Warning', + variant: AlertVariant.warning, + children: ( + + ), + actionCallbacks: [ + { + title: 'Continue with the default devfile', + callback: () => this.handleCreateWorkspaceError(), + }, + { + title: 'Reload', + callback: () => this.clearStepError(), + }, + ], + }; + } if (!error) { return; } @@ -249,6 +333,7 @@ const mapStateToProps = (state: AppState) => ({ defaultNamespace: selectDefaultNamespace(state), factoryResolver: selectFactoryResolver(state), factoryResolverConverted: selectFactoryResolverConverted(state), + defaultDevfile: selectDefaultDevfile(state), }); const connector = connect( diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx index 8ba137ae9..b946a07d7 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx @@ -147,13 +147,17 @@ class StepCheckExistingWorkspaces extends AbstractLoaderStep { let newWorkspaceName: string; if (prevLoaderStep.id === LoadingStep.CREATE_WORKSPACE__FETCH_DEVFILE) { if ( - factoryResolver?.location !== factoryParams.sourceUrl || - factoryResolverConverted?.devfileV2 === undefined + factoryResolver?.location === factoryParams.sourceUrl && + factoryResolverConverted?.devfileV2 !== undefined ) { + const devfile = factoryResolverConverted.devfileV2; + newWorkspaceName = devfile.metadata.name; + } else { + if (factoryResolver === undefined) { + return true; + } throw new Error('Failed to resolve the devfile.'); } - const devfile = factoryResolverConverted.devfileV2; - newWorkspaceName = devfile.metadata.name; } else { const resources = devWorkspaceResources[factoryParams.sourceUrl]?.resources; if (resources === undefined) { diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx index caf356d25..db5059dd6 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx @@ -13,7 +13,7 @@ import React from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { isEqual } from 'lodash'; -import { helpers } from '@eclipse-che/common'; +import common, { helpers } from '@eclipse-che/common'; import { AlertVariant } from '@patternfly/react-core'; import { AppState } from '../../../../../../store'; import * as FactoryResolverStore from '../../../../../../store/FactoryResolver'; @@ -36,6 +36,14 @@ import buildFactoryParams from '../../../buildFactoryParams'; import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../../AbstractStep'; import { AlertItem } from '../../../../../../services/helpers/types'; import OAuthService, { isOAuthResponse } from '../../../../../../services/oauth'; +import ExpandableWarningItems from '../../../../../../components/ExpandableWarning'; + +export class ApplyingDevfileError extends Error { + constructor(message) { + super(message); + this.name = 'ApplyingDevfileError'; + } +} const RELOADS_LIMIT = 2; type ReloadsInfo = { @@ -49,6 +57,7 @@ export type Props = MappedProps & export type State = LoaderStepState & { factoryParams: FactoryParams; shouldResolve: boolean; + useDefaultDevfile: boolean; }; class StepFetchDevfile extends AbstractLoaderStep { @@ -60,6 +69,7 @@ class StepFetchDevfile extends AbstractLoaderStep { this.state = { factoryParams: buildFactoryParams(props.searchParams), shouldResolve: true, + useDefaultDevfile: false, }; } @@ -106,9 +116,9 @@ class StepFetchDevfile extends AbstractLoaderStep { private init() { const { factoryResolver } = this.props; - const { factoryParams } = this.state; + const { factoryParams, useDefaultDevfile } = this.state; const { sourceUrl } = factoryParams; - if (sourceUrl && sourceUrl === factoryResolver?.location) { + if (sourceUrl && (useDefaultDevfile || sourceUrl === factoryResolver?.location)) { // prevent a resource being fetched one more time this.setState({ shouldResolve: false, @@ -129,7 +139,7 @@ class StepFetchDevfile extends AbstractLoaderStep { protected async runStep(): Promise { await delay(MIN_STEP_DURATION_MS); - const { factoryParams, shouldResolve } = this.state; + const { factoryParams, shouldResolve, useDefaultDevfile } = this.state; const { currentStepIndex, factoryResolver, factoryResolverConverted, loaderSteps } = this.props; const { sourceUrl } = factoryParams; @@ -151,15 +161,30 @@ class StepFetchDevfile extends AbstractLoaderStep { } if (shouldResolve === false) { + if (useDefaultDevfile) { + // go to the next step + return true; + } + if (this.state.lastError instanceof Error) { throw this.state.lastError; } throw new Error('Failed to resolve the devfile.'); } - // start resolving the devfile - const resolveDone = await this.resolveDevfile(sourceUrl); - if (resolveDone === false) { + let resolveDone = false; + try { + // start resolving the devfile + resolveDone = await this.resolveDevfile(sourceUrl); + } catch (e) { + const errorMessage = common.helpers.errors.getMessage(e); + // check if it is a scheme validation error + if (errorMessage.includes('schema validation failed')) { + throw new ApplyingDevfileError(errorMessage); + } + throw e; + } + if (!resolveDone) { return false; } @@ -176,6 +201,13 @@ class StepFetchDevfile extends AbstractLoaderStep { } } + private handleDevfileError(): void { + this.setState({ + useDefaultDevfile: true, + }); + this.clearStepError(); + } + /** * Resolves promise with `true` if devfile resolved successfully. Resolves promise with `false` if the devfile needs to be resolved one more time after authentication. */ @@ -261,6 +293,31 @@ class StepFetchDevfile extends AbstractLoaderStep { } private getAlertItem(error: unknown): AlertItem | undefined { + if (error instanceof ApplyingDevfileError) { + return { + key: 'factory-loader-devfile-error', + title: 'Warning', + variant: AlertVariant.warning, + children: ( + + ), + actionCallbacks: [ + { + title: 'Continue with the default devfile', + callback: () => this.handleDevfileError(), + }, + { + title: 'Reload', + callback: () => this.clearStepError(), + }, + ], + }; + } if (!error) { return; } diff --git a/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx b/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx index c1035b3ba..398c77a50 100644 --- a/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx +++ b/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx @@ -31,7 +31,10 @@ import { AppState } from '../../../store'; import * as DevfileRegistriesStore from '../../../store/DevfileRegistries'; import { SampleCard } from './SampleCard'; import { AlertItem } from '../../../services/helpers/types'; -import { selectMetadataFiltered } from '../../../store/DevfileRegistries/selectors'; +import { + EMPTY_WORKSPACE_TAG, + selectMetadataFiltered, +} from '../../../store/DevfileRegistries/selectors'; import { selectWorkspacesSettings } from '../../../store/Workspaces/Settings/selectors'; import * as FactoryResolverStore from '../../../store/FactoryResolver'; import { isDevworkspacesEnabled } from '../../../services/helpers/devworkspace'; @@ -60,7 +63,6 @@ type State = { }; export const VISIBLE_TAGS = ['Community', 'Tech-Preview']; -export const EMPTY_WORKSPACE_TAG = 'Empty'; const EXCLUDED_TARGET_EDITOR_NAMES = ['dirigible', 'jupyter', 'eclipseide', 'code-server']; diff --git a/packages/dashboard-frontend/src/services/bootstrap/index.ts b/packages/dashboard-frontend/src/services/bootstrap/index.ts index b9c82e1b7..df3249b37 100644 --- a/packages/dashboard-frontend/src/services/bootstrap/index.ts +++ b/packages/dashboard-frontend/src/services/bootstrap/index.ts @@ -42,6 +42,7 @@ import { buildDetailsLocation, buildIdeLoaderLocation } from '../helpers/locatio import { Workspace } from '../workspace-adapter'; import { WorkspaceRunningError, WorkspaceStoppedDetector } from './workspaceStoppedDetector'; import { selectOpenVSXUrl } from '../../store/ServerConfig/selectors'; +import { selectEmptyWorkspaceUrl } from '../../store/DevfileRegistries/selectors'; /** * This class executes a few initial instructions @@ -88,7 +89,7 @@ export default class Bootstrap { this.fetchPlugins().then(() => this.fetchDevfileSchema()), this.fetchDwPlugins(), this.fetchDefaultDwPlugins(), - this.fetchRegistriesMetadata(), + this.fetchRegistriesMetadata().then(() => this.fetchEmptyWorkspace()), this.watchNamespaces(), this.updateDevWorkspaceTemplates(), this.fetchWorkspaces().then(() => this.checkWorkspaceStopped()), @@ -287,6 +288,15 @@ export default class Bootstrap { ); } + private async fetchEmptyWorkspace(): Promise { + const { requestDevfile } = DevfileRegistriesStore.actionCreators; + const state = this.store.getState(); + const emptyWorkspaceUrl = selectEmptyWorkspaceUrl(state); + if (emptyWorkspaceUrl) { + await requestDevfile(emptyWorkspaceUrl)(this.store.dispatch, this.store.getState, undefined); + } + } + private async fetchDevfileSchema(): Promise { const { requestJsonSchema } = DevfileRegistriesStore.actionCreators; return requestJsonSchema()(this.store.dispatch, this.store.getState, undefined); diff --git a/packages/dashboard-frontend/src/services/dashboard-backend-client/devWorkspaceApi.ts b/packages/dashboard-frontend/src/services/dashboard-backend-client/devWorkspaceApi.ts index 6bc57ea71..9619339dd 100644 --- a/packages/dashboard-frontend/src/services/dashboard-backend-client/devWorkspaceApi.ts +++ b/packages/dashboard-frontend/src/services/dashboard-backend-client/devWorkspaceApi.ts @@ -25,7 +25,11 @@ export async function createWorkspace( ); return response.data; } catch (e) { - throw `Failed to create a new workspace. ${helpers.errors.getMessage(e)}`; + const errorMessage = helpers.errors.getMessage(e); + if (errorMessage.startsWith('Unable to create devworkspace')) { + throw errorMessage; + } + throw `Failed to create a new workspace. ${errorMessage}`; } } diff --git a/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts b/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts index 58c157ae8..68f47a9da 100644 --- a/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts +++ b/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts @@ -15,6 +15,10 @@ import { AppState } from '..'; import match from '../../services/helpers/filter'; import { selectWorkspacesSettingsState } from '../Workspaces/Settings/selectors'; import { isDevworkspacesEnabled } from '../../services/helpers/devworkspace'; +import { load } from 'js-yaml'; +import devfileApi from '../../services/devfileApi'; + +export const EMPTY_WORKSPACE_TAG = 'Empty'; const selectState = (state: AppState) => state.devfileRegistries; @@ -63,6 +67,35 @@ export const selectMetadataFiltered = createSelector( }, ); +export const selectEmptyWorkspaceUrl = createSelector( + selectState, + selectRegistriesMetadata, + (state, metadata) => { + const v2Metadata = filterDevfileV2Metadata(metadata); + const emptyWorkspaceMetadata = v2Metadata.find(meta => meta.tags.includes(EMPTY_WORKSPACE_TAG)); + return emptyWorkspaceMetadata?.links?.v2; + }, +); + +export const selectDefaultDevfile = createSelector( + selectState, + selectEmptyWorkspaceUrl, + (state, devfileLocation) => { + if (!devfileLocation) { + return undefined; + } + const devfileContent = state.devfiles[devfileLocation]?.content; + if (devfileContent) { + try { + return load(devfileContent) as devfileApi.Devfile; + } catch (e) { + console.error(e); + } + } + return undefined; + }, +); + function matches(meta: che.DevfileMetaData, filterValue: string): boolean { return match(meta.displayName, filterValue) || match(meta.description || '', filterValue); } diff --git a/packages/dashboard-frontend/src/store/FactoryResolver/__tests__/index.spec.ts b/packages/dashboard-frontend/src/store/FactoryResolver/__tests__/index.spec.ts index 00cb44660..4e9d80545 100644 --- a/packages/dashboard-frontend/src/store/FactoryResolver/__tests__/index.spec.ts +++ b/packages/dashboard-frontend/src/store/FactoryResolver/__tests__/index.spec.ts @@ -167,7 +167,7 @@ describe('FactoryResolver store', () => { }, { type: 'RECEIVE_FACTORY_RESOLVER_ERROR', - error: expect.stringContaining('Failed to request factory resolver'), + error: expect.stringContaining('Unexpected error'), }, ]; @@ -181,7 +181,7 @@ describe('FactoryResolver store', () => { const location = 'http://factory-link'; await expect( store.dispatch(factoryResolverStore.actionCreators.requestFactoryResolver(location)), - ).rejects.toMatch('Failed to request factory resolver'); + ).rejects.toMatch('Unexpected error'); expect(actions).toEqual(expectedActions); isAxiosErrorMock.mockRestore(); diff --git a/packages/dashboard-frontend/src/store/FactoryResolver/index.ts b/packages/dashboard-frontend/src/store/FactoryResolver/index.ts index da5d33bc5..0ed1978ea 100644 --- a/packages/dashboard-frontend/src/store/FactoryResolver/index.ts +++ b/packages/dashboard-frontend/src/store/FactoryResolver/index.ts @@ -38,7 +38,7 @@ export type OAuthResponse = { oauth_authentication_url: string; }; errorCode: number; - message: string; + message: string | undefined; }; export interface ResolverState extends FactoryResolver { @@ -72,7 +72,7 @@ interface ReceiveFactoryResolverAction { interface ReceiveFactoryResolverErrorAction { type: 'RECEIVE_FACTORY_RESOLVER_ERROR'; - error: string; + error: string | undefined; } export type KnownAction = @@ -212,8 +212,7 @@ export const actionCreators: ActionCreators = { throw response.data; } } - const errorMessage = - 'Failed to request factory resolver: ' + common.helpers.errors.getMessage(e); + const errorMessage = common.helpers.errors.getMessage(e); dispatch({ type: 'RECEIVE_FACTORY_RESOLVER_ERROR', error: errorMessage, diff --git a/packages/dashboard-frontend/src/store/Workspaces/cheWorkspaces/index.ts b/packages/dashboard-frontend/src/store/Workspaces/cheWorkspaces/index.ts index ab034be16..90a98c625 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/cheWorkspaces/index.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/cheWorkspaces/index.ts @@ -420,9 +420,7 @@ export const actionCreators: ActionCreators = { workspace, }); } catch (e) { - const errorMessage = - 'Failed to create a new workspace from the devfile, reason: ' + - common.helpers.errors.getMessage(e); + const errorMessage = common.helpers.errors.getMessage(e); dispatch({ type: 'CHE_RECEIVE_ERROR', error: errorMessage, diff --git a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts index e1563b639..e52a7b8e1 100644 --- a/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts +++ b/packages/dashboard-frontend/src/store/Workspaces/devWorkspaces/index.ts @@ -635,9 +635,7 @@ export const actionCreators: ActionCreators = { workspace, }); } catch (e) { - const errorMessage = - 'Failed to create a new workspace from the devfile, reason: ' + - common.helpers.errors.getMessage(e); + const errorMessage = common.helpers.errors.getMessage(e); dispatch({ type: 'RECEIVE_DEVWORKSPACE_ERROR', error: errorMessage, diff --git a/yarn.lock b/yarn.lock index 5533e9061..ca85567c6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10170,9 +10170,9 @@ raw-body@2.5.1: iconv-lite "0.4.24" unpipe "1.0.0" -react-copy-to-clipboard@^5.0.2: +react-copy-to-clipboard@^5.1.0: version "5.1.0" - resolved "https://registry.npmjs.org/react-copy-to-clipboard/-/react-copy-to-clipboard-5.1.0.tgz" + resolved "https://registry.yarnpkg.com/react-copy-to-clipboard/-/react-copy-to-clipboard-5.1.0.tgz#09aae5ec4c62750ccb2e6421a58725eabc41255c" integrity sha512-k61RsNgAayIJNoy9yDsYzDe/yAZAzEbEgcz3DZMhF686LEyukcE1hzurxe85JandPUG+yTfGVFzuEw3xt8WP/A== dependencies: copy-to-clipboard "^3.3.1" From 1bb4ed1bc7ea94d8027714a918116c026261c5ef Mon Sep 17 00:00:00 2001 From: Oleksii Orel Date: Mon, 24 Oct 2022 11:12:01 +0300 Subject: [PATCH 2/2] fixup! feat: handle gracefully when an invalid devfile is found in a git repository --- .../src/components/ExpandableWarning/index.tsx | 4 ++-- .../containers/Loader/Factory/Steps/Apply/Devfile/index.tsx | 4 ++-- .../containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx b/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx index 6320815db..434cb80f3 100644 --- a/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx +++ b/packages/dashboard-frontend/src/components/ExpandableWarning/index.tsx @@ -38,7 +38,7 @@ type State = { isExpanded: boolean; }; -class ExpandableWarningItems extends React.Component { +class ExpandableWarning extends React.Component { private readonly checkOverflow: () => void; private copiedTimer: number | undefined; @@ -152,4 +152,4 @@ class ExpandableWarningItems extends React.Component { } } -export default ExpandableWarningItems; +export default ExpandableWarning; diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx index 8753fb210..53a03dc72 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx @@ -38,7 +38,7 @@ import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../. import { AlertItem } from '../../../../../../services/helpers/types'; import { selectDefaultDevfile } from '../../../../../../store/DevfileRegistries/selectors'; import { getProjectName } from '../../../../../../services/helpers/getProjectName'; -import ExpandableWarningItems from '../../../../../../components/ExpandableWarning'; +import ExpandableWarning from '../../../../../../components/ExpandableWarning'; export class CreateWorkspaceError extends Error { constructor(message: string) { @@ -272,7 +272,7 @@ class StepApplyDevfile extends AbstractLoaderStep { title: 'Warning', variant: AlertVariant.warning, children: ( -