v0.3.26: fix billing, bubble up workflow block errors, credentials security improvements#967
v0.3.26: fix billing, bubble up workflow block errors, credentials security improvements#967icecrasher321 merged 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements two key improvements to the Sim workflow execution system:
1. Workflow Error Propagation Enhancement
The primary change reverts a previous modification that was preventing proper error handling in nested workflow executions. The WorkflowBlockHandler now properly bubbles up errors from child workflows to parent workflows by:
- Explicitly checking if child workflow execution returns
success: falseand throwing an error with enhanced context - Implementing smart error message handling to prevent duplication in deeply nested scenarios
- Updating all related tests to expect thrown exceptions instead of returned error objects
This ensures that when a child workflow fails, the parent workflow also fails appropriately rather than continuing execution or silently ignoring the failure.
2. Billing Environment Variable Separation
The second major change addresses a Next.js architecture issue by properly separating client-side and server-side billing configuration:
- Adds
NEXT_PUBLIC_BILLING_ENABLEDas a new client-side environment variable alongside the existing server-sideBILLING_ENABLED - Updates multiple React components (Sidebar, SettingsModal, SettingsNavigation) to use
getEnv('NEXT_PUBLIC_BILLING_ENABLED')instead of server-side utilities - Follows Next.js best practices where
NEXT_PUBLIC_prefixed variables are safely exposed to the browser
This change enables client-side components to properly determine billing status for conditional UI rendering while maintaining server-side billing logic separation. The implementation integrates with the existing environment variable system and maintains backward compatibility with server-side billing functionality.
Confidence score: 3/5
- This PR contains potentially breaking changes to error handling behavior that could affect workflow execution patterns
- Score reflects concerns about type safety in environment variable handling and the fundamental change to error propagation patterns
- Pay close attention to
apps/sim/executor/handlers/workflow/workflow-handler.tsand the billing-related React components for proper error handling and environment variable access
7 files reviewed, 3 comments
| const subscription = getSubscriptionStatus() | ||
|
|
||
| // Get billing status | ||
| const isBillingEnabled = getEnv('NEXT_PUBLIC_BILLING_ENABLED') || false |
There was a problem hiding this comment.
logic: getEnv returns string | undefined but you're using || false which could result in truthy strings like 'false' being treated as true. Consider using the isTruthy utility from @/lib/env for proper boolean conversion.
| ) | ||
|
|
||
| // If the child workflow failed, throw an error to trigger proper error handling in the parent | ||
| if ((mappedResult as any).success === false) { |
There was a problem hiding this comment.
style: Type assertion to any bypasses TypeScript safety. Consider defining a proper interface for the mapped result structure.
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| const isLoading = workflowsLoading || sessionLoading | ||
|
|
||
| // Get billing status | ||
| const isBillingEnabled = getEnv('NEXT_PUBLIC_BILLING_ENABLED') || false |
There was a problem hiding this comment.
logic: Type safety concern: getEnv() returns string | undefined, but isBillingEnabled is used as boolean. Consider using a proper boolean conversion like getEnv('NEXT_PUBLIC_BILLING_ENABLED') === 'true' instead of relying on falsy check with || false.
| const isBillingEnabled = getEnv('NEXT_PUBLIC_BILLING_ENABLED') || false | |
| const isBillingEnabled = getEnv('NEXT_PUBLIC_BILLING_ENABLED') === 'true' |
…er, fix google sheets block url split bug (#968) * improvement(credentials-sharing-security): cleanup and reuse helper to determine credential access * few more routes * fix google sheets block * fix test mocks * fix calendar route
icecrasher321
changed the title
2 participants
Uh oh!
There was an error while loading. Please reload this page.