Refactor error message passing in Pipeline stages (#15939)

Co-authored-by: Nick Cooke <36927374+ncooke3@users.noreply.github.com>
Co-authored-by: Yvonne Yu <yyyu@google.com>
Co-authored-by: Jeff Scudder <jscudder@google.com>

Author: 4 people

Firestore/Swift/Source/ExpressionImplementation.swift

@@ -13,6 +13,13 @@
 // limitations under the License.
 
 extension Expression {
+  /// Returns the internal error message. It is overridden in specific expression implementations
+  /// (like FunctionExpression) but defaults to `nil` for others.
+  /// This design is to support pipeline conversion to expression.
+  var errorMessage: String? {
+    return nil
+  }
+
   func toBridge() -> ExprBridge {
     return (self as! BridgeWrapper).bridge
   }

Firestore/Swift/Source/Helper/PipelineHelper.swift

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import Foundation
+
 enum Helper {
   enum HelperError: Error, LocalizedError {
     case duplicateAlias(String)
@@ -44,32 +46,57 @@ enum Helper {
 
   static func selectablesToMap(selectables: [Selectable]) -> ([String: Expression], Error?) {
     var exprMap = [String: Expression]()
+    var errors = [String]()
     for selectable in selectables {
       guard let value = selectable as? SelectableWrapper else {
         fatalError("Selectable class must conform to SelectableWrapper.")
       }
       let alias = value.alias
-      if exprMap.keys.contains(alias) {
-        return ([:], HelperError.duplicateAlias("Duplicate alias '\(alias)' found in selectables."))
+      if let errorMessage = value.expr.errorMessage {
+        errors.append(errorMessage)
+      }
+      if exprMap[alias] != nil {
+        errors.append("Duplicate alias '\(alias)' found in selectables.")
       }
       exprMap[alias] = value.expr
     }
+    if !errors.isEmpty {
+      return (
+        [:],
+        NSError(
+          domain: "com.google.firebase.firestore",
+          code: 3,
+          userInfo: [NSLocalizedDescriptionKey: errors.joined(separator: "\n")]
+        )
+      )
+    }
     return (exprMap, nil)
   }
 
   static func aliasedAggregatesToMap(accumulators: [AliasedAggregate])
     -> ([String: AggregateFunction], Error?) {
     var accumulatorMap = [String: AggregateFunction]()
+    var errors = [String]()
     for aliasedAggregate in accumulators {
       let alias = aliasedAggregate.alias
-      if accumulatorMap.keys.contains(alias) {
-        return (
-          [:],
-          HelperError.duplicateAlias("Duplicate alias '\(alias)' found in accumulators.")
-        )
+      if let errorMessage = aliasedAggregate.aggregate.errorMessage {
+        errors.append(errorMessage)
+      }
+      if accumulatorMap[alias] != nil {
+        errors.append("Duplicate alias '\(alias)' found in accumulators.")
       }
       accumulatorMap[alias] = aliasedAggregate.aggregate
     }
+    if !errors.isEmpty {
+      return (
+        [:],
+        NSError(
+          domain: "com.google.firebase.firestore",
+          code: 3,
+          userInfo: [NSLocalizedDescriptionKey: errors.joined(separator: "\n")]
+        )
+      )
+    }
     return (accumulatorMap, nil)
   }
 

Firestore/Swift/Source/Stages.swift

@@ -110,14 +110,17 @@ class Where: Stage {
 
   let bridge: StageBridge
   private var condition: BooleanExpression?
+  let errorMessage: String?
 
   init(condition: BooleanExpression) {
     self.condition = condition
     bridge = WhereStageBridge(expr: condition.toBridge())
+    errorMessage = condition.errorMessage
   }
 
   init(bridge: WhereStageBridge) {
     self.bridge = bridge
+    errorMessage = nil
   }
 }
 
@@ -313,10 +316,12 @@ class ReplaceWith: Stage {
   let name: String = "replace_with"
   let bridge: StageBridge
   private var expr: Expression
+  let errorMessage: String?
 
   init(expr: Expression) {
     self.expr = expr
     bridge = ReplaceWithStageBridge(expr: expr.toBridge())
+    errorMessage = expr.errorMessage
   }
 }
 
@@ -346,9 +351,12 @@ class Union: Stage {
   let bridge: StageBridge
   private var other: Pipeline
 
+  let errorMessage: String?
+
   init(other: Pipeline) {
     self.other = other
     bridge = UnionStageBridge(other: other.bridge)
+    errorMessage = other.errorMessage
   }
 }
 
@@ -359,12 +367,14 @@ class Unnest: Stage {
   private var alias: Expression
   private var field: Expression
   private var indexField: String?
+  let errorMessage: String?
 
   init(field: Selectable, indexField: String? = nil) {
     let seletable = field as! SelectableWrapper
     self.field = seletable.expr
     alias = Field(seletable.alias)
     self.indexField = indexField
+    errorMessage = self.field.errorMessage ?? alias.errorMessage
 
     bridge = UnnestStageBridge(
       field: self.field.toBridge(),
@@ -380,6 +390,7 @@ class RawStage: Stage {
   let bridge: StageBridge
   private var params: [Sendable]
   private var options: [String: Sendable]?
+  let errorMessage: String? = nil
 
   init(name: String, params: [Sendable], options: [String: Sendable]? = nil) {
     self.name = name

Firestore/Swift/Source/SwiftAPI/Pipeline/Aggregates/AggregateFunction.swift

@@ -23,6 +23,12 @@ public class AggregateFunction: AggregateBridgeWrapper, @unchecked Sendable {
   let functionName: String
   let args: [Expression]
 
+  /// The error message associated with this aggregate function or its arguments, if any.
+  var errorMessage: String? {
+    let errors = args.compactMap { $0.errorMessage }
+    return errors.isEmpty ? nil : errors.joined(separator: "\n")
+  }
+
   /// Creates a new `AggregateFunction`.
   ///
   /// - Parameters:

Firestore/Swift/Source/SwiftAPI/Pipeline/Expressions/FunctionExpressions/FunctionExpression.swift

@@ -25,6 +25,12 @@ public class FunctionExpression: Expression, BridgeWrapper, @unchecked Sendable
   let functionName: String
   let args: [Expression]
 
+  /// The error message associated with this expression or its arguments, if any.
+  var errorMessage: String? {
+    let errors = args.compactMap { $0.errorMessage }
+    return errors.isEmpty ? nil : errors.joined(separator: "\n")
+  }
+
   /// Creates a new `FunctionExpression`.
   ///
   /// - Parameters: