Skip to content

fix(functions): use app-id-based path for functions.fetch()#147

Closed
netanelgilad wants to merge 1 commit intomainfrom
fix/functions-fetch-use-app-id-path
Closed

fix(functions): use app-id-based path for functions.fetch()#147
netanelgilad wants to merge 1 commit intomainfrom
fix/functions-fetch-use-app-id-path

Conversation

@netanelgilad
Copy link
Collaborator

Summary

  • functions.fetch() was using the domain-based path (/api/functions/{path}) which relies on the host header to resolve the app. This fails against non-production servers (deploy previews, staging) where the host doesn't map to any app.
  • Changed to the app-id-based path (/api/apps/{appId}/functions/{path}), matching the pattern invoke() already uses. This makes functions.fetch() work correctly regardless of which server URL is configured.

Test plan

  • All 19 functions unit tests updated and passing
  • All 116 unit tests passing across 10 test files
  • Verified end-to-end in kitchensink app against apper deploy preview (PR #5232) — Hono multi-route demo, SSE streaming, and NDJSON streaming all work

Made with Cursor

functions.fetch() was using the domain-based path `/api/functions/{path}`
which relies on host-based app resolution. This fails against non-production
servers (deploy previews, staging) where the host doesn't map to an app.

Switch to the app-id-based path `/api/apps/{appId}/functions/{path}`,
matching the pattern invoke() already uses. This makes functions.fetch()
work correctly regardless of which server URL is configured.

Made-with: Cursor
@claude
Copy link

claude bot commented Mar 18, 2026

Claude finished @netanelgilad's task —— View job


Code Review for PR #147

Summary

This PR successfully addresses the issue with functions.fetch() failing on non-production servers by switching from domain-based routing to app-id-based routing. The change is minimal, focused, and aligns with the existing pattern used by invoke().

Review Checklist

  • Read changed files to understand the implementation
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage and modifications
  • Provide comprehensive feedback

Code Quality and Best Practices ✅

The implementation is clean and follows established patterns:

  • The change is minimal and focused on solving the specific issue
  • Maintains consistency with the invoke() method which already uses /apps/${appId}/functions/${functionName} pattern (src/modules/functions.ts:80)
  • No unnecessary changes or refactoring

Potential Bugs or Issues ✅

No bugs identified. The change is straightforward and low-risk:

  • Path construction remains safe with proper normalization
  • The appId parameter is already validated and used throughout the module
  • Backward compatibility is maintained at the API level

Performance Considerations ✅

No performance impact:

  • The change only affects the URL path construction
  • No additional network calls or processing overhead
  • String interpolation has negligible performance impact

Security Concerns ✅

No security issues identified:

  • The appId is already part of the authenticated context
  • No new attack vectors introduced
  • Path normalization prevents directory traversal attempts

Test Coverage ✅

Excellent test coverage:

  • All 3 affected test cases properly updated to expect the new path format
  • Tests verify both with and without leading slash scenarios
  • Authorization header tests ensure authentication is preserved
  • The comprehensive test suite (19 functions tests, 116 total) provides confidence in the change

Additional Observations

  1. Consistency: The change brings fetch() in line with invoke(), improving API consistency
  2. Documentation: The JSDoc comments for fetch() were recently added (docs: add functions.fetch() JSDoc and fix external link stripping #146) and don't need updating as they don't specify the path format
  3. Error Handling: Existing error handling remains intact and will work correctly with the new path

Recommendation

Approved ✅ - This is a well-executed fix that solves the stated problem without introducing any risks or side effects. The implementation is correct, the tests are comprehensive, and the change improves consistency across the functions module.

@github-actions
Copy link

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.22-pr.147.4a9b875

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.22-pr.147.4a9b875"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.22-pr.147.4a9b875"
  }
}

Preview published to npm registry — try new features instantly!

@netanelgilad netanelgilad deleted the fix/functions-fetch-use-app-id-path branch March 18, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant