✨ feat: improve error handling and messaging in PhpmdService

Author: Morgy93

src/services/phpmd-service.ts

@@ -139,7 +139,17 @@ export class PhpmdService {
 
         // Check if it's a DDEV project
         if (!DdevUtils.isDdevProject(document)) {
-            vscode.window.showErrorMessage('This workspace is not a DDEV project. Please make sure: 1. You are in a DDEV project directory, 2. The .ddev/config.yaml file exists, 3. DDEV is running (use "ddev start")');
+            vscode.window.showErrorMessage(
+                'DDEV environment not detected',
+                {
+                    modal: false,
+                    detail: 'This extension requires a running DDEV environment. Please make sure:\n\n' +
+                            '1. You are in a DDEV project directory\n' +
+                            '2. The .ddev/config.yaml file exists\n' +
+                            '3. DDEV is running (use "ddev start" in terminal)\n\n' +
+                            'If DDEV is already running, try restarting it with "ddev restart".'
+                }
+            );
             return;
         }
 
@@ -161,9 +171,26 @@ export class PhpmdService {
 
             // Process output
             this.processPhpmdOutput(output, document);
-        } catch (error) {
+        } catch (error: any) {
             console.error('Error running PHPMD:', error);
-            vscode.window.showErrorMessage('Error running PHPMD. Make sure PHPMD is installed in your DDEV container.');
+
+            // Show a more detailed error message to the user
+            let errorMessage = error.message || String(error);
+
+            // Cut the message at first occurrence of \n\n
+            const doubleCrlfPosition = errorMessage.indexOf('\n\n');
+            if (doubleCrlfPosition !== -1) {
+                errorMessage = errorMessage.substring(0, doubleCrlfPosition);
+            }
+
+            const detailedMessage = error.stderr
+                ? `PHPMD Error: ${error.stderr.split('\n\n')[0]}`
+                : `Error running PHPMD: ${errorMessage}`;
+
+            vscode.window.showErrorMessage(
+                detailedMessage,
+                { modal: false, detail: 'Make sure PHPMD is installed in your DDEV container.' }
+            );
         }
     }
 
@@ -192,6 +219,9 @@ export class PhpmdService {
      * @param document Document that was analyzed
      */
     private processPhpmdOutput(output: string, document: vscode.TextDocument): void {
+        // Store original output for error reporting
+        const originalOutput = output;
+
         try {
             // Parse output
             const result = JSON.parse(output) as PhpmdResult;
@@ -215,9 +245,32 @@ export class PhpmdService {
             if (diagnostics.length > 0) {
                 this.diagnosticCollection.set(document.uri, diagnostics);
             }
-        } catch (error) {
+        } catch (error: any) {
             console.error('Error processing PHPMD output:', error);
-            vscode.window.showErrorMessage(`Error processing PHPMD output: ${error}`);
+
+            // Extract a readable error message
+            let errorMessage = error.message || String(error);
+
+            // Cut the message at first occurrence of \n\n
+            const doubleCrlfPosition = errorMessage.indexOf('\n\n');
+            if (doubleCrlfPosition !== -1) {
+                errorMessage = errorMessage.substring(0, doubleCrlfPosition);
+            }
+
+            // Show the raw output for debugging if it's available
+            let detail = 'There was a problem processing the PHPMD output.';
+            if (originalOutput) {
+                const shortenedOutput = originalOutput.indexOf('\n\n') !== -1
+                    ? originalOutput.substring(0, originalOutput.indexOf('\n\n'))
+                    : originalOutput.substring(0, 200);
+
+                detail += `\n\nRaw output:\n${shortenedOutput}${originalOutput.length > shortenedOutput.length ? '...' : ''}`;
+            }
+
+            vscode.window.showErrorMessage(
+                `Error processing PHPMD output: ${errorMessage}`,
+                { modal: false, detail: detail }
+            );
         }
     }
 

src/test/extension.test.ts

@@ -327,4 +327,122 @@ suite('Extension Test Suite', () => {
       assert.fail('Second diagnostic should also have code object with target');
     }
   });
+
+  // Test for error message truncation at \n\n
+  test('Error messages are truncated at first \\n\\n occurrence', async () => {
+    // Test error message truncation
+    const truncateErrorMessage = (message: string): string => {
+      const doubleCrlfPosition = message.indexOf('\n\n');
+      if (doubleCrlfPosition !== -1) {
+        return message.substring(0, doubleCrlfPosition);
+      }
+      return message;
+    };
+
+    // Sample error messages
+    const singleLineError = 'Error: PHPMD command failed';
+    const multiLineError = 'Error: PHPMD command failed\nSome additional info';
+    const errorWithDoubleNewline = 'Error: PHPMD command failed\n\nStack trace:\nat line 1\nat line 2';
+    const multipleDoubleNewlines = 'Error: PHPMD command failed\n\nStack trace\n\nMore details';
+
+    // Test truncation
+    assert.strictEqual(
+      truncateErrorMessage(singleLineError),
+      singleLineError,
+      'Single line error should remain unchanged'
+    );
+
+    assert.strictEqual(
+      truncateErrorMessage(multiLineError),
+      multiLineError,
+      'Error with single newlines should remain unchanged'
+    );
+
+    assert.strictEqual(
+      truncateErrorMessage(errorWithDoubleNewline),
+      'Error: PHPMD command failed',
+      'Error with double newline should be truncated at first occurrence'
+    );
+
+    assert.strictEqual(
+      truncateErrorMessage(multipleDoubleNewlines),
+      'Error: PHPMD command failed',
+      'Error with multiple double newlines should be truncated at first occurrence'
+    );
+  });
+
+  // Test for stderr message truncation
+  test('STDERR messages are truncated at first \\n\\n occurrence', async () => {
+    // Test stderr truncation similar to what we're doing in the error handler
+    const truncateStderr = (stderr: string): string => {
+      return stderr.split('\n\n')[0];
+    };
+
+    // Sample stderr outputs
+    const singleLineStderr = 'Command failed with exit code 1';
+    const multiLineStderr = 'Command failed with exit code 1\nAdditional error details';
+    const stderrWithDoubleNewline = 'Command failed with exit code 1\n\nDetailed error information';
+
+    // Test truncation
+    assert.strictEqual(
+      truncateStderr(singleLineStderr),
+      singleLineStderr,
+      'Single line stderr should remain unchanged'
+    );
+
+    assert.strictEqual(
+      truncateStderr(multiLineStderr),
+      multiLineStderr,
+      'Stderr with single newlines should remain unchanged'
+    );
+
+    assert.strictEqual(
+      truncateStderr(stderrWithDoubleNewline),
+      'Command failed with exit code 1',
+      'Stderr with double newline should be truncated at first occurrence'
+    );
+  });
+
+  // Test for raw output truncation
+  test('Raw output is truncated at first \\n\\n or limited to 200 characters', async () => {
+    // Test raw output truncation similar to what we do in processPhpmdOutput error handling
+    const truncateRawOutput = (output: string): string => {
+      const doubleCrlfPosition = output.indexOf('\n\n');
+      if (doubleCrlfPosition !== -1) {
+        return output.substring(0, doubleCrlfPosition);
+      }
+      return output.substring(0, Math.min(output.length, 200));
+    };
+
+    // Sample outputs
+    const shortOutput = 'Valid JSON output';
+    const longOutputNoNewlines = 'x'.repeat(300); // 300 characters with no newlines
+    const outputWithNewlines = 'First part of output\n\nSecond part that should be truncated';
+    const outputWithMultipleDoubleNewlines = 'Start\n\nMiddle\n\nEnd';
+
+    // Test truncation
+    assert.strictEqual(
+      truncateRawOutput(shortOutput),
+      shortOutput,
+      'Short output should remain unchanged'
+    );
+
+    assert.strictEqual(
+      truncateRawOutput(longOutputNoNewlines).length,
+      200,
+      'Long output without newlines should be truncated to 200 characters'
+    );
+
+    assert.strictEqual(
+      truncateRawOutput(outputWithNewlines),
+      'First part of output',
+      'Output with newlines should be truncated at first \\n\\n'
+    );
+
+    assert.strictEqual(
+      truncateRawOutput(outputWithMultipleDoubleNewlines),
+      'Start',
+      'Output with multiple double newlines should be truncated at first occurrence'
+    );
+  });
 });