Skip to content

The library uses error.code instead of error.exitCode #38

@konard

Description

@konard

🐛 Bug Description

command-stream uses non-standard error property naming conventions that break compatibility with Node.js error handling patterns. The library uses error.code instead of the standard error.exitCode property, causing confusion and requiring developers to rewrite their error handling logic.

🔴 Impact

This inconsistency affects:

  • Migration efforts - Code from other libraries won't work without modification
  • Developer experience - Violates principle of least surprise
  • Code portability - Scripts can't be easily moved between different command execution libraries
  • Learning curve - Developers familiar with Node.js patterns need to learn custom conventions

📝 Detailed Problem Analysis

Standard Node.js Pattern (Expected)

// Standard Node.js child_process error handling
try {
  await someCommand();
} catch (error) {
  if (error.exitCode === 1) {  // ✅ Standard property
    // Handle specific exit code
  }
}

command-stream Pattern (Actual)

// command-stream's non-standard approach
try {
  await $\`ls /nonexistent\`;
} catch (error) {
  if (error.code === 1) {  // ⚠️ Non-standard property name
    // Handle specific exit code
  }
  // error.exitCode is undefined! 
}

🔄 Reproduction Steps

const { $ } = await use('command-stream');

try {
  // Execute a command that will fail with exit code 1
  await $\`ls /nonexistent/directory/that/does/not/exist\`;
} catch (error) {
  console.log('Standard property (exitCode):', error.exitCode); // undefined ❌
  console.log('command-stream property (code):', error.code);    // 1 ✅
  
  // This breaks standard error handling patterns!
}

✅ Expected Behavior

The error object should follow Node.js conventions:

  • error.exitCode should contain the process exit code
  • This maintains compatibility with existing Node.js code
  • Follows established patterns from child_process module

❌ Actual Behavior

  • error.exitCode is undefined (breaking standard patterns)
  • error.code contains the exit code (non-standard)
  • Requires custom error handling logic specific to command-stream
  • Breaks compatibility with code expecting standard Node.js errors

🔧 Current Workaround

Developers must remember to use the non-standard property:

// Instead of the standard pattern...
if (error.exitCode === 1) { /* ... */ }

// Must use command-stream specific pattern
if (error.code === 1) { /* ... */ }

🎯 Suggested Fix

Option 1: Add Alias (Backward Compatible) ✅

class CommandError extends Error {
  constructor(code, stdout, stderr) {
    super();
    this.code = code;
    this.exitCode = code;  // Add standard alias
    this.stdout = stdout;
    this.stderr = stderr;
  }
}

Option 2: Full Migration (Breaking Change)

Rename code to exitCode throughout the library, with deprecation period.

📊 Comparison with Other Libraries

Library Exit Code Property Node.js Compatible
Node.js child_process error.exitCode ✅ Native
execa error.exitCode ✅ Yes
cross-spawn error.exitCode ✅ Yes
command-stream error.code ❌ No

🔗 References

📋 Checklist for Fix

  • Add exitCode property as alias for code
  • Update documentation to mention both properties
  • Add deprecation notice if planning to remove code
  • Ensure all error scenarios populate both properties
  • Add tests for backward compatibility
  • Update TypeScript definitions if applicable

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions