Skip to content

fix(workflows): raise a clear error, not a cryptic crash, on non-string filter args#3522

Open
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/filter-nonstring-arg-crash
Open

fix(workflows): raise a clear error, not a cryptic crash, on non-string filter args#3522
Noor-ul-ain001 wants to merge 1 commit into
github:mainfrom
Noor-ul-ain001:fix/filter-nonstring-arg-crash

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

The map, join, and contains expression filters assumed their argument was a string. A non-string argument — an authoring mistake such as | map(5), | join(5), or | contains(5) — reached an operation that only strings support and raised a cryptic exception that escaped the evaluator entirely:

Expression Reaches Raises
| map(5) attr.split(".") AttributeError: 'int' object has no attribute 'split'
| join(5) separator.join(...) AttributeError: 'int' object has no attribute 'join'
| contains(5) on a string x in str TypeError: 'in <string>' requires string as left operand, not int

The engine wraps neither expression evaluation nor step_impl.execute() in a try/except, so each of these took down the whole run with a message that names none of the real problem.

Fix

Validate the argument type up front and raise a ValueError naming the filter and the offending type instead, mirroring the strict argument handling already in from_json.

contains guards only the string-value branch: for a list value, membership of any element type is legitimate (5 in [1, 2, 5]), so that branch is intentionally left unguarded.

Tests

Added to TestExpressions:

  • map / join / contains (on a string) with a non-string arg each raise a clear ValueError.
  • contains on a list with a non-string arg stays valid (5 in [1,2,5] → True).
  • Existing test_registered_filters_unaffected regression coverage is unchanged.

All 46 TestExpressions tests pass locally.

Follows the same "fail loudly instead of crashing the run" pattern as the merged from_json, non-list wait_for, and non-mapping cases fixes.

…ng filter args

The `map`, `join`, and `contains` expression filters assumed their
argument was a string. A non-string argument — an authoring mistake such
as `| map(5)`, `| join(5)`, or `| contains(5)` — reached an operation
that only strings support and raised a cryptic exception that escaped the
evaluator entirely:

  * `map(5)`      -> `attr.split(".")`  -> AttributeError
  * `join(5)`     -> `separator.join(...)` -> AttributeError
  * `contains(5)` on a string value -> `x in str` -> TypeError

The engine wraps neither expression evaluation nor `step_impl.execute()`
in a try/except, so each of these took down the whole run with a message
that names none of the real problem.

Validate the argument type up front and raise a `ValueError` naming the
filter and the offending type instead, mirroring the strict argument
handling already in `from_json`. `contains` guards only the string-value
branch: for a list value, membership of any element type is legitimate
(`5 in [1, 2, 5]`), so that branch is intentionally left unguarded.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Noor-ul-ain001 Noor-ul-ain001 requested a review from mnriem as a code owner July 14, 2026 17:18
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