From 513f44b1e1dda4e76dd829554e20e283f2eb1deb Mon Sep 17 00:00:00 2001 From: Discookie Date: Thu, 2 May 2024 07:11:13 +0000 Subject: [PATCH 1/3] Add parsing for CodeChecker log lines --- src/backend/executor/bridge.ts | 14 +-- src/backend/executor/process.ts | 117 ++++++++++++------ src/editor/executor.ts | 14 +-- src/editor/notifications.ts | 18 +-- src/test/executor/executor.functional.test.ts | 11 +- 5 files changed, 110 insertions(+), 64 deletions(-) diff --git a/src/backend/executor/bridge.ts b/src/backend/executor/bridge.ts index 4962516..d46e9aa 100644 --- a/src/backend/executor/bridge.ts +++ b/src/backend/executor/bridge.ts @@ -17,7 +17,7 @@ import { parseShellArgsAndReplaceVariables, replaceVariables } from '../../utils/config'; -import { ProcessStatus, ProcessType, ScheduledProcess } from '.'; +import { ProcessStatusType, ProcessType, ScheduledProcess } from '.'; import { NotificationType } from '../../editor/notifications'; import { Editor } from '../../editor'; @@ -461,7 +461,7 @@ export class ExecutorBridge implements Disposable { process.processStdout((output) => processOutput += output); process.processStatusChange((status) => { - if (status === ProcessStatus.finished) { + if (status.type === ProcessStatusType.finished) { ExtensionApi.metadata.parseCheckerData(processOutput); } }); @@ -504,7 +504,7 @@ export class ExecutorBridge implements Disposable { process.processStdout((output) => processOutput += output); process.processStatusChange((status) => { - if (status === ProcessStatus.finished) { + if (status.type === ProcessStatusType.finished) { ExtensionApi.diagnostics.parseDiagnosticsData(processOutput); } }); @@ -558,9 +558,9 @@ export class ExecutorBridge implements Disposable { process.processStdout((output) => processOutput += output); process.processStatusChange(async (status) => { - switch (status) { - case ProcessStatus.running: return; - case ProcessStatus.finished: + switch (status.type) { + case ProcessStatusType.running: return; + case ProcessStatusType.finished: try { const data = JSON.parse(processOutput) as AnalyzerVersion; @@ -651,7 +651,7 @@ export class ExecutorBridge implements Disposable { } break; - case ProcessStatus.removed: + case ProcessStatusType.removed: if (this.checkedVersion === undefined) { this.checkedVersion = false; } diff --git a/src/backend/executor/process.ts b/src/backend/executor/process.ts index 969ce52..503238d 100644 --- a/src/backend/executor/process.ts +++ b/src/backend/executor/process.ts @@ -3,18 +3,24 @@ import * as os from 'os'; import { quote } from 'shell-quote'; import { Disposable, Event, EventEmitter, ExtensionContext, workspace } from 'vscode'; -export enum ProcessStatus { +export enum ProcessStatusType { notRunning, running, // When added to the execution queue queued, killed, finished, + warning, errored, // When overwritten in the queue with 'replace', or cleared removed, } +export interface ProcessStatus { + type: ProcessStatusType, + reason?: string +} + export enum ProcessType { analyze = 'CodeChecker analyze', checkers = 'CodeChecker checkers', @@ -57,6 +63,7 @@ export class ScheduledProcess implements Disposable { public readonly commandArgs: string[]; private activeProcess?: childProcess.ChildProcess; + private lastLogMessage?: string; /** Contains parameters for the executor. All members are defined. */ public readonly processParameters: ProcessParameters; @@ -85,8 +92,8 @@ export class ScheduledProcess implements Disposable { return this._processStderr.event; } - private _processStatus: ProcessStatus = ProcessStatus.notRunning; - public get processStatus(): ProcessStatus { + private _processStatus: ProcessStatusType = ProcessStatusType.notRunning; + public get processStatus(): ProcessStatusType { return this._processStatus; } @@ -106,6 +113,18 @@ export class ScheduledProcess implements Disposable { if (this.processParameters.forwardStdoutToLogs === undefined) { this.processParameters.forwardStdoutToLogs = !forwardDefaults.includes(processType); } + + const parseLogMessage = (line: string) => { + // Do not store json output as last error + if (line.startsWith('{') || line === '') { + return; + } + + this.lastLogMessage = line; + }; + + this.processStdout(parseLogMessage, this); + this.processStderr(parseLogMessage, this); } dispose() { @@ -113,7 +132,7 @@ export class ScheduledProcess implements Disposable { this.killProcess(); } - this._processStatusChange.fire(ProcessStatus.removed); + this._processStatusChange.fire({ type: ProcessStatusType.removed }); this._processStatusChange.dispose(); this._processStdout.dispose(); @@ -153,7 +172,7 @@ export class ScheduledProcess implements Disposable { this.activeProcess.on('error', (err) => { this._processStderr.fire(`>>> Process '${commonName}' errored: ${err.message}\n`); - this.updateStatus(ProcessStatus.errored); + this.updateStatus(ProcessStatusType.errored); }); // Guaranteed to fire after all datastreams are closed this.activeProcess.on('close', (code: number | null) => { @@ -163,15 +182,15 @@ export class ScheduledProcess implements Disposable { case null: case 0: case 2: - this.updateStatus(ProcessStatus.finished); + this.updateStatus(ProcessStatusType.finished); break; default: - this.updateStatus(ProcessStatus.errored); + this.updateStatus(ProcessStatusType.errored); break; } }); - this.updateStatus(ProcessStatus.running); + this.updateStatus(ProcessStatusType.running); } public killProcess() { @@ -182,28 +201,54 @@ export class ScheduledProcess implements Disposable { this.activeProcess.kill('SIGINT'); this._processStderr.fire('>>> Process killed\n'); - this.updateStatus(ProcessStatus.killed); + this.updateStatus(ProcessStatusType.killed); } - private updateStatus(status: ProcessStatus) { - switch (status) { - case ProcessStatus.running: - if (this._processStatus !== ProcessStatus.running) { - this._processStatus = ProcessStatus.running; - this._processStatusChange.fire(ProcessStatus.running); + private updateStatus(type: ProcessStatusType) { + switch (type) { + case ProcessStatusType.running: + if (this._processStatus !== ProcessStatusType.running) { + this._processStatus = ProcessStatusType.running; + this._processStatusChange.fire({ type: ProcessStatusType.running }); } - break; - case ProcessStatus.removed: + return; + case ProcessStatusType.removed: // dispose() calls killProcess before dispatching this event. - this._processStatusChange.fire(ProcessStatus.removed); - break; - default: - if (this._processStatus === ProcessStatus.running) { - this.activeProcess = undefined; - this._processStatus = ProcessStatus.notRunning; - this._processStatusChange.fire(status); + this._processStatusChange.fire({ type: ProcessStatusType.removed }); + return; + } + + if (this._processStatus === ProcessStatusType.running) { + this.activeProcess = undefined; + this._processStatus = ProcessStatusType.notRunning; + + const lastLogMessage = this.lastLogMessage?.replace(/^\[.+\]/, ''); + const lastLogSeverity = this.lastLogMessage?.match(/^\[(\w+)/)?.[1]; + + if (type === ProcessStatusType.errored) { + this._processStatusChange.fire({ type, reason: lastLogMessage }); + return; + } else if (type !== ProcessStatusType.finished) { + this._processStatusChange.fire({ type }); + return; + } + + // Refine the finished process status based on the last log message + switch (lastLogSeverity) { + case 'CRITICAL': + case 'ERROR': + this._processStatusChange.fire({ type: ProcessStatusType.errored, reason: lastLogMessage }); + break; + case 'WARNING': + this._processStatusChange.fire({ type: ProcessStatusType.warning, reason: lastLogMessage }); + break; + case 'DEBUG': + this._processStatusChange.fire({ type: ProcessStatusType.finished }); + break; + default: + this._processStatusChange.fire({ type: ProcessStatusType.finished, reason: lastLogMessage }); + break; } - break; } } } @@ -253,8 +298,8 @@ export class ExecutorManager implements Disposable { return this._processStderr.event; } - private _processStatus: ProcessStatus = ProcessStatus.notRunning; - public get processStatus(): ProcessStatus { + private _processStatus: ProcessStatusType = ProcessStatusType.notRunning; + public get processStatus(): ProcessStatusType { return this._processStatus; } @@ -273,12 +318,12 @@ export class ExecutorManager implements Disposable { } private updateStatus([status, process]: [ProcessStatus, ScheduledProcess]) { - switch (status) { - case ProcessStatus.removed: + switch (status.type) { + case ProcessStatusType.removed: this._processStatusChange.fire([status, process]); break; - case ProcessStatus.running: - case ProcessStatus.queued: + case ProcessStatusType.running: + case ProcessStatusType.queued: this._processStatusChange.fire([status, process]); break; default: @@ -330,7 +375,7 @@ export class ExecutorManager implements Disposable { switch (method) { case 'replace': for (const entry of namedQueue) { - this.updateStatus([ProcessStatus.removed, entry]); + this.updateStatus([{ type: ProcessStatusType.removed }, entry]); entry.dispose(); } @@ -345,7 +390,7 @@ export class ExecutorManager implements Disposable { break; } - this.updateStatus([ProcessStatus.queued, process]); + this.updateStatus([{ type: ProcessStatusType.queued }, process]); this.queue.set(name, namedQueue); this.startNextProcess(); @@ -359,7 +404,7 @@ export class ExecutorManager implements Disposable { if (namedQueue.some((queueItem) => queueItem.commandLine === process.commandLine)) { if (!silent) { for (const entry of namedQueue.filter((queueItem) => queueItem.commandLine === process.commandLine)) { - this.updateStatus([ProcessStatus.removed, entry]); + this.updateStatus([{ type: ProcessStatusType.removed }, entry]); entry.dispose(); } } @@ -375,7 +420,7 @@ export class ExecutorManager implements Disposable { public clearQueue(name?: string) { if (name) { for (const entry of this.queue.get(name) ?? []) { - this.updateStatus([ProcessStatus.removed, entry]); + this.updateStatus([{ type: ProcessStatusType.removed }, entry]); entry.dispose(); } @@ -383,7 +428,7 @@ export class ExecutorManager implements Disposable { } else { for (const [, queue] of this.queue.entries()) { for (const entry of queue) { - this.updateStatus([ProcessStatus.removed, entry]); + this.updateStatus([{ type: ProcessStatusType.removed }, entry]); entry.dispose(); } } diff --git a/src/editor/executor.ts b/src/editor/executor.ts index ccfa536..5dcef4d 100644 --- a/src/editor/executor.ts +++ b/src/editor/executor.ts @@ -11,7 +11,7 @@ import { } from 'vscode'; import { Editor } from '.'; import { ExtensionApi } from '../backend'; -import { ProcessStatus, ScheduledProcess } from '../backend/executor/process'; +import { ProcessStatus, ProcessStatusType, ScheduledProcess } from '../backend/executor/process'; import { getConfigAndReplaceVariables } from '../utils/config'; import { NotificationType } from './notifications'; @@ -93,23 +93,23 @@ export class ExecutorAlerts { return; } - if (status === ProcessStatus.running) { + if (status.type === ProcessStatusType.running) { this.statusBarItem.text = '$(loading) CodeChecker: analysis in progress...'; this.statusBarItem.show(); return; } - switch (status) { - case ProcessStatus.finished: + switch (status.type) { + case ProcessStatusType.finished: this.statusBarItem.text = '$(testing-passed-icon) CodeChecker: analysis finished'; break; - case ProcessStatus.killed: + case ProcessStatusType.killed: this.statusBarItem.text = '$(testing-failed-icon) CodeChecker: analysis killed'; break; - case ProcessStatus.notRunning: + case ProcessStatusType.notRunning: this.statusBarItem.text = '$(info) CodeChecker: ready'; break; - case ProcessStatus.errored: + case ProcessStatusType.errored: this.statusBarItem.text = '$(testing-error-icon) CodeChecker: analysis errored'; Editor.notificationHandler.showNotification( diff --git a/src/editor/notifications.ts b/src/editor/notifications.ts index cb41433..0d370e8 100644 --- a/src/editor/notifications.ts +++ b/src/editor/notifications.ts @@ -1,6 +1,6 @@ import { Command, ConfigurationChangeEvent, ExtensionContext, commands, window, workspace } from 'vscode'; import { ExtensionApi } from '../backend'; -import { ProcessStatus, ProcessType, ScheduledProcess } from '../backend/executor'; +import { ProcessStatus, ProcessStatusType, ProcessType, ScheduledProcess } from '../backend/executor'; import { SidebarContainer } from '../sidebar'; import { NotificationItem } from '../sidebar/views'; @@ -87,7 +87,7 @@ export class NotificationHandler { } const notification = this.activeNotifications.get(process.commandLine); - if (notification === undefined && status !== ProcessStatus.queued) { + if (notification === undefined && status.type !== ProcessStatusType.queued) { return; } @@ -99,8 +99,8 @@ export class NotificationHandler { }; }; - switch (status) { - case ProcessStatus.queued: { + switch (status.type) { + case ProcessStatusType.queued: { const newNotification = SidebarContainer.notificationView.addNotification( 'browser', makeMessage('added to the process queue'), [ { @@ -118,7 +118,7 @@ export class NotificationHandler { break; } - case ProcessStatus.running: { + case ProcessStatusType.running: { notification!.silentUpdate({ choices: [] }); const newNotification = SidebarContainer.notificationView.addNotification( @@ -136,7 +136,7 @@ export class NotificationHandler { break; } - case ProcessStatus.killed: { + case ProcessStatusType.killed: { notification!.update({ message: makeMessage('was killed'), choices: [] @@ -145,7 +145,7 @@ export class NotificationHandler { break; } - case ProcessStatus.finished: { + case ProcessStatusType.finished: { notification!.update({ message: makeMessage('finished running'), choices: [] @@ -154,7 +154,7 @@ export class NotificationHandler { break; } - case ProcessStatus.errored: { + case ProcessStatusType.errored: { notification!.update({ message: makeMessage('finished with errors'), choices: [{ @@ -166,7 +166,7 @@ export class NotificationHandler { break; } - case ProcessStatus.removed: { + case ProcessStatusType.removed: { notification!.update({ message: makeMessage('removed from the process queue'), type: 'browser', diff --git a/src/test/executor/executor.functional.test.ts b/src/test/executor/executor.functional.test.ts index 1008e23..1fb7ea7 100644 --- a/src/test/executor/executor.functional.test.ts +++ b/src/test/executor/executor.functional.test.ts @@ -2,7 +2,7 @@ import * as assert from 'assert'; import * as path from 'path'; const sinon = require('sinon'); import { ConfigurationTarget, Uri, commands, extensions, workspace } from 'vscode'; -import { ExecutorBridge, ExecutorManager, ProcessStatus, ProcessType } from '../../backend/executor'; +import { ExecutorBridge, ExecutorManager, ProcessStatusType, ProcessType } from '../../backend/executor'; import { CodeCheckerExtension } from '../../extension'; import { STATIC_WORKSPACE_PATH } from '../utils/constants'; import { closeAllTabs, openDocument } from '../utils/files'; @@ -17,13 +17,14 @@ suite('Functional Test: Backend - Executor', () => { const processStatusChange = async () => new Promise((res, rej) => { const disposable = executorManager.processStatusChange(([status, _]) => { - switch (status) { - case ProcessStatus.finished: + switch (status.type) { + case ProcessStatusType.finished: + case ProcessStatusType.warning: disposable.dispose(); res(); return; - case ProcessStatus.errored: - case ProcessStatus.killed: + case ProcessStatusType.errored: + case ProcessStatusType.killed: disposable.dispose(); rej('process not exited cleanly'); return; From 5d1d5fb18be8d3c51cf73c7d0aacb3cc9f163c97 Mon Sep 17 00:00:00 2001 From: Discookie Date: Thu, 2 May 2024 07:56:39 +0000 Subject: [PATCH 2/3] Fix log message parsing --- src/backend/executor/process.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/backend/executor/process.ts b/src/backend/executor/process.ts index 503238d..05d35b6 100644 --- a/src/backend/executor/process.ts +++ b/src/backend/executor/process.ts @@ -114,13 +114,14 @@ export class ScheduledProcess implements Disposable { this.processParameters.forwardStdoutToLogs = !forwardDefaults.includes(processType); } - const parseLogMessage = (line: string) => { - // Do not store json output as last error - if (line.startsWith('{') || line === '') { - return; - } + const parseLogMessage = (stdout: string) => { + // Do not store json output or meta messages as last error + const lines = stdout.split('\n') + .filter((line) => !line.startsWith('{') && !line.startsWith('>') && line !== ''); - this.lastLogMessage = line; + if (lines.length > 0) { + this.lastLogMessage = lines[lines.length - 1]; + } }; this.processStdout(parseLogMessage, this); From e3c139164e06f3c2f7466bcb659ad99f84063b28 Mon Sep 17 00:00:00 2001 From: Discookie Date: Thu, 2 May 2024 08:03:15 +0000 Subject: [PATCH 3/3] Add warning/error reasons to the sidebar --- src/editor/executor.ts | 4 +++- src/editor/notifications.ts | 32 +++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/editor/executor.ts b/src/editor/executor.ts index 5dcef4d..639b3a8 100644 --- a/src/editor/executor.ts +++ b/src/editor/executor.ts @@ -112,9 +112,11 @@ export class ExecutorAlerts { case ProcessStatusType.errored: this.statusBarItem.text = '$(testing-error-icon) CodeChecker: analysis errored'; + const logLocation = status.reason ? 'sidebar' : 'output log'; + Editor.notificationHandler.showNotification( NotificationType.error, - 'CodeChecker finished with error - see logs for details' + `CodeChecker finished with error - see the ${logLocation} for details` ); break; default: diff --git a/src/editor/notifications.ts b/src/editor/notifications.ts index 0d370e8..2c7dbb5 100644 --- a/src/editor/notifications.ts +++ b/src/editor/notifications.ts @@ -99,6 +99,22 @@ export class NotificationHandler { }; }; + const makeReason = (): (Command)[] => { + if (!status.reason) { + return []; + } + + return [{ + title: `Reason: ${status.reason}`, + command: 'codechecker.executor.showOutput', + tooltip: `Reason: ${status.reason}\nSee the output log for full details` + }, + { + title: 'Show process logs', + command: 'codechecker.executor.showOutput' + }]; + }; + switch (status.type) { case ProcessStatusType.queued: { const newNotification = SidebarContainer.notificationView.addNotification( @@ -148,7 +164,16 @@ export class NotificationHandler { case ProcessStatusType.finished: { notification!.update({ message: makeMessage('finished running'), - choices: [] + choices: makeReason() + }); + this.activeNotifications.delete(process.commandLine); + + break; + } + case ProcessStatusType.warning: { + notification!.update({ + message: makeMessage('finished with warnings'), + choices: makeReason() }); this.activeNotifications.delete(process.commandLine); @@ -157,10 +182,7 @@ export class NotificationHandler { case ProcessStatusType.errored: { notification!.update({ message: makeMessage('finished with errors'), - choices: [{ - title: 'Show process logs', - command: 'codechecker.executor.showOutput' - }] + choices: makeReason() }); this.activeNotifications.delete(process.commandLine);