From 6d9d11313b19304fd53fde1edf5bbf4da843d0a8 Mon Sep 17 00:00:00 2001 From: Knut Funkel Date: Wed, 9 Oct 2024 14:10:31 +0200 Subject: [PATCH] fix: improve error log when git repo is faulty [IDE-657] (#538) * 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 --- CHANGELOG.md | 1 + package.json | 4 ++-- src/snyk/common/editor/codeActionsProvider.ts | 7 +++---- src/snyk/common/languageServer/types.ts | 7 +++++-- src/snyk/common/services/productService.ts | 2 +- src/snyk/common/views/analysisTreeNodeProvider.ts | 15 +++++++++++++++ src/snyk/common/views/issueTreeProvider.ts | 7 ++++++- .../providers/ossVulnerabilityTreeProvider.ts | 8 +++++++- .../unit/common/services/productService.test.ts | 9 +++++++++ src/test/unit/snykCode/codeService.test.ts | 1 + src/test/unit/snykIac/iacService.test.ts | 1 + src/test/unit/snykOss/ossService.test.ts | 2 ++ 12 files changed, 53 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56d45bdac..d900d016d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/package.json b/package.json index 9af082e30..a3e4fbc89 100644 --- a/package.json +++ b/package.json @@ -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" @@ -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": { diff --git a/src/snyk/common/editor/codeActionsProvider.ts b/src/snyk/common/editor/codeActionsProvider.ts index 714423520..68b1f394d 100644 --- a/src/snyk/common/editor/codeActionsProvider.ts +++ b/src/snyk/common/editor/codeActionsProvider.ts @@ -24,9 +24,7 @@ export abstract class CodeActionsProvider 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; } @@ -36,10 +34,11 @@ export abstract class CodeActionsProvider 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; } diff --git a/src/snyk/common/languageServer/types.ts b/src/snyk/common/languageServer/types.ts index 8e3a236da..cc53c2f9e 100644 --- a/src/snyk/common/languageServer/types.ts +++ b/src/snyk/common/languageServer/types.ts @@ -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 = { folderPath: string; product: ScanProduct; status: ScanStatus; issues: Issue[]; + errorMessage: string; }; export type Issue = { diff --git a/src/snyk/common/services/productService.ts b/src/snyk/common/services/productService.ts index 96e02559d..374142682 100644 --- a/src/snyk/common/services/productService.ts +++ b/src/snyk/common/services/productService.ts @@ -171,7 +171,7 @@ export abstract class ProductService 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) { diff --git a/src/snyk/common/views/analysisTreeNodeProvider.ts b/src/snyk/common/views/analysisTreeNodeProvider.ts index 264ca87a4..314bb5fe3 100644 --- a/src/snyk/common/views/analysisTreeNodeProvider.ts +++ b/src/snyk/common/views/analysisTreeNodeProvider.ts @@ -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, diff --git a/src/snyk/common/views/issueTreeProvider.ts b/src/snyk/common/views/issueTreeProvider.ts index d16ff42df..f1084c960 100644 --- a/src/snyk/common/views/issueTreeProvider.ts +++ b/src/snyk/common/views/issueTreeProvider.ts @@ -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'; import { messages as commonMessages } from '../../common/messages/analysisMessages'; import { IContextService } from '../../common/services/contextService'; import { IProductService, ProductService } from '../../common/services/productService'; @@ -209,6 +209,11 @@ export abstract class ProductIssueTreeProvider extends AnalysisTreeNodeProvid 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; diff --git a/src/snyk/snykOss/providers/ossVulnerabilityTreeProvider.ts b/src/snyk/snykOss/providers/ossVulnerabilityTreeProvider.ts index c8f810ce4..dfb99b55a 100644 --- a/src/snyk/snykOss/providers/ossVulnerabilityTreeProvider.ts +++ b/src/snyk/snykOss/providers/ossVulnerabilityTreeProvider.ts @@ -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'; @@ -52,6 +52,12 @@ export default class OssIssueTreeProvider extends ProductIssueTreeProvider { folderPath: 'test/path', issues: [], status: ScanStatus.InProgress, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, true); @@ -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); @@ -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); @@ -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); @@ -146,6 +154,7 @@ suite('Product Service', () => { folderPath: folder2Path, issues: [], status: ScanStatus.Success, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, false); diff --git a/src/test/unit/snykCode/codeService.test.ts b/src/test/unit/snykCode/codeService.test.ts index 2befb6070..f5d917817 100644 --- a/src/test/unit/snykCode/codeService.test.ts +++ b/src/test/unit/snykCode/codeService.test.ts @@ -59,6 +59,7 @@ suite('Code Service', () => { folderPath: 'test/path', issues: [], status: ScanStatus.InProgress, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, false); diff --git a/src/test/unit/snykIac/iacService.test.ts b/src/test/unit/snykIac/iacService.test.ts index 7de37b8da..5ed6645e3 100644 --- a/src/test/unit/snykIac/iacService.test.ts +++ b/src/test/unit/snykIac/iacService.test.ts @@ -61,6 +61,7 @@ suite('IaC Service', () => { folderPath: 'test/path', issues: [], status: ScanStatus.InProgress, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, false); diff --git a/src/test/unit/snykOss/ossService.test.ts b/src/test/unit/snykOss/ossService.test.ts index b450be148..9950e7bad 100644 --- a/src/test/unit/snykOss/ossService.test.ts +++ b/src/test/unit/snykOss/ossService.test.ts @@ -59,6 +59,7 @@ suite('OSS Service', () => { folderPath: 'test/path', issues: [], status: ScanStatus.InProgress, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, true); @@ -71,6 +72,7 @@ suite('OSS Service', () => { folderPath: 'test/path', issues: [], status: ScanStatus.InProgress, + errorMessage: '', }); strictEqual(service.isAnalysisRunning, false);