Skip to content

Add App Logs#42

Merged
ChenMachBase merged 1 commit intomainfrom
add-app-logs
Oct 22, 2025
Merged

Add App Logs#42
ChenMachBase merged 1 commit intomainfrom
add-app-logs

Conversation

@ChenMachBase
Copy link
Contributor

No description provided.

@claude
Copy link

claude bot commented Oct 22, 2025

Claude finished @ChenMachBase's task —— View job


Review Tasks

  • Read and analyze the changes in src/client.ts
  • Read and analyze the new src/modules/app-logs.ts file
  • Check for repo-specific guidelines (CLAUDE.md)
  • Review code quality and best practices
  • Identify potential bugs or issues
  • Assess performance considerations
  • Evaluate security concerns
  • Check test coverage

Code Review for PR #42: Add App Logs

Summary

This PR introduces an app logs module to the Base44 SDK, providing functionality to track user activity, fetch logs, and retrieve statistics. The implementation adds 43 lines of new code and integrates the module into the existing client structure.

Code Quality and Best Practices ✅

Strengths:

  • Follows consistent module pattern established in the codebase (similar to entities, integrations, etc.)
  • Proper TypeScript typing with AxiosInstance import
  • Clean, modular structure with a factory function pattern
  • Appropriate JSDoc comments for all public methods

Areas for improvement:

  1. Return type specificity: The fetchLogs() and getStats() methods return Promise<any> which reduces type safety. Consider defining proper interfaces for the response data.

Potential Bugs or Issues 🐛

  1. Missing error handling: The methods don't handle or document potential errors. While Axios client might have global error handling, specific error cases should be documented.

  2. Response handling inconsistency: In fetchLogs() and getStats(), the methods return the full Axios response object rather than just the data (unlike other modules that typically return response.data). This breaks consistency with patterns in entities.ts:

    // Current implementation returns full response
    return response;
    
    // Should probably be:
    return response.data;
  3. URL encoding missing: The pageName parameter in logUserInApp() is directly interpolated into the URL path without encoding, which could cause issues with special characters:

    // Current: 
    await axios.post(`${baseURL}/log-user-in-app/${pageName}`);
    
    // Should be:
    await axios.post(`${baseURL}/log-user-in-app/${encodeURIComponent(pageName)}`);

Performance Considerations ⚡

  1. No pagination parameters: While fetchLogs() accepts generic params, there's no clear interface for pagination (limit, skip, etc.), which could lead to performance issues with large datasets.

  2. No caching mechanism: Frequent calls to getStats() might benefit from client-side caching, especially if stats don't change frequently.

Security Concerns 🔒

  1. Input validation: No client-side validation for the pageName parameter, which could potentially allow malformed data to be sent to the server.

  2. PII exposure: The module logs user activity without any apparent anonymization or privacy controls. Consider if GDPR compliance requires additional privacy measures.

  3. Service role access: The module is exposed to both user and service role clients (lines 120 and 138 in client.ts), but there's no distinction in functionality. Consider if all methods should be available to regular users.

Test Coverage ❌

Critical issue: No tests were added for the new module. The codebase has comprehensive test coverage for other modules (entities, auth, integrations), but this PR introduces untested code.

Recommended test cases:

  • Unit tests mocking Axios calls (similar to entities.test.js pattern)
  • Test URL encoding for special characters in pageName
  • Test error handling scenarios
  • Test parameter passing for fetchLogs and getStats
  • Integration tests if applicable

Additional Recommendations

  1. TypeScript improvements:

    interface LogEntry {
      id: string;
      userId?: string;
      pageName: string;
      timestamp: Date;
      // ... other fields
    }
    
    interface LogStats {
      totalUsers: number;
      pageViews: number;
      // ... other stats
    }
    
    async fetchLogs(params: Record<string, any> = {}): Promise<LogEntry[]> {
      const response = await axios.get(baseURL, { params });
      return response.data;
    }
  2. Add specific parameter interfaces:

    interface FetchLogsParams {
      limit?: number;
      skip?: number;
      startDate?: string;
      endDate?: string;
      userId?: string;
    }
  3. Documentation: Consider adding more detailed JSDoc comments explaining the purpose of each method and expected parameter formats.

Conclusion

While the implementation follows existing patterns and integrates well with the SDK architecture, it needs improvements in type safety, error handling, and critically, test coverage. The security and data handling aspects should also be reviewed to ensure compliance with privacy regulations.

@ChenMachBase ChenMachBase merged commit 7a02156 into main Oct 22, 2025
3 checks passed
@github-actions
Copy link

🚀 Package Preview Available!


Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.1-pr.42.20d6390

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.1-pr.42.20d6390"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.1-pr.42.20d6390"
  }
}

Preview published to npm registry — try new features instantly!

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