Skip to content

Commit

Permalink
fix: improve error log when git repo is faulty [IDE-657] (#538)
Browse files Browse the repository at this point in the history
* fix: improve error log when git repo is faulty

* tidy: small code refactoring

* fix: add errorMessage to the Scan types in unit tests

* tidy: extract hardcoded strings to Snyk Language Server types

* chore: update changelog

* fix: update wording in the settings dialog, to clarify that we only support the git.

* fix: update Net new issues description

* tidy: remove commenented code
  • Loading branch information
acke authored Oct 9, 2024
1 parent ec8491f commit 6d9d113
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [2.19.0]
- Moved delta scan preview setting to settings page.
- New error message in UI when net new scan is done on an invalid repository. Net new scans only work on Git.

## [2.18.2]
- Update Language Server Protocol version to 15.
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@
"snyk.allIssuesVsNetNewIssues": {
"type": "string",
"default": "All issues",
"description": "Specifies whether to see all issues or only net new issues. Only applies to Code Security and Code Quality.",
"description": "Specifies whether to see all issues or only net new issues. Net new issues requires a Git repository, where it compares findings with those in the base branch.",
"enum": [
"All issues",
"Net new issues"
Expand All @@ -180,7 +180,7 @@
"Shows all issues that have been identified, including both new and existing issues.",
"Shows only new issues that have been introduced in the latest scan, filtering out previously known issues."
],
"markdownDescription": "Specifies whether to see all issues or only net new issues. Only applies to Code Security and Code Quality.\n\n",
"markdownDescription": "Specifies whether to see all issues or only net new issues. Net new issues a Git repository, where it compares findings with those in the base branch.\n\n",
"order": 6
},
"snyk.advanced.additionalParameters": {
Expand Down
7 changes: 3 additions & 4 deletions src/snyk/common/editor/codeActionsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ export abstract class CodeActionsProvider<T> implements CodeActionProvider {
return undefined;
}

for (const result of this.issues.entries()) {
const folderPath = result[0];
const issues = result[1];
for (const [folderPath, issues] of this.issues.entries()) {
if (issues instanceof Error || !issues) {
continue;
}
Expand All @@ -36,10 +34,11 @@ export abstract class CodeActionsProvider<T> implements CodeActionProvider {
continue;
}

// returns list of actions, all new actions should be added to this list
// If an issue is found, return the actions
return this.getActions(folderPath, document, issue, range);
}

// If no issues were found after checking all entries, return undefined
return undefined;
}

Expand Down
7 changes: 5 additions & 2 deletions src/snyk/common/languageServer/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,22 @@ export enum LsScanProduct {
Unknown = '',
}

export type InProgress = 'inProgress';

export enum ScanStatus {
InProgress = 'inProgress',
Success = 'success',
Error = 'error',
}

export enum LsErrorMessage {
repositoryInvalidError = 'repository does not exist',
}

export type Scan<T> = {
folderPath: string;
product: ScanProduct;
status: ScanStatus;
issues: Issue<T>[];
errorMessage: string;
};

export type Issue<T> = {
Expand Down
2 changes: 1 addition & 1 deletion src/snyk/common/services/productService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export abstract class ProductService<T> extends AnalysisStatusProvider implement
const issues = this.diagnosticsIssueProvider.getIssuesFromDiagnostics(scanMsg.product);
this._result.set(scanMsg.folderPath, issues);
} else {
this._result.set(scanMsg.folderPath, new Error('Failed to analyze.'));
this._result.set(scanMsg.folderPath, new Error(scanMsg.errorMessage));
}

if (this.runningScanCount <= 0) {
Expand Down
15 changes: 15 additions & 0 deletions src/snyk/common/views/analysisTreeNodeProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,21 @@ export abstract class AnalysisTreeNodeProvider extends TreeNodeProvider {
});
}

protected getFaultyRepositoryErrorTreeNode(scanPath?: string, errorMessage?: string): TreeNode {
return new TreeNode({
icon: NODE_ICONS.error,
text: scanPath ? path.basename(scanPath) : messages.scanFailed,
description: errorMessage,
internal: {
isError: true,
},
command: {
command: SNYK_SHOW_LS_OUTPUT_COMMAND,
title: 'errorMessage',
},
});
}

protected getNoWorkspaceTrustTreeNode(): TreeNode {
return new TreeNode({
text: messages.noWorkspaceTrust,
Expand Down
7 changes: 6 additions & 1 deletion src/snyk/common/views/issueTreeProvider.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _, { flatten } from 'lodash';
import * as vscode from 'vscode'; // todo: invert dependency
import { IConfiguration, IssueViewOptions } from '../../common/configuration/configuration';
import { Issue, IssueSeverity, ScanProduct } from '../../common/languageServer/types';
import { Issue, IssueSeverity, ScanProduct, LsErrorMessage } from '../../common/languageServer/types';

Check warning on line 4 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (ubuntu-latest)

'ScanProduct' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 4 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (macos-latest)

'ScanProduct' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 4 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (windows-latest)

'ScanProduct' is defined but never used. Allowed unused vars must match /^_/u
import { messages as commonMessages } from '../../common/messages/analysisMessages';
import { IContextService } from '../../common/services/contextService';
import { IProductService, ProductService } from '../../common/services/productService';

Check warning on line 7 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (ubuntu-latest)

'ProductService' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 7 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (macos-latest)

'ProductService' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 7 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (windows-latest)

'ProductService' is defined but never used. Allowed unused vars must match /^_/u
Expand Down Expand Up @@ -209,6 +209,11 @@ export abstract class ProductIssueTreeProvider<T> extends AnalysisTreeNodeProvid
const folderName = shortFolderPath.pop() || uri.path;

let folderVulnCount = 0;
if (folderResult instanceof Error && folderResult.message === LsErrorMessage.repositoryInvalidError) {

Check warning on line 212 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (ubuntu-latest)

The two values in this comparison do not have a shared enum type

Check warning on line 212 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (macos-latest)

The two values in this comparison do not have a shared enum type

Check warning on line 212 in src/snyk/common/views/issueTreeProvider.ts

View workflow job for this annotation

GitHub Actions / build / Build and Test (windows-latest)

The two values in this comparison do not have a shared enum type
nodes.push(this.getFaultyRepositoryErrorTreeNode(folderName, folderResult.toString()));
continue;
}

if (folderResult instanceof Error) {
nodes.push(this.getErrorEncounteredTreeNode(folderName));
continue;
Expand Down
8 changes: 7 additions & 1 deletion src/snyk/snykOss/providers/ossVulnerabilityTreeProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { IConfiguration } from '../../common/configuration/configuration';
import { configuration } from '../../common/configuration/instance';
import { SNYK_OPEN_ISSUE_COMMAND } from '../../common/constants/commands';
import { SNYK_ANALYSIS_STATUS } from '../../common/constants/views';
import { Issue, IssueSeverity, OssIssueData } from '../../common/languageServer/types';
import { Issue, IssueSeverity, OssIssueData, LsErrorMessage } from '../../common/languageServer/types';
import { IContextService } from '../../common/services/contextService';
import { IProductService } from '../../common/services/productService';
import { IViewManagerService } from '../../common/services/viewManagerService';
Expand Down Expand Up @@ -52,6 +52,12 @@ export default class OssIssueTreeProvider extends ProductIssueTreeProvider<OssIs
const folderName = shortFolderPath.pop() || uri.path;

let folderVulnCount = 0;

if (folderResult instanceof Error && folderResult.message === LsErrorMessage.repositoryInvalidError) {
nodes.push(this.getFaultyRepositoryErrorTreeNode(folderName, folderResult.toString()));
continue;
}

if (folderResult instanceof Error) {
nodes.push(this.getErrorEncounteredTreeNode(folderName));
continue;
Expand Down
9 changes: 9 additions & 0 deletions src/test/unit/common/services/productService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ suite('Product Service', () => {
folderPath: 'test/path',
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, true);
Expand All @@ -86,12 +87,14 @@ suite('Product Service', () => {
folderPath,
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});
ls.scan$.next({
product: ScanProduct.InfrastructureAsCode,
folderPath,
issues: [],
status: ScanStatus.Success,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, false);
Expand All @@ -105,12 +108,14 @@ suite('Product Service', () => {
folderPath,
issues: [],
status: ScanStatus.InProgress,
errorMessage: 'Scan failed',
});
ls.scan$.next({
product: ScanProduct.InfrastructureAsCode,
folderPath,
issues: [],
status: ScanStatus.Error,
errorMessage: 'Scan failed',
});

strictEqual(service.isAnalysisRunning, false);
Expand All @@ -125,18 +130,21 @@ suite('Product Service', () => {
folderPath: folder1Path,
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});
ls.scan$.next({
product: ScanProduct.InfrastructureAsCode,
folderPath: folder2Path,
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});
ls.scan$.next({
product: ScanProduct.InfrastructureAsCode,
folderPath: folder1Path,
issues: [],
status: ScanStatus.Success,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, true);
Expand All @@ -146,6 +154,7 @@ suite('Product Service', () => {
folderPath: folder2Path,
issues: [],
status: ScanStatus.Success,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, false);
Expand Down
1 change: 1 addition & 0 deletions src/test/unit/snykCode/codeService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ suite('Code Service', () => {
folderPath: 'test/path',
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, false);
Expand Down
1 change: 1 addition & 0 deletions src/test/unit/snykIac/iacService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ suite('IaC Service', () => {
folderPath: 'test/path',
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, false);
Expand Down
2 changes: 2 additions & 0 deletions src/test/unit/snykOss/ossService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ suite('OSS Service', () => {
folderPath: 'test/path',
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, true);
Expand All @@ -71,6 +72,7 @@ suite('OSS Service', () => {
folderPath: 'test/path',
issues: [],
status: ScanStatus.InProgress,
errorMessage: '',
});

strictEqual(service.isAnalysisRunning, false);
Expand Down

0 comments on commit 6d9d113

Please sign in to comment.