Fix unit tests to handle JSON parsing errors (#8)

DVC2 · DVC2 · commit 01ac29f09b87 · 2025-04-03T08:03:31.000-05:00
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -21,9 +21,94 @@ jobs:
       - name: Run security tests
         run: node tests/run-tests.js security
 
+  json_parsing_test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Node.js
+        uses: actions/setup-node@v3
+        with:
+          node-version: '16'
+          cache: 'npm'
+      - name: Install dependencies
+        run: npm ci
+      - name: Test JSON parsing error handling
+        run: |
+          # Create test script
+          echo "
+          /**
+           * Simple JSON Parsing Test
+           */
+          
+          const { safeJsonParse } = require('./lib/utils/security-middleware');
+          const { formatJsonResult } = require('./lib/utils/structured-output');
+          
+          let allTestsPassed = true;
+          let testsRun = 0;
+          
+          function runTest(name, testFn) {
+            testsRun++;
+            console.log(\`\n=== Testing \${name} ===\`);
+            try {
+              const passed = testFn();
+              console.log(\`Test passed: \${passed}\`);
+              if (!passed) allTestsPassed = false;
+              return passed;
+            } catch (error) {
+              console.error(\`Test failed: \${error.message}\`);
+              allTestsPassed = false;
+              return false;
+            }
+          }
+          
+          // Test valid JSON
+          runTest('Valid JSON', () => {
+            const validJson = '{\"name\":\"test\",\"value\":42}';
+            const validResult = safeJsonParse(validJson);
+            return validResult.name === 'test' && validResult.value === 42;
+          });
+          
+          // Test empty string
+          runTest('Empty String', () => {
+            const emptyResult = safeJsonParse('', { default: true });
+            return emptyResult.default === true;
+          });
+          
+          // Test malformed JSON
+          runTest('Malformed JSON', () => {
+            const malformedJson = '{name: \"test\"}';
+            const malformedResult = safeJsonParse(malformedJson, { default: true });
+            return malformedResult.default === true;
+          });
+          
+          // Test non-string input
+          runTest('Non-String Input', () => {
+            const nullResult = safeJsonParse(null, { default: true });
+            return nullResult.default === true;
+          });
+          
+          // Test circular references
+          runTest('Circular References', () => {
+            const obj = { name: 'circular' };
+            obj.self = obj; // Create circular reference
+            const circularResult = formatJsonResult(obj);
+            return circularResult.success === true && 
+                   circularResult.data.name === 'circular' && 
+                   typeof circularResult.data.self === 'string';
+          });
+          
+          console.log(\`\n=== Testing Complete ===\`);
+          console.log(\`\${testsRun} tests run, all tests passed: \${allTestsPassed}\`);
+          
+          process.exit(allTestsPassed ? 0 : 1);
+          " > json-parsing-test.js
+          
+          # Run the test
+          node json-parsing-test.js
+
   unit_test:
     runs-on: ubuntu-latest
-    needs: security_test
+    needs: [security_test, json_parsing_test]
     steps:
       - uses: actions/checkout@v3
       - name: Set up Node.js
diff --git a/.gitignore b/.gitignore
@@ -10,6 +10,9 @@
 .tasktracker/stats/
 .tasktracker/cron.log
 .tasktracker/code_health.json
+# Exclude backup files
+.tasktracker/*.backup.*
+.tasktracker/*.corrupted.*
 # But keep the .tasktracker directory itself with a .gitkeep
 !.tasktracker/.gitkeep
 !.tasktracker/reports/.gitkeep
@@ -35,6 +38,12 @@ coverage/
 test-results/
 tests/temp/*
 !tests/temp/.gitkeep
+tests/test-json-parsing/
+test-json-parsing/
+simple-json-test.js
+*-json-test.js
+tests/*-json-test.js
+json-parsing-test.js
 
 # IDE and editor files
 .idea/
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -196,4 +196,12 @@
 - Initial release of TaskTracker
 - Basic task tracking functionality
 - File change detection
-- Simple reporting features 
+- Simple reporting features
+
+## [Unreleased]
+### Added
+- Improved JSON parsing error handling to prevent test failures
+- Enhanced error messages for JSON parsing failures with more context
+- Added support for handling circular references in JSON serialization
+- Created dedicated test suite for JSON parsing functionality
+- Fixed unit tests to handle JSON parsing errors gracefully 
diff --git a/lib/core/formatting.js b/lib/core/formatting.js
@@ -98,37 +98,49 @@ function output(message, type = 'info', options = {}) {
   if (globalOptions.json) {
     let jsonResult;
     
-    switch (type) {
-      case 'error':
-        jsonResult = structuredOutput.formatError(message, {
-          errorCode: options.errorCode || 'ERROR',
-          metadata: options.metadata || {}
-        });
-        break;
-      
-      case 'data':
-        // If it's already a structured result, use it directly
-        if (message && typeof message === 'object' && 'success' in message) {
-          jsonResult = message;
-        } else {
-          jsonResult = structuredOutput.formatJsonResult(message, {
+    try {
+      switch (type) {
+        case 'error':
+          jsonResult = structuredOutput.formatError(message, {
+            errorCode: options.errorCode || 'ERROR',
             metadata: options.metadata || {}
           });
-        }
-        break;
+          break;
         
-      default:
-        // For info, success, warning - create appropriate structured format
-        jsonResult = structuredOutput.formatJsonResult(message, {
-          metadata: { 
-            messageType: type,
-            ...options.metadata
+        case 'data':
+          // If it's already a structured result, use it directly
+          if (message && typeof message === 'object' && 'success' in message) {
+            jsonResult = message;
+          } else {
+            jsonResult = structuredOutput.formatJsonResult(message, {
+              metadata: options.metadata || {}
+            });
           }
-        });
+          break;
+          
+        default:
+          // For info, success, warning - create appropriate structured format
+          jsonResult = structuredOutput.formatJsonResult(message, {
+            metadata: { 
+              messageType: type,
+              ...options.metadata
+            }
+          });
+      }
+      
+      // Ensure the result is valid JSON by checking if it can be stringified
+      const jsonString = JSON.stringify(jsonResult, null, 2);
+      
+      // Output valid JSON
+      console.log(jsonString);
+    } catch (err) {
+      // If JSON conversion fails for any reason, output an error response
+      console.error(JSON.stringify({
+        success: false,
+        error: `Failed to generate JSON output: ${err.message}`,
+        data: null
+      }));
     }
-    
-    // Output JSON
-    console.log(JSON.stringify(jsonResult, null, 2));
     return;
   }
   
diff --git a/lib/utils/security-middleware.js b/lib/utils/security-middleware.js
@@ -203,9 +203,36 @@ function safeFileOperation(operation, filePath, data = null) {
  */
 function safeJsonParse(jsonString, defaultValue = {}) {
   try {
-    return JSON.parse(jsonString);
+    if (typeof jsonString !== 'string') {
+      throw new Error('Input is not a string');
+    }
+    
+    // Trim the input to remove any leading/trailing whitespace
+    const trimmedJson = jsonString.trim();
+    
+    if (trimmedJson === '') {
+      throw new Error('Empty JSON string');
+    }
+    
+    return JSON.parse(trimmedJson);
   } catch (error) {
-    output(`❌ Error parsing JSON: ${error.message}`, 'error');
+    // Provide more specific error details for debugging
+    let errorMsg = `Error parsing JSON: ${error.message}`;
+    
+    // Add position information if available (for SyntaxError)
+    if (error instanceof SyntaxError && 'position' in error) {
+      errorMsg += ` at position ${error.position}`;
+    }
+    
+    // Adding a snippet of the problematic JSON for context
+    if (typeof jsonString === 'string' && jsonString.length > 0) {
+      const snippet = jsonString.length > 50 
+        ? jsonString.substring(0, 47) + '...' 
+        : jsonString;
+      errorMsg += ` | Input snippet: "${snippet}"`;
+    }
+    
+    output(`❌ ${errorMsg}`, 'error');
     return defaultValue;
   }
 }
diff --git a/lib/utils/structured-output.js b/lib/utils/structured-output.js
@@ -5,37 +5,87 @@
  * especially useful for AI agent integration and non-interactive usage.
  */
 
+/**
+ * Safely serialize data, handling non-JSON serializable values
+ * @param {any} data Data to serialize
+ * @returns {any} Safely serializable value
+ */
+function safeSerialize(data) {
+  const seen = new WeakSet();
+  
+  return JSON.parse(JSON.stringify(data, (key, value) => {
+    // Handle undefined
+    if (value === undefined) return null;
+    
+    // Handle functions
+    if (typeof value === 'function') return '[Function]';
+    
+    // Handle error objects
+    if (value instanceof Error) {
+      return {
+        name: value.name,
+        message: value.message,
+        stack: value.stack
+      };
+    }
+    
+    // Handle circular references
+    if (typeof value === 'object' && value !== null) {
+      if (seen.has(value)) {
+        return '[Circular]';
+      }
+      seen.add(value);
+    }
+    
+    return value;
+  }));
+}
+
 /**
  * Format a result into a standardized JSON structure
  * @param {any} data Result data to format
  * @param {object} options Formatting options
  * @returns {object} Standardized JSON object
  */
 function formatJsonResult(data, options = {}) {
-  // Check if data already has a standardized format
-  if (data && typeof data === 'object' && 'success' in data) {
-    // Just ensure consistent structure
+  try {
+    // Check if data already has a standardized format
+    if (data && typeof data === 'object' && 'success' in data) {
+      // Just ensure consistent structure
+      return {
+        success: !!data.success,
+        data: safeSerialize(data.data || data.result || null),
+        error: data.error || null,
+        metadata: {
+          ...safeSerialize(data.metadata || {}),
+          timestamp: new Date().toISOString()
+        }
+      };
+    }
+    
+    // Format data into standardized structure
+    return {
+      success: !options.error, 
+      data: safeSerialize(data),
+      error: options.error || null,
+      metadata: {
+        timestamp: new Date().toISOString(),
+        ...safeSerialize(options.metadata || {})
+      }
+    };
+  } catch (error) {
+    // Provide a fallback if serialization fails
     return {
-      success: !!data.success,
-      data: data.data || data.result || null,
-      error: data.error || null,
+      success: false,
+      data: null,
+      error: `Failed to serialize data: ${error.message}`,
       metadata: {
-        ...data.metadata,
-        timestamp: new Date().toISOString()
+        timestamp: new Date().toISOString(),
+        originalDataType: data === null ? 'null' : typeof data,
+        isArray: Array.isArray(data)
       }
     };
   }
-  
-  // Format data into standardized structure
-  return {
-    success: !options.error, 
-    data: data,
-    error: options.error || null,
-    metadata: {
-      timestamp: new Date().toISOString(),
-      ...options.metadata
-    }
-  };
 }
 
 /**
diff --git a/tests/unit/json-parsing.test.js b/tests/unit/json-parsing.test.js
diff --git a/tests/unit/tasktracker.test.js b/tests/unit/tasktracker.test.js
