Skip to content

Commit

Permalink
Avoid uninteresting user facing errors
Browse files Browse the repository at this point in the history
This change avoids popping up error messages in two cases:

1. When doing test discovery, do not run discovery on non-existant
   directories. Also, if there is an error, print to the log, and do not
   pop up an error window. The reason is that test discovery is a
   background operation and these should not normally cause pop-ups.
2. When looking for orphaned databases, don't pop up an error if the
   storagePath can't be found. This is normal when working in a new,
   single root workspace.
  • Loading branch information
aeisenberg committed Dec 14, 2020
1 parent 5ad4337 commit e11ef0f
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 33 deletions.
37 changes: 22 additions & 15 deletions extensions/ql-vscode/src/databases-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,21 +378,28 @@ export class DatabaseUI extends DisposableObject {

handleRemoveOrphanedDatabases = async (): Promise<void> => {
logger.log('Removing orphaned databases from workspace storage.');
let dbDirs =
// read directory
(await fs.readdir(this.storagePath, { withFileTypes: true }))
// remove non-directories
.filter(dirent => dirent.isDirectory())
// get the full path
.map(dirent => path.join(this.storagePath, dirent.name))
// remove databases still in workspace
.filter(dbDir => {
const dbUri = Uri.file(dbDir);
return this.databaseManager.databaseItems.every(item => item.databaseUri.fsPath !== dbUri.fsPath);
});

// remove non-databases
dbDirs = await asyncFilter(dbDirs, isLikelyDatabaseRoot);
let dbDirs = undefined;

try {
dbDirs =
// read directory
(await fs.readdir(this.storagePath, { withFileTypes: true }))
// remove non-directories
.filter(dirent => dirent.isDirectory())
// get the full path
.map(dirent => path.join(this.storagePath, dirent.name))
// remove databases still in workspace
.filter(dbDir => {
const dbUri = Uri.file(dbDir);
return this.databaseManager.databaseItems.every(item => item.databaseUri.fsPath !== dbUri.fsPath);
});

// remove non-databases
dbDirs = await asyncFilter(dbDirs, isLikelyDatabaseRoot);
} catch (e) {
logger.log(`Not able to find the storage path '${e.message}'. Ignoring request to remove orphaned database directories.`);
return;
}

if (!dbDirs.length) {
logger.log('No orphaned databases found.');
Expand Down
4 changes: 2 additions & 2 deletions extensions/ql-vscode/src/discovery.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DisposableObject } from './vscode-utils/disposable-object';
import { showAndLogErrorMessage } from './helpers';
import { logger } from './logging';

/**
* Base class for "discovery" operations, which scan the file system to find specific kinds of
Expand Down Expand Up @@ -62,7 +62,7 @@ export abstract class Discovery<T> extends DisposableObject {
});

discoveryPromise.catch(err => {
showAndLogErrorMessage(`${this.name} failed. Reason: ${err.message}`);
logger.log(`${this.name} failed. Reason: ${err.message}`);
});

discoveryPromise.finally(() => {
Expand Down
14 changes: 8 additions & 6 deletions extensions/ql-vscode/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,16 @@ export function commandRunner(
try {
return await task(...args);
} catch (e) {
const errorMessage = `${e.message || e} (${commandId})`;
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
logger.log(e.message);
logger.log(errorMessage);
} else {
showAndLogWarningMessage(e.message);
showAndLogWarningMessage(errorMessage);
}
} else {
showAndLogErrorMessage(e.message || e);
showAndLogErrorMessage(errorMessage);
}
return undefined;
}
Expand Down Expand Up @@ -161,15 +162,16 @@ export function commandRunnerWithProgress<R>(
try {
return await withProgress(progressOptionsWithDefaults, task, ...args);
} catch (e) {
const errorMessage = `${e.message || e} (${commandId})`;
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
logger.log(e.message);
logger.log(errorMessage);
} else {
showAndLogWarningMessage(e.message);
showAndLogWarningMessage(errorMessage);
}
} else {
showAndLogErrorMessage(e.message || e);
showAndLogErrorMessage(errorMessage);
}
return undefined;
}
Expand Down
23 changes: 13 additions & 10 deletions extensions/ql-vscode/src/qltest-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Discovery } from './discovery';
import { EventEmitter, Event, Uri, RelativePattern, WorkspaceFolder, env } from 'vscode';
import { MultiFileSystemWatcher } from './vscode-utils/multi-file-system-watcher';
import { CodeQLCliServer } from './cli';
import * as fs from 'fs-extra';

/**
* A node in the tree of tests. This will be either a `QLTestDirectory` or a `QLTestFile`.
Expand Down Expand Up @@ -180,19 +181,21 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
private async discoverTests(): Promise<QLTestDirectory> {
const fullPath = this.workspaceFolder.uri.fsPath;
const name = this.workspaceFolder.name;
const resolvedTests = (await this.cliServer.resolveTests(fullPath))
.filter((testPath) => !QLTestDiscovery.ignoreTestPath(testPath));

const rootDirectory = new QLTestDirectory(fullPath, name);
for (const testPath of resolvedTests) {
const relativePath = path.normalize(path.relative(fullPath, testPath));
const dirName = path.dirname(relativePath);
const parentDirectory = rootDirectory.createDirectory(dirName);
parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
}

rootDirectory.finish();
// Don't try discovery on workspace folders that don't exist on the filesystem
if ((await fs.pathExists(fullPath))) {
const resolvedTests = (await this.cliServer.resolveTests(fullPath))
.filter((testPath) => !QLTestDiscovery.ignoreTestPath(testPath));
for (const testPath of resolvedTests) {
const relativePath = path.normalize(path.relative(fullPath, testPath));
const dirName = path.dirname(relativePath);
const parentDirectory = rootDirectory.createDirectory(dirName);
parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
}

rootDirectory.finish();
}
return rootDirectory;
}

Expand Down

0 comments on commit e11ef0f

Please sign in to comment.