Skip to content

Commit

Permalink
Show a warning notification on fetch token error when starting a work…
Browse files Browse the repository at this point in the history
…space
  • Loading branch information
vinokurig committed Sep 17, 2024
1 parent e35bccb commit 6916e23
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,17 @@ import {
import { ProgressStepTitle } from '@/components/WorkspaceProgress/StepTitle';
import { TimeLimit } from '@/components/WorkspaceProgress/TimeLimit';
import workspaceStatusIs from '@/components/WorkspaceProgress/workspaceStatusIs';
import { lazyInject } from '@/inversify.config';
import { WorkspaceParams } from '@/Routes/routes';
import { AppAlerts } from '@/services/alerts/appAlerts';
import { findTargetWorkspace } from '@/services/helpers/factoryFlow/findTargetWorkspace';
import { AlertItem, DevWorkspaceStatus, LoaderTab } from '@/services/helpers/types';
import { Workspace, WorkspaceAdapter } from '@/services/workspace-adapter';
import { AppState } from '@/store';
import { selectApplications } from '@/store/ClusterInfo/selectors';
import { selectStartTimeout } from '@/store/ServerConfig/selectors';
import * as WorkspaceStore from '@/store/Workspaces';
import { selectDevWorkspaceWarnings } from '@/store/Workspaces/devWorkspaces/selectors';
import { selectAllWorkspaces } from '@/store/Workspaces/selectors';

export type Props = MappedProps &
Expand All @@ -47,15 +50,20 @@ export type Props = MappedProps &
export type State = ProgressStepState & {
shouldStart: boolean; // should the loader start a workspace?
shouldUpdateWithDefaultDevfile: boolean;
warning: string | undefined;
};

class StartingStepStartWorkspace extends ProgressStep<Props, State> {
protected readonly name = 'Waiting for workspace to start';

@lazyInject(AppAlerts)
private readonly appAlerts: AppAlerts;

constructor(props: Props) {
super(props);

this.state = {
warning: undefined,
shouldStart: true,
name: this.name,
shouldUpdateWithDefaultDevfile: false,
Expand Down Expand Up @@ -112,6 +120,15 @@ class StartingStepStartWorkspace extends ProgressStep<Props, State> {
return true;
}

if (
workspace !== undefined &&
nextWorkspace !== undefined &&
this.props.devWorkspaceWarnings[workspace.uid] !==
nextProps.devWorkspaceWarnings[nextWorkspace.uid]
) {
return true;
}

return false;
}

Expand All @@ -132,6 +149,15 @@ class StartingStepStartWorkspace extends ProgressStep<Props, State> {
});
}

if (workspace !== undefined) {
const warning = this.props.devWorkspaceWarnings[workspace.uid];
if (warning) {
this.setState({
warning,
});
}
}

this.prepareAndRun();
}

Expand Down Expand Up @@ -169,6 +195,15 @@ class StartingStepStartWorkspace extends ProgressStep<Props, State> {
);
}

if (this.state.warning !== undefined) {
this.appAlerts.showAlert({
key: 'start-workspace-warning',
title: `WARNING: ${this.state.warning}`,
variant: AlertVariant.warning,
});
return true;
}

if (this.state.shouldUpdateWithDefaultDevfile) {
await this.props.updateWorkspaceWithDefaultDevfile(workspace);
this.setState({ shouldUpdateWithDefaultDevfile: false });
Expand Down Expand Up @@ -298,6 +333,7 @@ const mapStateToProps = (state: AppState) => ({
allWorkspaces: selectAllWorkspaces(state),
applications: selectApplications(state),
startTimeout: selectStartTimeout(state),
devWorkspaceWarnings: selectDevWorkspaceWarnings(state),
});

const connector = connect(mapStateToProps, WorkspaceStore.actionCreators, null, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,29 @@ describe('SshPassphrase', () => {
});

describe('text area', () => {
it('should handle SSH passphrase', () => {
it('should handle SSH passphrase', async () => {
renderComponent();

expect(mockOnChange).not.toHaveBeenCalled();

const input = screen.getByPlaceholderText('Enter passphrase (optional)');

const passphrase = 'passphrase';
userEvent.paste(input, passphrase);
await userEvent.click(input);
await userEvent.paste(passphrase);

expect(mockOnChange).toHaveBeenCalledWith(passphrase);
});

it('should handle the empty value', () => {
it('should handle the empty value', async () => {
renderComponent();

expect(mockOnChange).not.toHaveBeenCalled();

const input = screen.getByPlaceholderText('Enter passphrase (optional)');

// fill the SSH key data field
userEvent.paste(input, 'ssh-key-data');
await userEvent.click(input);
await userEvent.paste('ssh-key-data');

mockOnChange.mockClear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ describe('OAuth service', () => {
try {
await OAuthService.refreshTokenIfProjectExists(devWorkspace);
} catch (e: any) {
fail('it should not reach here');
// ignore
}

expect(refreshFactoryOauthTokenSpy).toHaveBeenCalledWith('origin:project');
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('OAuth service', () => {
try {
await OAuthService.refreshTokenIfProjectExists(devWorkspace);
} catch (e: any) {
fail('it should not reach here');
// ignore
}

expect(refreshFactoryOauthTokenSpy).toHaveBeenCalledWith('origin:project');
Expand Down
4 changes: 1 addition & 3 deletions packages/dashboard-frontend/src/services/oauth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,8 @@ export class OAuthService {
response.data.attributes.oauth_authentication_url,
redirectUrl.toString(),
);
// Interrupt the workspace start. The workspace should start again after the authentication.
throw e;
}
// Skip other exceptions to proceed the workspace start.
throw e;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

/* eslint-disable @typescript-eslint/no-unused-vars */

import { api } from '@eclipse-che/common';
import common, { api } from '@eclipse-che/common';
import { V1Status } from '@kubernetes/client-node';
import { dump } from 'js-yaml';
import { AnyAction } from 'redux';
Expand All @@ -21,6 +21,7 @@ import { ThunkDispatch } from 'redux-thunk';

import { container } from '@/inversify.config';
import { isRunningDevWorkspacesClusterLimitExceeded } from '@/services/backend-client/devWorkspaceClusterApi';
import * as factoryApi from '@/services/backend-client/factoryApi';
import { fetchServerConfig } from '@/services/backend-client/serverConfigApi';
import { WebsocketClient } from '@/services/backend-client/websocketClient';
import devfileApi from '@/services/devfileApi';
Expand Down Expand Up @@ -145,6 +146,8 @@ const mockOnStart = jest.fn();
const mockUpdate = jest.fn();
const mockUpdateAnnotation = jest.fn();

const refreshFactoryOauthTokenSpy = jest.spyOn(factoryApi, 'refreshFactoryOauthToken');

describe('DevWorkspace store, actions', () => {
const devWorkspaceClient = container.get(DevWorkspaceClient);
let storeBuilder: FakeStoreBuilder;
Expand Down Expand Up @@ -490,6 +493,61 @@ describe('DevWorkspace store, actions', () => {

expect(actions).toStrictEqual(expectedActions);
});
it('should refresh token', async () => {
// given
const projects = [
{
name: 'project',
git: {
remotes: {
origin: 'origin:project',
},
},
},
];
const devWorkspace = new DevWorkspaceBuilder().withProjects(projects).build();
const store = storeBuilder.withDevWorkspaces({ workspaces: [devWorkspace] }).build();
refreshFactoryOauthTokenSpy.mockResolvedValueOnce();

// when
await store.dispatch(testStore.actionCreators.startWorkspace(devWorkspace));

// then
expect(refreshFactoryOauthTokenSpy).toHaveBeenCalledWith('origin:project');
});

it('should dispatch notification on refresh token failure', async () => {
// given
const projects = [
{
name: 'project',
git: {
remotes: {
origin: 'origin:project',
},
},
},
];
const devWorkspace = new DevWorkspaceBuilder().withProjects(projects).build();
const store = storeBuilder.withDevWorkspaces({ workspaces: [devWorkspace] }).build();
const error = {
response: {
data: { attributes: { provider: 'github' }, message: 'test message' },
},
};
jest.spyOn(common.helpers.errors, 'includesAxiosResponse').mockImplementation(() => true);
refreshFactoryOauthTokenSpy.mockRejectedValueOnce(error);

// when
await store.dispatch(testStore.actionCreators.startWorkspace(devWorkspace));

// then
const actions = store.getActions();
expect(actions[0].type).toBe('UPDATE_WARNING');
expect(actions[0].warning).toBe(
"GitHub might not be operational, please check the provider's status page.",
);
});
});

describe('updateWorkspaceWithDefaultDevfile', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import { V1alpha2DevWorkspaceStatus } from '@devfile/api';
import common, { api, ApplicationId } from '@eclipse-che/common';
import { includesAxiosResponse } from '@eclipse-che/common/lib/helpers/errors';
import { dump } from 'js-yaml';
import { Action, Reducer } from 'redux';

Expand Down Expand Up @@ -312,6 +313,36 @@ export const actionCreators: ActionCreators = {
}
try {
await OAuthService.refreshTokenIfProjectExists(workspace);
} catch (e: unknown) {
if (includesAxiosResponse(e)) {
// Do not interrupt the workspace start, but show a warning notification.
const response = e.response;
const attributes = response.data.attributes;
let message = response.data.message;
let provider = '';
if (attributes !== undefined && attributes.provider !== undefined) {
const providerAttribute: string = attributes.provider;
if (providerAttribute.startsWith('github')) {
provider = 'GitHub';
} else if (providerAttribute.startsWith('gitlab')) {
provider = 'Gitlab';
} else if (providerAttribute.startsWith('bitbucket')) {
provider = 'Bitbucket';
}
}
if (provider.length > 0) {
// eslint-disable-next-line no-warning-comments
// TODO add status page url for each provider when https://github.com/eclipse-che/che/issues/23142 is fixed
message = `${provider} might not be operational, please check the provider's status page.`;
}
dispatch({
type: Type.UPDATE_WARNING,
workspace: workspace,
warning: message,
});
}
}
try {
await dispatch(
DevWorkspacesCluster.actionCreators.requestRunningDevWorkspacesClusterLimitExceeded(),
);
Expand Down

0 comments on commit 6916e23

Please sign in to comment.