Skip to content

fix: When the function parameter is of non string type, if the parameter is empty, the function will run with an error #3053 #3107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2025
Merged

fix: When the function parameter is of non string type, if the parameter is empty, the function will run with an error #3053 #3107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/application/flow/step_node/function_lib_node/impl/base_function_lib_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def valid_reference_value(_type, value, name):


def convert_value(name: str, value, _type, is_required, source, node):
if not is_required and value is None:
if not is_required and (value is None or (isinstance(value, str) and len(value) == 0)):
return None
if not is_required and source == 'reference' and (value is None or len(value) == 0):
return None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has a few concerns:

  1. Redundant Check: The second if statement checks both if source is 'reference' and if the value is empty. This redundancy can be resolved by checking only one condition.

  2. String Length Consideration: The current implementation does not explicitly handle cases where the input could contain non-string values that should also be considered "empty," although it implicitly handles them because None is handled separately.

  3. Immutability of List: Although this doesn't appear to cause an issue with the given code, lists cannot be used immutably in certain contexts without explicit conversion (e.g., using tuple assignment).

Here's a revised version with these considerations addressed:

def valid_reference_value(_type, value, name):
    
def convert_value(name: str, value, _type, is_required, source, node):
    if value is None and is_required:
        raise ValueError(f"Required field '{name}' missing.")

    # Convert string to title case unless required specifically
    if _type == 'title-cased':
        if isinstance(value, str) and len(value.strip()) > 0:
            return value.title()

    # Handle null or blank values appropriately based on requirement and source
    if not is_required and (value is None or 
                               (isinstance(value, str) and len(value.strip()) == 0)):
        return None

Key Changes:

  • Removed redundant check inside the second if.
  • Added logic for converting strings to title case when _type is 'title-cased' and there is no content.
  • Raised a ValueError for required fields that are None.

These changes help improve readability, error handling, and potentially performance depending on context.

Expand Down
2 changes: 1 addition & 1 deletion apps/application/flow/step_node/function_node/impl/base_function_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def valid_reference_value(_type, value, name):


def convert_value(name: str, value, _type, is_required, source, node):
if not is_required and value is None:
if not is_required and (value is None or (isinstance(value, str) and len(value) == 0)):
return None
if source == 'reference':
value = node.workflow_manage.get_reference_field(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided function convert_value has a minor issue with handling empty string conversions from references:

Issue: The current check (isinstance(value, str) and len(value) == 0) will always result in False, because both conditions must be True. If the condition fails at all (None), the outer check will automatically handle it.

Suggestion:
Remove the first condition that checks for an empty string when the value is already None.

Here's the updated code snippet:

@@ -49,7 +49,6 @@ def valid_reference_value(_type, value, name):

def convert_value(name: str, value, _type, is_required, source, node):
    if not is_required and value is None:
        return None
    
    # Check if source is reference
    if source == 'reference':
        # Get value using workflow_manage.get_reference_field method

This ensures that the conversion logic only executes if the value is not None.

Expand Down