Skip to content

Commit

Permalink
fix: improve error notifications for better UX
Browse files Browse the repository at this point in the history
  • Loading branch information
f1ames committed Aug 18, 2023
1 parent 8ab0476 commit 35fda66
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 34 deletions.
7 changes: 4 additions & 3 deletions src/commands/bootstrap-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Uri, commands, window } from 'vscode';
import { canRun } from '../utils/commands';
import { getWorkspaceConfig, getWorkspaceFolders } from '../utils/workspace';
import { createDefaultConfigFile } from '../utils/validation';
import { raiseInfo, raiseWarning } from '../utils/errors';
import logger from '../utils/logger';
import type { Folder } from '../utils/workspace';

Expand All @@ -27,17 +28,17 @@ export function getBootstrapConfigurationCommand() {

const currentConfig = await getWorkspaceConfig(folder);
if (currentConfig.type === 'file') {
window.showInformationMessage(`Local '${currentConfig.fileName}' configuration file already exists, opening it.`);
raiseInfo(`Local '${currentConfig.fileName}' configuration file already exists, opening it.`);
return currentConfig.path;
}

if (currentConfig.type === 'config') {
window.showWarningMessage(`Shared '${currentConfig.path}' configuration file already exists, opening it.`);
raiseWarning(`Shared '${currentConfig.path}' configuration file already exists, opening it.`);
return currentConfig.path;
}

if (currentConfig.type === 'remote') {
window.showWarningMessage(`Remote '${currentConfig.fileName}' configuration file already exists, opening it.`);
raiseWarning(`Remote '${currentConfig.fileName}' configuration file already exists, opening it.`);
return currentConfig.path;
}

Expand Down
5 changes: 3 additions & 2 deletions src/commands/synchronize.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { window, commands } from 'vscode';
import { commands } from 'vscode';
import { canRun } from '../utils/commands';
import { COMMANDS, COMMAND_NAMES } from '../constants';
import { raiseWarning } from '../utils/errors';
import globals from '../utils/globals';
import type { RuntimeContext } from '../utils/runtime-context';

Expand All @@ -11,7 +12,7 @@ export function getSynchronizeCommand(context: RuntimeContext) {
}

if (!globals.user.isAuthenticated) {
window.showWarningMessage(`You are not authenticated, cannot synchronize policies. Run ${COMMAND_NAMES.LOGIN} to authenticate first.}`);
raiseWarning(`You are not authenticated, cannot synchronize policies. Run ${COMMAND_NAMES.LOGIN} to authenticate first.}`);
return null;
}

Expand Down
3 changes: 2 additions & 1 deletion src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,6 @@ export const COMMAND_NAMES = {

export const STATUS_BAR_TEXTS = {
DEFAULT: '🔍 Monokle',
VALIDATING: '🤔 Validating...',
VALIDATING: '🤔 Monokle: Validating...',
ERROR: '❌ Monokle: Needs attention...',
};
3 changes: 2 additions & 1 deletion src/utils/commands.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { workspace, window } from 'vscode';
import { SETTINGS } from '../constants';
import { raiseInfo } from './errors';

export function canRun() {
const isEnabled = workspace.getConfiguration(SETTINGS.NAMESPACE).get(SETTINGS.ENABLED);
if (!isEnabled) {
window.showInformationMessage('Monokle is disabled for this workspace. Enable it in the settings.');
raiseInfo('Monokle is disabled for this workspace. Enable it in the settings.');
}

return isEnabled;
Expand Down
16 changes: 4 additions & 12 deletions src/utils/errors.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { MessageOptions, window } from 'vscode';
import { SETTINGS } from '../constants';
import globals from './globals';
import type { WorkspaceFolderConfig, Folder } from './workspace';
import type { WorkspaceFolderConfig } from './workspace';

// Important: Notification promises resolves only after interaction with it (like closing),
// so in most cases we don't want to wait for it to not block rest of the flow.
Expand All @@ -11,8 +11,8 @@ export type ErrorAction = {
callback: () => void | Promise<void>
};

export async function raiseInvalidConfigError(config: WorkspaceFolderConfig, owner: Folder) {
let errorMsg: string;
export function getInvalidConfigError(config: WorkspaceFolderConfig) {
let errorMsg = '';

if (config.type === 'file') {
errorMsg = `Your local configuration file, under '${config.path}' path, is invalid.`;
Expand All @@ -27,15 +27,7 @@ export async function raiseInvalidConfigError(config: WorkspaceFolderConfig, own
errorMsg = `Your remote configuration file from '${globals.remotePolicyUrl}' is invalid.`;
}

if (!errorMsg) {
return null;
}

return raiseError(`${errorMsg} Resources in '${owner.name}' folder will not be validated.`);
}

export async function raiseCannotGetPolicyError(msg: string, actions?: ErrorAction[]) {
return raiseError(`${msg} Remote policy cannot be fetched and resources will not be validated.`, actions);
return errorMsg;
}

export async function raiseError(msg: string, actions: ErrorAction[] = [], options: MessageOptions = {}) {
Expand Down
18 changes: 18 additions & 0 deletions src/utils/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,16 @@ import { workspace } from 'vscode';
import { DEFAULT_REMOTE_POLICY_URL, SETTINGS } from '../constants';
import { getAuthenticator } from './authentication';
import { getSynchronizer } from './synchronization';
import { Folder } from './workspace';

export type FolderStatus = {
valid: boolean;
error?: string;
};

class Globals {
private _storagePath: string = '';
private statuses: Record<string, FolderStatus> = {};
private _authenticator: Awaited<ReturnType<typeof getAuthenticator>> = null;
private _synchronizer: Awaited<ReturnType<typeof getSynchronizer>> = null;

Expand Down Expand Up @@ -52,6 +59,17 @@ class Globals {
return this._synchronizer.getPolicy(path);
}

getFolderStatus(folder: Folder) {
return this.statuses[folder.id];
}

setFolderStatus(folder: Folder, error?: string) {
this.statuses[folder.id] = {
valid: !error,
error,
};
}

setAuthenticator(authenticator: Awaited<ReturnType<typeof getAuthenticator>>) {
this._authenticator = authenticator;
}
Expand Down
47 changes: 45 additions & 2 deletions src/utils/policy-puller.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { rm } from 'fs/promises';
import { getWorkspaceFolders } from './workspace';
import { getSynchronizer } from './synchronization';
import logger from './logger';
Expand Down Expand Up @@ -67,13 +68,55 @@ export class PolicyPuller {
try {
const policy = await this._synchronizer.synchronize(folder.uri.fsPath, globals.user.token);
logger.log('fetchPolicyFiles', policy);
globals.setFolderStatus(folder);
} catch (error) {
logger.error(error);
// @TODO update status bar
const errorDetails = this.getErrorDetails(error);

logger.error('fetchPolicyFiles', error, errorDetails);

if (errorDetails.type === 'NO_PROJECT' || errorDetails.type === 'NO_POLICY') {
const outdatedPolicy = await this._synchronizer.getPolicy(folder.uri.fsPath);

if (outdatedPolicy.path) {
await rm(outdatedPolicy.path, { force: true });
}
}

globals.setFolderStatus(folder, errorDetails.message);
}
}

this._isPulling = false;
this._pullPromise = undefined;
}

private getErrorDetails(err: any) {
const message = err.message || '';

let type = 'UNKNOWN';

if (message.length === 0) {
return {
type,
message: ''
};
}

if (message.includes('Cannot fetch user data')) {
type = 'NO_USER';
}

if (message.includes('does not belong to any project')) {
type = 'NO_PROJECT';
}

if (message.includes('does not have policy defined')) {
type = 'NO_POLICY';
}

return {
type,
message: `${type}: ${message}`
};
}
}
6 changes: 4 additions & 2 deletions src/utils/runtime-context.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

import { STATUS_BAR_TEXTS } from '../constants';
import { getTooltipContent } from './tooltip';
import { getTooltipData } from './tooltip';
import { getAuthenticator } from './authentication';
import { getSynchronizer } from './synchronization';
import type { Disposable, ExtensionContext, StatusBarItem } from 'vscode';
Expand Down Expand Up @@ -58,7 +58,9 @@ export class RuntimeContext {
}

async updateTooltip() {
this._statusBarItem.tooltip = await getTooltipContent();
const tooltipData = await getTooltipData();
this._statusBarItem.text = tooltipData.status === 'ok' ? STATUS_BAR_TEXTS.DEFAULT : STATUS_BAR_TEXTS.ERROR;
this._statusBarItem.tooltip = tooltipData.content;
}

registerDisposables(disposables: Disposable[]) {
Expand Down
52 changes: 46 additions & 6 deletions src/utils/tooltip.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
import { MarkdownString } from 'vscode';
import { getWorkspaceConfig, getWorkspaceFolders } from './workspace';
import { getValidationResult } from './validation';
import { COMMAND_NAMES } from '../constants';
import globals from './globals';
import logger from './logger';

export type TooltipData = {
content: string | MarkdownString;
status: 'ok' | 'error';
};

export function getTooltipContentDefault() {
if (!globals.enabled) {
Expand All @@ -11,18 +18,32 @@ export function getTooltipContentDefault() {
return 'Monokle extension initializing...';
}

export async function getTooltipContent() {
export async function getTooltipData(): Promise<TooltipData> {
if (!globals.enabled) {
return 'Monokle extension disabled';
return {
content: 'Monokle extension disabled',
status: 'ok',
};
}

let isStatusOk = true;

const folders = getWorkspaceFolders();
const folderList = await Promise.all(folders.map(async (folder) => {
const config = await getWorkspaceConfig(folder);
const configType = config.type === 'file' || config.type === 'config' ? 'local' : config.type;

const folderStatus = globals.getFolderStatus(folder);
const errorStatus = folderStatus?.error ?? '';
const fixTip = getFixTip(errorStatus);

logger.log('folderStatus', folderStatus, fixTip);

if (!config.isValid) {
return `**${folder.name}**: invalid ${configType} config`;
isStatusOk = false;

return `**${folder.name}**: invalid ${configType} config` +
(fixTip ? `<br>_Fix suggestion_: ${fixTip}` : '');
}

const validationResult = await getValidationResult(folder.id);
Expand All @@ -46,8 +67,27 @@ export async function getTooltipContent() {
activeUserText = `<hr><br>Logged in as **${globals.user.email}**`;
}

const content = new MarkdownString(`${folderList.join('<br>')}${activeUserText}<hr><br>Show validation panel`);
const content = new MarkdownString(`${folderList.join('<br>')}${activeUserText}<hr><br>Click to show validation panel`);
content.supportHtml = true;

return content;
}
return {
content,
status: isStatusOk ? 'ok' : 'error',
};
}

function getFixTip(err: string) {
if (err.startsWith('NO_USER')) {
return `Try logging again with _${COMMAND_NAMES.LOGIN}_ command.`;
}

if (err.startsWith('NO_PROJECT')) {
return 'Add to project in Monokle Cloud.';
}

if (err.startsWith('NO_POLICY')) {
return 'Add policy in Monokle Cloud related project.';
}

return '';
}
11 changes: 6 additions & 5 deletions src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Uri } from 'vscode';
import { Document } from 'yaml';
import { getWorkspaceConfig, getWorkspaceResources } from './workspace';
import { VALIDATION_FILE_SUFFIX, DEFAULT_CONFIG_FILE_NAME, TMP_POLICY_FILE_SUFFIX } from '../constants';
import { raiseInvalidConfigError } from './errors';
import { getInvalidConfigError } from './errors';
import logger from '../utils/logger';
import globals from './globals';
import type { Folder } from './workspace';
Expand Down Expand Up @@ -67,11 +67,10 @@ export async function validateFolder(root: Folder): Promise<Uri | null> {
const workspaceConfig = await getWorkspaceConfig(root);

if (workspaceConfig.isValid === false) {
if (workspaceConfig.type === 'remote') {
return null; // Error will be already shown by policy puller.
if (workspaceConfig.type !== 'remote') {
// For remote config, error is already send from policy puller.
globals.setFolderStatus(root, getInvalidConfigError(workspaceConfig));
}

raiseInvalidConfigError(workspaceConfig, root);
return null;
}

Expand Down Expand Up @@ -101,6 +100,8 @@ export async function validateFolder(root: Folder): Promise<Uri | null> {

logger.log(root.name, 'resultsFilePath', resultsFilePath);

globals.setFolderStatus(root);

return Uri.file(resultsFilePath);
}

Expand Down

0 comments on commit 35fda66

Please sign in to comment.