fix: validate Node max_retries and wait parameters#134
fix: validate Node max_retries and wait parameters#134Exploreunive wants to merge 3 commits intoThe-Pocket:mainfrom
Conversation
Previously, Node(max_retries=0) or Node(max_retries=-1) silently skipped execution entirely — range(0) and range(-1) are empty, so exec() was never called and None was returned with no warning or error. This commit adds validation in Node.__init__ to raise ValueError when: - max_retries < 1 (must be at least 1) - wait < 0 (must be non-negative) Since AsyncNode inherits from Node, the validation automatically applies to all async variants (AsyncNode, AsyncBatchNode, etc.). Includes 16 new tests covering: - ValueError for max_retries=0, -1, -100 - ValueError for wait=-1 - Normal operation with valid params - exec() is actually called with valid max_retries
…hNode BatchNode._exec gracefully handles None via (items or []), but AsyncBatchNode and AsyncParallelBatchNode did not, causing a TypeError when prep_async returned None (which is the default for BaseNode.prep). Before this fix: BatchNode(None) -> [] (correct) AsyncBatchNode(None) -> TypeError: 'NoneType' object is not iterable After this fix, all three batch node variants return [] when prep returns None, maintaining consistent behavior across sync/async. Includes 8 tests covering None, empty list, and normal data for BatchNode, AsyncBatchNode, and AsyncParallelBatchNode.
|
Update: Added a second fix for a related bug. Additional fix: handle None prep results in async batch nodes
# Before: BatchNode handles None, AsyncBatchNode crashes
BatchNode(None) -> [] ✅
AsyncBatchNode(None) -> TypeError ❌
# After: consistent behavior
BatchNode(None) -> [] ✅
AsyncBatchNode(None) -> [] ✅Added 8 new tests. All 80 tests pass. |
Flow.get_next_node used `if not nxt` to detect missing successors, but this conflated 'key not found' with 'key exists but value is None'. When users wrote `node - "end" >> None` to create a terminal branch, returning "end" triggered a spurious warning: "Flow ends: 'end' not found in ['end']" Fix: use `key not in curr.successors` to distinguish between a key that was never registered vs a key explicitly set to None. Fixes The-Pocket#56 Includes 4 tests covering: - >> None with explicit action (no warning) - Mixed >> node and >> None branches - Unknown action still warns - Default >> None successor
|
Update 2: Added a third fix for issue #56. Fix: don't warn when >> None successor is explicitly returned
When users wrote Fix: Use Fixes #56 This PR now contains 3 fixes:
All 84 tests pass. |
Update: 3rd fix added (issue #56)Bug: Root cause: Fix: Changed to Test case from issue #56: meta_batch - "end" >> None
# Returns "end" → no warning (correct termination)4 new tests added. All 84 tests pass ✅ |
Problem
Node(max_retries=0)andNode(max_retries=-1)silently skip execution entirely —exec()is never called andNoneis returned, with no warning or error.This happens because
range(0)andrange(-1)are empty, so the retry loop body never executes:Fix
Add validation in
Node.__init__to raiseValueErrorwhen:max_retries < 1(must be at least 1)wait < 0(must be non-negative)Since
AsyncNodeinherits fromNode, the validation automatically applies to all async variants (AsyncNode,AsyncBatchNode,AsyncParallelBatchNode).Testing
tests/test_node_params_validation.pycovering:ValueErrorformax_retries=0,-1,-100ValueErrorforwait=-1exec()is actually called (not silently skipped)