Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions extensions/ql-vscode/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion extensions/ql-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1179,7 +1179,7 @@
"ts-loader": "^8.1.0",
"ts-node": "^8.3.0",
"ts-protoc-gen": "^0.9.0",
"typescript": "^4.3.2",
"typescript": "^4.5.5",
"typescript-formatter": "^7.2.2",
"vsce": "^1.65.0",
"vscode-test": "^1.4.0",
Expand Down
3 changes: 2 additions & 1 deletion extensions/ql-vscode/src/cli-version.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as semver from 'semver';
import { runCodeQlCliCommand } from './cli';
import { Logger } from './logging';
import { getErrorMessage } from './pure/helpers-pure';

/**
* Get the version of a CodeQL CLI.
Expand All @@ -18,7 +19,7 @@ export async function getCodeQlCliVersion(codeQlPath: string, logger: Logger): P
} catch (e) {
// Failed to run the version command. This might happen if the cli version is _really_ old, or it is corrupted.
// Either way, we can't determine compatibility.
void logger.log(`Failed to run 'codeql version'. Reason: ${e.message}`);
void logger.log(`Failed to run 'codeql version'. Reason: ${getErrorMessage(e)}`);
return undefined;
}
}
12 changes: 6 additions & 6 deletions extensions/ql-vscode/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { CancellationToken, Disposable, Uri } from 'vscode';
import { BQRSInfo, DecodedBqrsChunk } from './pure/bqrs-cli-types';
import { CliConfig } from './config';
import { DistributionProvider, FindDistributionResultKind } from './distribution';
import { assertNever } from './pure/helpers-pure';
import { assertNever, getErrorMessage, getErrorStack } from './pure/helpers-pure';
import { QueryMetadata, SortDirection } from './pure/interface-types';
import { Logger, ProgressReporter } from './logging';
import { CompilationMessage } from './pure/messages';
Expand Down Expand Up @@ -346,7 +346,7 @@ export class CodeQLCliServer implements Disposable {
stderrBuffers.length == 0
? new Error(`${description} failed: ${err}`)
: new Error(`${description} failed: ${Buffer.concat(stderrBuffers).toString('utf8')}`);
newError.stack += (err.stack || '');
newError.stack += getErrorStack(err);
throw newError;
} finally {
void this.logger.log(Buffer.concat(stderrBuffers).toString('utf8'));
Expand Down Expand Up @@ -448,7 +448,7 @@ export class CodeQLCliServer implements Disposable {
try {
yield JSON.parse(event) as EventType;
} catch (err) {
throw new Error(`Parsing output of ${description} failed: ${err.stderr || err}`);
throw new Error(`Parsing output of ${description} failed: ${(err as any).stderr || getErrorMessage(err)}`);
}
}
}
Expand Down Expand Up @@ -503,7 +503,7 @@ export class CodeQLCliServer implements Disposable {
try {
return JSON.parse(result) as OutputType;
} catch (err) {
throw new Error(`Parsing output of ${description} failed: ${err.stderr || err}`);
throw new Error(`Parsing output of ${description} failed: ${(err as any).stderr || getErrorMessage(err)}`);
}
}

Expand Down Expand Up @@ -751,7 +751,7 @@ export class CodeQLCliServer implements Disposable {
const dot = await this.readDotFiles(interpretedResultsPath);
return dot;
} catch (err) {
throw new Error(`Reading output of interpretation failed: ${err.stderr || err}`);
throw new Error(`Reading output of interpretation failed: ${getErrorMessage(err)}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to give priority to err.stderr here as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that was an error. There is no stderr property on the Error class.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were other places in this file where err.stderr || err is translated to
(err as any).stderr || getErrorMessage(err). Is the difference due to the catch blocks catching different objects? If so, it might be useful to document how we can tell which expression to use where, for the next person who needs to modify this file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting. I am really struggling to find out why err.stderr is being used, or if that property even exists.

I suspect that it is coming from here:

const result = await promisify(child_process.execFile)(codeQlPath, args);

await promisify(child_process.execFile)(...);

returns an object with { stderr, stdout }. It may be that when catching the error from a child process, the stderr property is being read (even though that's a mistake and it doesn't exist). The other locations in the code could just be copies of the first.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have a better handle on what is happening here than I do, so I will let you do what you think is best.

}
}

Expand Down Expand Up @@ -1050,7 +1050,7 @@ export async function runCodeQlCliCommand(
void logger.log('CLI command succeeded.');
return result.stdout;
} catch (err) {
throw new Error(`${description} failed: ${err.stderr || err}`);
throw new Error(`${description} failed: ${(err as any).stderr || getErrorMessage(err)}`);
}
}

Expand Down
19 changes: 11 additions & 8 deletions extensions/ql-vscode/src/commandRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'vscode';
import { showAndLogErrorMessage, showAndLogWarningMessage } from './helpers';
import { logger } from './logging';
import { getErrorMessage, getErrorStack } from './pure/helpers-pure';
import { telemetryListener } from './telemetry';

export class UserCancellationException extends Error {
Expand Down Expand Up @@ -121,8 +122,9 @@ export function commandRunner(
try {
return await task(...args);
} catch (e) {
error = e;
const errorMessage = `${e.message || e} (${commandId})`;
const errorMessage = `${getErrorMessage(e) || e} (${commandId})`;
error = e instanceof Error ? e : new Error(errorMessage);
const errorStack = getErrorStack(e);
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
Expand All @@ -132,8 +134,8 @@ export function commandRunner(
}
} else {
// Include the full stack in the error log only.
const fullMessage = e.stack
? `${errorMessage}\n${e.stack}`
const fullMessage = errorStack
? `${errorMessage}\n${errorStack}`
: errorMessage;
void showAndLogErrorMessage(errorMessage, {
fullMessage
Expand Down Expand Up @@ -173,8 +175,9 @@ export function commandRunnerWithProgress<R>(
try {
return await withProgress(progressOptionsWithDefaults, task, ...args);
} catch (e) {
error = e;
const errorMessage = `${e.message || e} (${commandId})`;
const errorMessage = `${getErrorMessage(e) || e} (${commandId})`;
error = e instanceof Error ? e : new Error(errorMessage);
const errorStack = getErrorStack(e);
if (e instanceof UserCancellationException) {
// User has cancelled this action manually
if (e.silent) {
Expand All @@ -184,8 +187,8 @@ export function commandRunnerWithProgress<R>(
}
} else {
// Include the full stack in the error log only.
const fullMessage = e.stack
? `${errorMessage}\n${e.stack}`
const fullMessage = errorStack
? `${errorMessage}\n${errorStack}`
: errorMessage;
void showAndLogErrorMessage(errorMessage, {
outputLogger,
Expand Down
3 changes: 2 additions & 1 deletion extensions/ql-vscode/src/compare/compare-interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { getHtmlForWebview, jumpToLocation } from '../interface-utils';
import { transformBqrsResultSet, RawResultSet, BQRSInfo } from '../pure/bqrs-cli-types';
import resultsDiff from './resultsDiff';
import { CompletedLocalQueryInfo } from '../query-results';
import { getErrorMessage } from '../pure/helpers-pure';

interface ComparePair {
from: CompletedLocalQueryInfo;
Expand Down Expand Up @@ -70,7 +71,7 @@ export class CompareInterfaceManager extends DisposableObject {
try {
rows = this.compareResults(fromResultSet, toResultSet);
} catch (e) {
message = e.message;
message = getErrorMessage(e);
}

await this.postMessage({
Expand Down
8 changes: 4 additions & 4 deletions extensions/ql-vscode/src/databaseFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { logger } from './logging';
import { tmpDir } from './helpers';
import { Credentials } from './authentication';
import { REPO_REGEX } from './pure/helpers-pure';
import { REPO_REGEX, getErrorMessage } from './pure/helpers-pure';

/**
* Prompts a user to fetch a database from a remote location. Database is assumed to be an archive file.
Expand Down Expand Up @@ -230,7 +230,7 @@ export async function importArchiveDatabase(
}
return item;
} catch (e) {
if (e.message.includes('unexpected end of file')) {
if (getErrorMessage(e).includes('unexpected end of file')) {
throw new Error('Database is corrupt or too large. Try unzipping outside of VS Code and importing the unzipped folder instead.');
} else {
// delegate
Expand Down Expand Up @@ -491,7 +491,7 @@ export async function convertGithubNwoToDatabaseUrl(
return `https://api.github.com/repos/${owner}/${repo}/code-scanning/codeql/databases/${language}`;

} catch (e) {
void logger.log(`Error: ${e.message}`);
void logger.log(`Error: ${getErrorMessage(e)}`);
throw new Error(`Unable to get database for '${githubRepo}'`);
}
}
Expand Down Expand Up @@ -596,7 +596,7 @@ export async function convertLgtmUrlToDatabaseUrl(
language,
].join('/')}`;
} catch (e) {
void logger.log(`Error: ${e.message}`);
void logger.log(`Error: ${getErrorMessage(e)}`);
throw new Error(`Invalid LGTM URL: ${lgtmUrl}`);
}
}
Expand Down
9 changes: 4 additions & 5 deletions extensions/ql-vscode/src/databases-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
promptImportLgtmDatabase,
} from './databaseFetcher';
import { CancellationToken } from 'vscode';
import { asyncFilter } from './pure/helpers-pure';
import { asyncFilter, getErrorMessage } from './pure/helpers-pure';
import { Credentials } from './authentication';

type ThemableIconPath = { light: string; dark: string } | string;
Expand Down Expand Up @@ -393,7 +393,7 @@ export class DatabaseUI extends DisposableObject {
try {
return await this.chooseAndSetDatabase(true, progress, token);
} catch (e) {
void showAndLogErrorMessage(e.message);
void showAndLogErrorMessage(getErrorMessage(e));
return undefined;
}
};
Expand Down Expand Up @@ -461,7 +461,7 @@ export class DatabaseUI extends DisposableObject {
try {
return await this.chooseAndSetDatabase(false, progress, token);
} catch (e) {
void showAndLogErrorMessage(e.message);
void showAndLogErrorMessage(getErrorMessage(e));
return undefined;
}
};
Expand Down Expand Up @@ -622,8 +622,7 @@ export class DatabaseUI extends DisposableObject {
} catch (e) {
// rethrow and let this be handled by default error handling.
throw new Error(
`Could not set database to ${path.basename(uri.fsPath)}. Reason: ${e.message
}`
`Could not set database to ${path.basename(uri.fsPath)}. Reason: ${getErrorMessage(e)}`
);
}
};
Expand Down
7 changes: 4 additions & 3 deletions extensions/ql-vscode/src/databases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { DisposableObject } from './pure/disposable-object';
import { Logger, logger } from './logging';
import { registerDatabases, Dataset, deregisterDatabases } from './pure/messages';
import { QueryServerClient } from './queryserver-client';
import { getErrorMessage } from './pure/helpers-pure';

/**
* databases.ts
Expand Down Expand Up @@ -359,7 +360,7 @@ export class DatabaseItemImpl implements DatabaseItem {
}
catch (e) {
this._contents = undefined;
this._error = e;
this._error = e instanceof Error ? e : new Error(String(e));
throw e;
}
}
Expand Down Expand Up @@ -726,7 +727,7 @@ export class DatabaseManager extends DisposableObject {
}
} catch (e) {
// database list had an unexpected type - nothing to be done?
void showAndLogErrorMessage(`Database list loading failed: ${e.message}`);
void showAndLogErrorMessage(`Database list loading failed: ${getErrorMessage(e)}`);
}
});
}
Expand Down Expand Up @@ -841,7 +842,7 @@ export class DatabaseManager extends DisposableObject {
void logger.log('Deleting database from filesystem.');
fs.remove(item.databaseUri.fsPath).then(
() => void logger.log(`Deleted '${item.databaseUri.fsPath}'`),
e => void logger.log(`Failed to delete '${item.databaseUri.fsPath}'. Reason: ${e.message}`));
e => void logger.log(`Failed to delete '${item.databaseUri.fsPath}'. Reason: ${getErrorMessage(e)}`));
}

// note that we use undefined as the item in order to reset the entire tree
Expand Down
19 changes: 10 additions & 9 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import {
showInformationMessageWithAction,
tmpDir
} from './helpers';
import { assertNever } from './pure/helpers-pure';
import { asError, assertNever, getErrorMessage } from './pure/helpers-pure';
import { spawnIdeServer } from './ide-server';
import { InterfaceManager } from './interface';
import { WebviewReveal } from './interface-utils';
Expand Down Expand Up @@ -489,7 +489,7 @@ async function activateWithInstalledDistribution(
try {
await cmpm.showResults(from, to);
} catch (e) {
void showAndLogErrorMessage(e.message);
void showAndLogErrorMessage(getErrorMessage(e));
}
}

Expand Down Expand Up @@ -541,8 +541,9 @@ async function activateWithInstalledDistribution(
// Note we must update the query history view after showing results as the
// display and sorting might depend on the number of results
} catch (e) {
e.message = `Error running query: ${e.message}`;
item.failureReason = e.message;
const err = asError(e);
err.message = `Error running query: ${err.message}`;
item.failureReason = err.message;
throw e;
} finally {
await qhm.refreshTreeView();
Expand All @@ -567,11 +568,11 @@ async function activateWithInstalledDistribution(
try {
await cliServer.generateQueryHelp(pathToQhelp, absolutePathToMd);
await commands.executeCommand('markdown.showPreviewToSide', uri);
} catch (err) {
const errorMessage = err.message.includes('Generating qhelp in markdown') ? (
} catch (e) {
const errorMessage = getErrorMessage(e).includes('Generating qhelp in markdown') ? (
`Could not generate markdown from ${pathToQhelp}: Bad formatting in .qhelp file.`
) : `Could not open a preview of the generated file (${absolutePathToMd}).`;
void showAndLogErrorMessage(errorMessage, { fullMessage: `${errorMessage}\n${err}` });
void showAndLogErrorMessage(errorMessage, { fullMessage: `${errorMessage}\n${e}` });
}
}

Expand Down Expand Up @@ -694,9 +695,9 @@ async function activateWithInstalledDistribution(
for (const item of quickpick) {
try {
await compileAndRunQuery(false, uri, progress, token, item.databaseItem);
} catch (error) {
} catch (e) {
skippedDatabases.push(item.label);
errors.push(error.message);
errors.push(getErrorMessage(e));
}
}
if (skippedDatabases.length > 0) {
Expand Down
11 changes: 5 additions & 6 deletions extensions/ql-vscode/src/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as cli from './cli';
import { CodeQLCliServer } from './cli';
import { DatabaseEventKind, DatabaseItem, DatabaseManager } from './databases';
import { showAndLogErrorMessage, tmpDir } from './helpers';
import { assertNever } from './pure/helpers-pure';
import { assertNever, getErrorMessage, getErrorStack } from './pure/helpers-pure';
import {
FromResultsViewMsg,
Interpretation,
Expand Down Expand Up @@ -353,8 +353,8 @@ export class InterfaceManager extends DisposableObject {
assertNever(msg);
}
} catch (e) {
void showAndLogErrorMessage(e.message, {
fullMessage: e.stack
void showAndLogErrorMessage(getErrorMessage(e), {
fullMessage: getErrorStack(e)
});
}
}
Expand Down Expand Up @@ -729,7 +729,7 @@ export class InterfaceManager extends DisposableObject {
// If interpretation fails, accept the error and continue
// trying to render uninterpreted results anyway.
void showAndLogErrorMessage(
`Showing raw results instead of interpreted ones due to an error. ${e.message}`
`Showing raw results instead of interpreted ones due to an error. ${getErrorMessage(e)}`
);
}
}
Expand Down Expand Up @@ -768,9 +768,8 @@ export class InterfaceManager extends DisposableObject {
try {
await this.showProblemResultsAsDiagnostics(interpretation, database);
} catch (e) {
const msg = e instanceof Error ? e.message : e.toString();
void this.logger.log(
`Exception while computing problem results as diagnostics: ${msg}`
`Exception while computing problem results as diagnostics: ${getErrorMessage(e)}`
);
this._diagnosticCollection.clear();
}
Expand Down
Loading