fix: return 500 errors in function invoke instead of throwing

[netanelgilad]

Fixes #14

Summary

• Modified functions.invoke() to catch errors and return them in { data: null, error } format
• Updated all function tests to expect the new { data, error } response format
• Added specific test case for 500 errors to verify they are returned instead of thrown

Test plan

• All unit tests pass (15/15)
• TypeScript build compiles successfully
• New error handling behavior verified with test cases

🤖 Generated with Claude Code

[claude]

Claude finished @netanelgilad's task —— View job

---

Code Review for PR #15

Review Tasks:

• Read and analyze the modified functions.ts file
• Review the updated test file
• Check code quality and best practices
• Identify potential bugs or issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage
• Provide comprehensive feedback

Summary

This PR successfully changes the error handling behavior from throwing exceptions to returning errors in a structured format. The implementation is well-tested and follows good practices.

✅ Positive Aspects

Code Quality & Design

• Consistent API: The new {data, error} return format provides a consistent interface for both success and error cases
• Backward Compatibility: Maintains the existing function signature while changing only the error handling behavior
• Clean Implementation: The try-catch block in src/modules/functions.ts:44-54 is straightforward and handles errors gracefully

Test Coverage

• Comprehensive Testing: Added specific test cases for error scenarios (tests/unit/functions.test.ts:425-449)
• Updated Existing Tests: All 15 test cases properly updated to expect the new response format
• Edge Case Coverage: Tests handle various error codes (500, 404) and different input types

⚠️ Issues & Recommendations

1. Error Information Loss - Medium Priority

Location: src/modules/functions.ts:53

return { data: null, error: error };

Issue: The raw error object from Axios contains sensitive information and may not serialize well.

Recommendation: Extract relevant error information:

return { 
  data: null, 
  error: {
    message: error.response?.data?.message || error.message,
    status: error.response?.status,
    code: error.response?.data?.code
  }
};

2. Inconsistent Error Handling - Low Priority

Location: src/modules/functions.ts:15-17
The string validation still throws an error while HTTP errors are now returned. Consider consistency:

if (typeof data === "string") {
  return { 
    data: null, 
    error: { message: `Function ${functionName} must receive an object...` }
  };
}

3. Type Safety - Low Priority

Consider adding TypeScript interfaces for the return type:

interface FunctionResult<T = any> {
  data: T | null;
  error: any | null;
}

4. Content-Type Header Issue - Low Priority

Location: src/modules/functions.ts:48
The multipart/form-data content type should not be set manually - let the browser/axios handle it for proper boundary generation.

🚀 Performance Considerations

• Minimal Impact: Error handling change has negligible performance impact
• Memory Usage: Error objects may contain large response data - consider limiting what's stored

🔒 Security Assessment

• No New Vulnerabilities: The change doesn't introduce security issues
• Error Exposure: Consider whether full error details should be exposed to calling code

✅ Test Coverage Assessment

• Excellent Coverage: 15 comprehensive test cases covering:
  • Success scenarios with various data types
  • Error scenarios (404, 500)
  • File uploads and FormData handling
  • Edge cases (null/undefined values, arrays)

🎯 Recommendation: APPROVE

This PR successfully implements the requested error handling change with good test coverage. The suggested improvements above are minor and can be addressed in follow-up PRs if desired.

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i https://pkg.pr.new/base44dev/javascript-sdk/@base44/sdk@6e5f730cd2add298a92cfce430c98dafebf739a2

• 📦 Preview Package on pkg.pr.new
• 🔗 View this commit on GitHub

---

_{Preview powered by pkg.pr.new — try new features instantly, no NPM release needed!}