Improve error handling when parsing ansible-lint output (#11)

tomaciazek · Jul 27, 2021 · d79cf7f · d79cf7f
1 parent fe55b5c
commit d79cf7f
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,11 @@ All notable changes to the Ansible VS Code extension will be documented in this
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [1.0.3] - 2021-07-27
+### Changed
+- Error handling in case output from Ansible Lint can't be parsed is now more
+  informative. Contextual information is now logged in `Ansible Server` output.
+
 ## [1.0.2] - 2021-07-19
 ### Fixed
 - Modules from pre-installed Ansible collections will now be resolved when using

diff --git a/package.json b/package.json
@@ -5,7 +5,7 @@
   "description": "Ansible language support",
   "author": "Tomasz Maciążek",
   "license": "MIT",
-  "version": "1.0.2",
+  "version": "1.0.3",
   "repository": {
     "type": "git",
     "url": "https://github.com/tomaciazek/vscode-ansible.git"

diff --git a/server/src/services/ansibleLint.ts b/server/src/services/ansibleLint.ts
@@ -1,7 +1,6 @@
 import * as child_process from 'child_process';
 import { ExecException } from 'child_process';
 import { promises as fs } from 'fs';
-import * as path from 'path';
 import { URL } from 'url';
 import { promisify } from 'util';
 import {
@@ -108,15 +107,11 @@ export class AnsibleLint {
           stderr: string;
         };
         if (execError.code === 2) {
-          try {
-            diagnostics = this.processReport(
-              execError.stdout,
-              await ansibleLintConfigPromise,
-              workingDirectory
-            );
-          } catch (error) {
-            this.connection.window.showErrorMessage(execError.message);
-          }
+          diagnostics = this.processReport(
+            execError.stdout,
+            await ansibleLintConfigPromise,
+            workingDirectory
+          );
         } else {
           this.connection.window.showErrorMessage(execError.message);
         }
@@ -143,70 +138,96 @@ export class AnsibleLint {
     workingDirectory: string
   ): Map<string, Diagnostic[]> {
     const diagnostics: Map<string, Diagnostic[]> = new Map();
-    const report = JSON.parse(result);
-    if (report instanceof Array) {
-      for (const item of report) {
-        if (
-          typeof item.check_name === 'string' &&
-          item.location &&
-          typeof item.location.path === 'string' &&
-          item.location.lines &&
-          (item.location.lines.begin ||
-            typeof item.location.lines.begin === 'number')
-        ) {
-          const begin_line =
-            item.location.lines.begin.line || item.location.lines.begin || 1;
-          const begin_column = item.location.lines.begin.column || 1;
-          const start: Position = {
-            line: begin_line - 1,
-            character: begin_column - 1,
-          };
-          const end: Position = {
-            line: begin_line - 1,
-            character: Number.MAX_SAFE_INTEGER,
-          };
-          const range: Range = {
-            start: start,
-            end: end,
-          };
-
-          let severity: DiagnosticSeverity = DiagnosticSeverity.Error;
-          if (ansibleLintConfig) {
-            const lintRuleName = (item.check_name as string).match(
-              /\[(?<name>[a-z\-]+)\].*/
-            )?.groups?.name;
+    if (!result) {
+      this.connection.console.warn(
+        'Standard output from ansible-lint is suspiciously empty.'
+      );
+      return diagnostics;
+    }
+    try {
+      const report = JSON.parse(result);
+      if (report instanceof Array) {
+        for (const item of report) {
+          if (
+            typeof item.check_name === 'string' &&
+            item.location &&
+            typeof item.location.path === 'string' &&
+            item.location.lines &&
+            (item.location.lines.begin ||
+              typeof item.location.lines.begin === 'number')
+          ) {
+            const begin_line =
+              item.location.lines.begin.line || item.location.lines.begin || 1;
+            const begin_column = item.location.lines.begin.column || 1;
+            const start: Position = {
+              line: begin_line - 1,
+              character: begin_column - 1,
+            };
+            const end: Position = {
+              line: begin_line - 1,
+              character: Number.MAX_SAFE_INTEGER,
+            };
+            const range: Range = {
+              start: start,
+              end: end,
+            };
 
-            if (lintRuleName && ansibleLintConfig.warnList.has(lintRuleName)) {
-              severity = DiagnosticSeverity.Warning;
-            }
+            let severity: DiagnosticSeverity = DiagnosticSeverity.Error;
+            if (ansibleLintConfig) {
+              const lintRuleName = (item.check_name as string).match(
+                /\[(?<name>[a-z\-]+)\].*/
+              )?.groups?.name;
 
-            const categories = item.categories;
-            if (categories instanceof Array) {
-              if (categories.some((c) => ansibleLintConfig.warnList.has(c))) {
+              if (
+                lintRuleName &&
+                ansibleLintConfig.warnList.has(lintRuleName)
+              ) {
                 severity = DiagnosticSeverity.Warning;
               }
+
+              const categories = item.categories;
+              if (categories instanceof Array) {
+                if (categories.some((c) => ansibleLintConfig.warnList.has(c))) {
+                  severity = DiagnosticSeverity.Warning;
+                }
+              }
             }
-          }
 
-          const locationUri = `file://${workingDirectory}/${item.location.path}`;
+            const locationUri = `file://${workingDirectory}/${item.location.path}`;
 
-          let fileDiagnostics = diagnostics.get(locationUri);
-          if (!fileDiagnostics) {
-            fileDiagnostics = [];
-            diagnostics.set(locationUri, fileDiagnostics);
-          }
-          let message: string = item.check_name;
-          if (item.description) {
-            message += `\nDescription: ${item.description}`;
+            let fileDiagnostics = diagnostics.get(locationUri);
+            if (!fileDiagnostics) {
+              fileDiagnostics = [];
+              diagnostics.set(locationUri, fileDiagnostics);
+            }
+            let message: string = item.check_name;
+            if (item.description) {
+              message += `\nDescription: ${item.description}`;
+            }
+            fileDiagnostics.push({
+              message: message,
+              range: range || Range.create(0, 0, 0, 0),
+              severity: severity,
+              source: 'Ansible',
+            });
           }
-          fileDiagnostics.push({
-            message: message,
-            range: range || Range.create(0, 0, 0, 0),
-            severity: severity,
-            source: 'Ansible',
-          });
         }
       }
+    } catch (error) {
+      this.connection.window.showErrorMessage(
+        'Could not parse ansible-lint output. Please check your ansible-lint installation & configuration.' +
+          ' More info in `Ansible Server` output.'
+      );
+      let message: string;
+      if (error instanceof Error) {
+        message = error.message;
+      } else {
+        message = JSON.stringify(error);
+      }
+      this.connection.console.error(
+        `Exception while parsing ansible-lint output: ${message}` +
+          `\nTried to parse the following:\n${result}`
+      );
     }
     return diagnostics;
   }