Skip to content

breaking: Throw error on empty concatenate, remove force cast#12

Merged
takeshishimada merged 2 commits intomainfrom
breaking/throw-on-empty-concatenate
Oct 30, 2025
Merged

breaking: Throw error on empty concatenate, remove force cast#12
takeshishimada merged 2 commits intomainfrom
breaking/throw-on-empty-concatenate

Conversation

@takeshishimada
Copy link
Copy Markdown
Contributor

Summary

Make ActionTask.concatenate() throw FlowError.noTasksToExecute when given an empty array, implementing the fail-fast principle for better error detection. This change completely removes the force cast from Store.swift.

Problem

The current implementation has a critical flaw:

// Store.swift:352 - Force cast with runtime type check
guard let result = lastResult else {
  if F.ActionResult.self is Void.Type {
    return (() as! F.ActionResult)  // ❌ Force cast
  }
  throw StoreError.cancelled
}

This code is actually unreachable because:

  1. concatenate([]) calls fatalError (crashes app)
  2. Therefore empty concatenated tasks never reach Store

The force cast exists to handle a case that cannot occur.

Root Cause Analysis

Empty task arrays indicate one of two scenarios:

  1. Logic error (bug) - Tasks were expected but none generated
  2. Valid business logic - Empty collection is normal (e.g., no items to process)

Current fatalError approach:

  • ❌ Crashes production apps
  • ❌ No error message or context
  • ❌ Cannot recover
  • ❌ Difficult to debug

Solution: Throw Typed Error

Changes

1. New Error Case

// FlowError.swift
case noTasksToExecute(context: String? = nil)

With comprehensive documentation:

  • Clear error message
  • Recovery suggestions
  • Debugging guidance

2. Make concatenate() Throwing

// ActionTask.swift
public static func concatenate(_ tasks: [ActionTask]) throws -> ActionTask {
  guard let first = tasks.first else {
    throw FlowError.noTasksToExecute(context: "concatenate(_:)")
  }
  return tasks.dropFirst().reduce(first) { $0.concatenate(with: $1) }
}

3. Remove Force Cast

// Store.swift
case .concatenated:
  let tasks = task.flattenConcatenated()
  
  // INVARIANT: Always ≥1 element (empty throws at construction)
  precondition(!tasks.isEmpty, "concatenated task list is empty")
  
  var lastResult = try await self.executeTask(tasks[0])
  
  for task in tasks.dropFirst() {
    lastResult = try await self.executeTask(task)
  }
  
  return lastResult  // ✅ No force cast!

Benefits

1. Fail Fast

let tasks = buildTasks()  // Bug: returns []
return try .concatenate(tasks)  // ✅ Throws immediately with clear message

2. Explicit Intent

// Developer must explicitly handle empty case
guard !tasks.isEmpty else {
  return .none  // "Empty is valid" - explicit
}
return try .concatenate(tasks)

3. Recoverable Errors

ActionHandler { action, state in
  return try .concatenate(tasks)
}
.catch { error, state in
  if case FlowError.noTasksToExecute = error {
    state.message = "No items to process"
  }
}

4. Type Safety

  • Works for all ActionResult types (not just Void)
  • No runtime type checking
  • Compile-time safety

5. Better Debugging

Error: No tasks to execute in concatenate(_:). Empty task arrays are not allowed.

Recovery Suggestion:
guard !tasks.isEmpty else {
  return .none
}
return try .concatenate(tasks)

Migration Guide

Before

let tasks = items.map { ... }
return .concatenate(tasks)  // Crashes if empty

After - Option 1: Explicit Guard

let tasks = items.map { ... }
guard !tasks.isEmpty else {
  return .none  // Empty is intentional
}
return try .concatenate(tasks)

After - Option 2: Propagate Error

let tasks = items.map { ... }
return try .concatenate(tasks)  // Error propagates to .catch

Variadic Version (Non-Breaking)

// Still safe - compile-time guaranteed non-empty
return .concatenate(
  .run { ... },
  .run { ... }
)

Breaking Changes

⚠️ This is a breaking change for v2.0

  • ActionTask.concatenate([ActionTask]) now throws
  • Callers must add try or guard

Testing

  • ✅ All 299 tests pass
  • ✅ No existing code used empty concatenate
  • ✅ Force cast completely removed
  • ✅ SwiftLint clean

Technical Details

Why Throw Instead of Returning .none?

  1. Fail Fast Principle: Bugs should be loud, not silent
  2. Swift Philosophy: Use try to mark dangerous operations
  3. Type Safety: Works for all ActionResult types
  4. Explicit Intent: Forces developers to think about empty case

Why This is Safe:

The invariant is enforced at construction time:

  • concatenate([]) throws before creating .concatenated
  • Therefore .concatenated always contains ≥1 task
  • flattenConcatenated() preserves this property
  • precondition documents and verifies this invariant

🤖 Generated with Claude Code

Make ActionTask.concatenate() throw FlowError.noTasksToExecute when
given an empty array, implementing fail-fast principle for better
error detection and debugging.

Changes:
- Add FlowError.noTasksToExecute case with comprehensive documentation
- Make ActionTask.concatenate(_:) throwing for array version
- Remove force cast from Store.swift executeTask() method
- Add precondition with clear invariant documentation
- Update all LocalizedError implementations

Benefits:
- Fail fast: Empty arrays detected immediately
- Better debugging: Clear error messages with recovery suggestions
- Type safety: Works for all ActionResult types (not just Void)
- No force cast: Eliminates runtime type checking
- Explicit intent: Developers must handle empty case explicitly

Breaking Changes:
- ActionTask.concatenate([ActionTask]) now throws
- Callers must either:
  1. Use try and handle the error
  2. Guard against empty arrays before calling
  3. Variadic version remains non-throwing (compile-time safe)

Migration:
```swift
// Before
return .concatenate(tasks)

// After Option 1: Explicit guard
guard !tasks.isEmpty else {
  return .none  // Empty is valid
}
return try .concatenate(tasks)

// After Option 2: Propagate error
return try .concatenate(tasks)
```

All tests pass (299 tests). No existing code used empty concatenate.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Fix SwiftLint errors and add documentation for dynamic array usage.

Changes:
- Add swiftlint:disable for justified force_try in variadic concatenate
- Fix line length violations in Store.swift and FlowError.swift
- Add DocC example for dynamic task concatenation with empty handling

SwiftLint fixes:
- ActionTask.swift:479 - Disable force_try (justified: variadic guarantees non-empty)
- Store.swift:343 - Split long precondition message across multiple lines
- FlowError.swift:193,195 - Use multiline strings for long error messages

DocC updates:
- Add migration guide for dynamic concatenate in CoreElements.md
- Show both static (variadic) and dynamic (array) usage patterns
- Demonstrate explicit empty array handling with guard

All tests pass (299 tests). SwiftLint clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@takeshishimada takeshishimada self-assigned this Oct 30, 2025
@takeshishimada takeshishimada merged commit 63aba13 into main Oct 30, 2025
8 checks passed
@takeshishimada takeshishimada deleted the breaking/throw-on-empty-concatenate branch October 31, 2025 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant