Skip to content

feat(functions): add low-level functions.fetch API#141

Merged
netanelgilad merged 10 commits intomainfrom
functions-fetch-streaming-api
Mar 16, 2026
Merged

feat(functions): add low-level functions.fetch API#141
netanelgilad merged 10 commits intomainfrom
functions-fetch-streaming-api

Conversation

@netanelgilad
Copy link
Collaborator

@netanelgilad netanelgilad commented Mar 12, 2026

Summary

  • Add base44.functions.fetch(path, init) for low-level HTTP requests to backend functions with native fetch interface
  • Support all HTTP methods (GET, POST, PUT, PATCH, DELETE, etc.) and custom request configurations
  • Enable streaming responses (SSE, chunked text, NDJSON), custom headers, and access to raw response metadata
  • Automatically propagate authentication headers from the SDK client
  • Refactor to use dependency injection pattern, removing direct axios dependency

Key Features

This PR adds a new functions.fetch method that complements the existing high-level invoke method:

Use Cases

  • Streaming responses: Handle Server-Sent Events (SSE), chunked text, NDJSON, or other streaming formats
  • Custom HTTP methods: Use PUT, PATCH, DELETE, or any other HTTP method beyond POST
  • Custom headers: Set custom request headers or modify default ones
  • Raw response access: Get full response metadata including status codes and headers
  • Direct body control: Send raw request bodies without automatic JSON/FormData transformation

Implementation Details

  • Uses native fetch API under the hood
  • Requests are sent to /api/functions/<path>
  • Authentication headers are automatically included from the SDK client
  • Clean separation from axios with dependency injection pattern

Test Plan

  • npm run test:unit -- tests/unit/functions.test.ts
  • npm run build
  • Unit tests cover direct endpoint routing, authorization header propagation, and various HTTP methods

Made with Cursor

Add a native fetch-based functions.fetch(path, init) method for streaming use cases where invoke() buffers responses. Include JSON data convenience, endpoint fallback compatibility, and unit tests for routing and auth headers.

Made-with: Cursor
@claude
Copy link

claude bot commented Mar 12, 2026

Claude finished @netanelgilad's task —— View job


Code Review Feedback

Review Checklist

  • Read and understand the implementation
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Summary

Overall, this is a well-implemented feature that adds streaming support to the Base44 SDK. The code follows good patterns and includes comprehensive test coverage. I've identified some areas for improvement below.

Code Quality & Best Practices ✅

Strengths:

  • Clean separation of concerns with the new fetch method alongside existing invoke
  • Proper TypeScript typing with FunctionsFetchInit extending RequestInit
  • Good reuse of existing axios configuration (headers, baseURL)
  • Clear JSDoc documentation with examples

Suggestions:

  1. Naming conflict concern: The method name fetch shadows the global fetch function used internally (src/modules/functions.ts:130). Consider renaming to stream or fetchStream to avoid confusion.

  2. Error handling: Both fetch calls should be wrapped in try-catch blocks for better error handling and debugging.

Potential Bugs 🐛

  1. Global fetch reference: At src/modules/functions.ts:130 and :136, you're calling fetch() which will use the global fetch. This could be problematic in Node.js environments or when the global is mocked. Consider injecting fetch or using axios for consistency.

  2. Headers mutation: The toHeaders function creates a new Headers instance, but if the same init object is reused, the headers could accumulate unintentionally.

Performance Considerations 🚀

Good:

  • Efficient fallback mechanism with 404 detection
  • Proper streaming support without buffering

Could improve:

  • The double fetch on 404 adds latency. Consider adding an option to skip fallback for users who know their endpoints exist at the primary path.

Security Concerns 🔒

Good practices:

  • Preserves authentication headers correctly
  • Sets X-App-Id header appropriately
  • No credential leaks in the implementation

Minor concern:

  • No explicit validation of the path parameter. While the URL construction is safe, consider validating that paths don't contain unexpected characters or patterns.

Test Coverage 📋

Excellent coverage including:

  • Direct endpoint routing ✅
  • 404 fallback behavior ✅
  • Authorization header propagation ✅
  • Different content types (JSON, FormData) ✅
  • Edge cases with paths ✅

Missing tests:

  • Error scenarios (network errors, non-404 error responses)
  • Streaming response handling verification
  • Edge cases like empty path or special characters in path
  • Multiple sequential calls to verify no header pollution

Additional Recommendations

  1. Type safety: Consider adding a generic type parameter to fetch for response typing:

    fetch<T = any>(path: string, init?: FunctionsFetchInit): Promise<Response>
  2. Constants: Extract the API path prefixes as constants:

    const FUNCTIONS_PATH_PREFIX = '/functions';
    const APP_FUNCTIONS_PATH_PREFIX = `/apps/${appId}/functions`;
  3. Documentation: Add examples in the JSDoc showing how to handle streaming responses (SSE parsing, chunked reading, etc.)

Verdict

The implementation is solid and ready for use. The suggestions above are mostly for improving maintainability and edge case handling. Great work on adding this streaming capability! 🎉

@github-actions
Copy link

github-actions bot commented Mar 12, 2026

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.21-pr.141.dd114e6

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.21-pr.141.dd114e6"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.21-pr.141.dd114e6"
  }
}

Preview published to npm registry — try new features instantly!

@github-actions github-actions bot added the docs-draft PR has auto-drafted documentation suggestions label Mar 12, 2026
Copy link
Collaborator Author

@netanelgilad netanelgilad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude address the comments

@claude
Copy link

claude bot commented Mar 12, 2026

Claude finished @netanelgilad's task —— View job


Completed Tasks

  • Read CLAUDE.md for repo-specific instructions
  • Read current implementation in src/modules/functions.ts
  • Read test file to understand what tests need updating
  • Remove fallback path logic
  • Remove X-App-Id header
  • Remove content-type handling
  • Update tests to reflect changes
  • Run tests to ensure everything works
  • Commit and push changes

Summary

I've addressed all the review comments:

  1. Removed fallback path logic - The fetch method now only uses the /functions endpoint without any fallback to /apps/${appId}/functions
  2. Removed X-App-Id header - No longer setting this header automatically
  3. Removed content-type handling - The method now acts as a pure fetch wrapper, letting the native fetch handle content types
  4. Updated tests - Modified the test that was checking for fallback behavior to verify it no longer falls back on 404, and removed assertions for X-App-Id and automatic Content-Type headers

All tests are passing and the build is successful. The functions.fetch method is now a simpler wrapper that just adds authorization headers and constructs the proper base URL.

- Remove fallback path logic - only use /functions endpoint
- Remove X-App-Id header - not needed
- Remove automatic content-type handling - let fetch handle it natively
- Update tests to reflect the simplified behavior

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
Copy link
Collaborator Author

@netanelgilad netanelgilad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude address the comments

@claude
Copy link

claude bot commented Mar 12, 2026

Claude finished @netanelgilad's task —— View job


Completed Tasks

  • Read CLAUDE.md for repo-specific instructions (not found)
  • Read current implementation in src/modules/functions.ts
  • Remove body handling/transformation logic
  • Simplify header usage to only use common headers
  • Check if tests need updating
  • Run tests to ensure everything works
  • Run build command
  • Commit and push changes

Summary

I've successfully addressed both review comments:

  1. Removed body handling - The fetch method no longer transforms the body in any way. It acts as a pure wrapper around the native fetch, simply passing through whatever is provided in the init parameter.

  2. Simplified header usage - Now only appending common headers from axios defaults, not post-specific headers.

Additional changes made:

  • Removed the unused isBodyInit helper function
  • Updated the FunctionsFetchInit type to be just RequestInit (no more data property)
  • Updated tests to use body instead of data property
  • Updated JSDoc comments to reflect the changes

All tests are passing and the build is successful. The functions.fetch method is now a simpler, more transparent wrapper around native fetch.

- Remove body handling/transformation logic - fetch now acts as a pure wrapper
- Only append common headers from axios defaults, not post-specific headers
- Remove isBodyInit helper function as it's no longer needed
- Update tests to use body instead of data property
- Update type definitions to use RequestInit directly

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
claude bot and others added 3 commits March 16, 2026 12:11
- Add FunctionsModuleConfig interface with getAuthHeaders and baseURL options
- Pass auth header getter function from client to functions module
- Remove direct axios usage in fetch method for better separation of concerns
- Maintain backward compatibility by making config optional

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
@claude
Copy link

claude bot commented Mar 16, 2026

Claude finished @netanelgilad's task —— View job


Code Review Completed

Review Checklist

  • Read repo-specific guidelines (CLAUDE.md)
  • Read and understand the implementation
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage

Summary

The PR successfully implements a streaming-friendly functions.fetch API. The implementation has evolved through multiple iterations based on review feedback, and the current version is cleaner and more focused. However, there are some critical issues that need to be addressed.

Code Quality & Best Practices ⚠️

Strengths:

  • Clean separation between buffered invoke() and streaming fetch() methods
  • Good TypeScript typing with proper use of native RequestInit
  • Comprehensive JSDoc documentation
  • The refactoring to use dependency injection pattern is excellent

Issues:

  1. Variable shadowing (src/modules/functions.ts:103): The local fetch function call will recursively call itself instead of the global fetch, causing a stack overflow. This is a critical bug.

Potential Bugs 🐛

Critical Bug:

// src/modules/functions.ts:103
const response = await fetch(
  joinBaseUrl(config?.baseURL, primaryPath),
  requestInit
);

This will cause infinite recursion because fetch refers to the module's fetch method, not the global fetch function.

Fix: Use the global fetch explicitly:

const response = await globalThis.fetch(
  joinBaseUrl(config?.baseURL, primaryPath),
  requestInit
);

Performance Considerations 🚀

Good:

  • Direct streaming support without buffering is excellent for performance
  • No unnecessary data transformations
  • Headers are constructed efficiently

Suggestions:

  • Consider memoizing the auth headers getter if it's called frequently

Security Concerns 🔒

Good practices:

  • Authorization headers are properly propagated
  • No credential leaks in the implementation
  • Dependency injection pattern improves security by decoupling auth logic

Minor concerns:

  • Path normalization is good, but consider validating that paths don't contain .. or other potentially dangerous patterns

Test Coverage 📋

Good coverage:

  • Authorization header propagation ✅
  • Different HTTP methods and body types ✅
  • Path normalization ✅
  • 404 behavior (correctly no longer falls back) ✅

Missing tests:

  • The critical recursive fetch bug would have been caught with proper test execution
  • Error scenarios (network errors, non-404 errors)
  • Streaming response handling verification
  • Edge cases with malformed paths
  • The getAuthHeaders error scenarios

Additional Recommendations

  1. Fix the critical fetch bug - This must be fixed before merging:

    const response = await globalThis.fetch(
      joinBaseUrl(config?.baseURL, primaryPath),
      requestInit
    );
  2. Consider adding integration tests that actually test streaming behavior with real fetch calls

  3. Add path validation to prevent potential security issues:

    if (path.includes('..')) {
      throw new Error('Invalid path: Path traversal not allowed');
    }
  4. Document the streaming usage - While the JSDoc mentions it's for streaming, examples showing how to handle SSE, NDJSON, or chunked responses would be helpful

Verdict

The implementation is well-structured and the dependency injection refactoring is excellent. However, the recursive fetch call bug is a blocker that must be fixed before this can be merged. Once that's addressed, this will be a solid addition to the SDK.

@claude
Copy link

claude bot commented Mar 16, 2026

Claude finished @netanelgilad's task —— View job


Code Review for feat(functions): add streaming-friendly functions.fetch API

Review Checklist

  • Read all changed files to understand the implementation
  • Analyze code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Review security concerns
  • Assess test coverage
  • Update comment with comprehensive review feedback

Overall Assessment

The implementation of the streaming-friendly functions.fetch API is well-architected and production-ready. The recent refactoring to remove axios dependency shows good architectural decision-making, resulting in a cleaner separation of concerns.

Code Quality & Best Practices ✅

Strengths:

  • Clean abstraction with FunctionsModuleConfig interface
  • Good use of TypeScript types
  • Consistent with SDK patterns
  • Clear JSDoc documentation
  • Proper error handling with config validation

Minor Suggestions:

  1. The JSDoc comment on line 91 needs the period added (as suggested by @netanelgilad)
  2. Consider adding a more descriptive example in the JSDoc showing actual streaming usage

Architecture & Design 🏗️

The refactoring to remove axios dependency is excellent:

  • Better separation of concerns
  • More flexible and testable
  • Reduces coupling between modules
  • Clean getter pattern for auth headers

Potential Bugs 🐛

Critical Issue: Name shadowing at src/modules/functions.ts:103

const response = await fetch(
  joinBaseUrl(config?.baseURL, primaryPath),
  requestInit
);

The local fetch method shadows the global fetch function. This will cause infinite recursion. Should be:

const response = await globalThis.fetch(
  joinBaseUrl(config?.baseURL, primaryPath),
  requestInit
);

Performance Considerations 🚀

Good:

  • Direct streaming support without buffering
  • No unnecessary transformations
  • Minimal overhead wrapper

Suggestions:

  • Consider memoizing the toHeaders function if the same headers are reused frequently
  • The header extraction in client.ts could be optimized by iterating once instead of using filter+forEach

Security Review 🔒

Excellent practices:

  • Auth headers properly propagated
  • No credential leaks
  • Headers are not mutated directly
  • Proper null/undefined checks

No security issues found.

Test Coverage 📋

Well tested:

  • Direct endpoint routing ✅
  • Authorization header propagation ✅
  • Various HTTP methods ✅
  • 404 behavior (no fallback) ✅

Missing coverage:

  • Error scenarios (network failures, timeouts)
  • Actual streaming response handling
  • Edge cases with malformed paths
  • Headers override scenarios

Summary

This PR successfully adds a clean, streaming-friendly fetch API to the SDK. The removal of axios dependency improves the architecture. The only critical issue is the fetch name shadowing that needs to be fixed before merging. Once that's addressed, this implementation is ready for production use.

Recommendation: Fix the fetch shadowing issue, then approve and merge.

- Updated getAuthHeaders functions in client.ts to compute Authorization headers directly
- User modules now use token from config or getAccessToken() utility
- Service role modules use serviceToken directly
- Fixed double '/api' prefix in functions.ts since baseURL already contains it

This reduces dependency on axios internals in preparation for eventual axios removal.

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
Copy link
Collaborator Author

@netanelgilad netanelgilad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude
Copy link

claude bot commented Mar 16, 2026

Claude finished @netanelgilad's task —— View job


Completed Tasks

  • Read tests/unit/functions.test.ts to understand the tests
  • Remove the test at line 480
  • Simplify the test at line 462 (make it a simple test unrelated to streaming)
  • Run tests to ensure everything passes
  • Run build command
  • Commit and push changes

Summary

I've successfully addressed your review comments:

  1. Removed the test at line 480 - The "should not fallback to app-scoped functions endpoint on 404" test has been completely removed
  2. Simplified the test at line 462 - Changed from "should fetch function endpoint directly for streaming" to "should fetch function endpoint directly", removing all streaming-related references and simplifying the test to just verify basic fetch functionality

All tests are passing (111 tests passed) and the build completed successfully. The changes have been committed and pushed to the branch.

- Remove test for 404 fallback behavior (no longer relevant)
- Simplify streaming test to just test basic fetch functionality

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
Updated the JSDoc comment to describe all use cases for the fetch method,
not just streaming. Now mentions custom HTTP methods, headers, raw response
access, and direct body control as additional reasons to use fetch over invoke.

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
@netanelgilad netanelgilad changed the title feat(functions): add streaming-friendly functions.fetch API feat(functions): add low-level functions.fetch API Mar 16, 2026
@netanelgilad
Copy link
Collaborator Author

@claude

move FunctionsModuleConfig to types file

and address:

  • No test for path without leading slash (e.g., "my_function" vs "/my_function") verifying normalization works — though the auth test at line 486 does use "streaming_demo" without a slash, it only checks headers not the URL.
  • No test for service role auth (base44.asServiceRole.functions.fetch) with a serviceToken.

@claude
Copy link

claude bot commented Mar 16, 2026

Claude finished @netanelgilad's task —— View job


Completed Tasks

  • Read current implementation to understand the structure
  • Move FunctionsModuleConfig from functions.ts to functions.types.ts
  • Update imports in affected files
  • Add test for path normalization (with/without leading slash)
  • Add test for service role auth with serviceToken
  • Run tests to ensure everything works
  • Run build to verify no issues
  • Commit and push changes

Summary

Successfully completed all requested changes:

  1. Moved FunctionsModuleConfig interface from src/modules/functions.ts to src/modules/functions.types.ts where it logically belongs with other type definitions
  2. Added test for path normalization that verifies both /my_function and my_function paths are correctly normalized to /api/functions/my_function
  3. Added test for service role auth that verifies asServiceRole.functions.fetch correctly includes the service token in Authorization header

All 113 tests are passing and the build completed successfully. The changes have been committed and pushed to the branch.

- Moved FunctionsModuleConfig interface from functions.ts to functions.types.ts
- Added test for path normalization (with/without leading slash)
- Added test for service role auth using asServiceRole.functions.fetch

Co-authored-by: Netanel Gilad <netanelgilad@users.noreply.github.com>
@netanelgilad netanelgilad merged commit 7743fe8 into main Mar 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-draft PR has auto-drafted documentation suggestions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants