[feat]: add swift-log dep & some more logging

jamieQ · jamieQ · commit 0700bd76f748 · 2025-09-16T17:10:54.000-05:00
diff --git a/Package.swift b/Package.swift
@@ -62,13 +62,17 @@ let package = Package(
         .package(url: "https://github.com/pointfreeco/swift-perception", "1.5.0" ..< "3.0.0"),
         .package(url: "https://github.com/pointfreeco/swift-custom-dump", from: "1.3.3"),
         .package(url: "https://github.com/pointfreeco/xctest-dynamic-overlay", from: "1.0.0"),
+        .package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"),
     ],
     targets: [
         // MARK: Workflow
 
         .target(
             name: "Workflow",
-            dependencies: ["ReactiveSwift"],
+            dependencies: [
+                .product(name: "Logging", package: "swift-log"),
+                .product(name: "ReactiveSwift", package: "ReactiveSwift"),
+            ],
             path: "Workflow/Sources"
         ),
         .target(
@@ -125,7 +129,10 @@ let package = Package(
 
         .target(
             name: "WorkflowReactiveSwift",
-            dependencies: ["ReactiveSwift", "Workflow"],
+            dependencies: [
+                "Workflow",
+                .product(name: "ReactiveSwift", package: "ReactiveSwift"),
+            ],
             path: "WorkflowReactiveSwift/Sources"
         ),
         .target(
@@ -139,7 +146,10 @@ let package = Package(
 
         .target(
             name: "WorkflowRxSwift",
-            dependencies: ["RxSwift", "Workflow"],
+            dependencies: [
+                "Workflow",
+                .product(name: "RxSwift", package: "RxSwift"),
+            ],
             path: "WorkflowRxSwift/Sources"
         ),
         .target(
diff --git a/Samples/Tuist/Package.resolved b/Samples/Tuist/Package.resolved
diff --git a/Workflow/Sources/RenderContext.swift b/Workflow/Sources/RenderContext.swift
@@ -153,7 +153,10 @@ public class RenderContext<WorkflowType: Workflow>: RenderContextType {
         }
 
         private func assertStillValid() {
-            assert(isValid, "A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
+            if !isValid {
+                WorkflowLogger.logExternal(as: .error, "A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
+                assertionFailure("A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
+            }
         }
     }
 }
diff --git a/Workflow/Sources/SubtreeManager.swift b/Workflow/Sources/SubtreeManager.swift
@@ -15,6 +15,7 @@
  */
 
 import Dispatch
+import IssueReporting
 
 extension WorkflowNode {
     /// Manages the subtree of a workflow. Specifically, this type encapsulates the logic required to update and manage
@@ -76,6 +77,17 @@ extension WorkflowNode {
 
             wrapped.invalidate()
 
+            #if DEBUG
+            var contextEscapeCheck = consume wrapped
+            if !isKnownUniquelyReferenced(&contextEscapeCheck) {
+                if let _ = TestContext.current {
+                    reportIssue("Detected escape of a RenderContext<\(WorkflowType.self)> instance from a `render()` method. A RenderContext is only valid to use within render(); Allowing it to escape can lead to incorrect behavior.")
+                } else {
+                    assertionFailure("Detected escape of a RenderContext<\(WorkflowType.self)> instance from a `render()` method. A RenderContext is only valid to use within render(); Allowing it to escape can lead to incorrect behavior.")
+                }
+            }
+            #endif
+
             /// After the render is complete, assign children using *only the children that were used during the render
             /// pass.* This means that any pre-existing children that were *not* used during the render pass are removed
             /// as a result of this call to `render`.
@@ -409,27 +421,28 @@ extension WorkflowNode.SubtreeManager {
             case .valid(let handler):
                 handler(event)
 
-            case .invalid:
+            case .invalid where isReentrantCall:
                 #if DEBUG
                 // Reentrancy seems to often be due to UIKit behaviors over
                 // which we have little control (e.g. synchronous resignation
                 // of first responder after a new Rendering is assigned). Emit
                 // some debug info in these cases.
-                if isReentrantCall {
-                    print("[\(WorkflowType.self)]: ℹ️ Sink sent another action after it was invalidated but before its original action handling was resolved. This new action will be ignored. If this is unexpected, set a Swift error breakpoint on `\(InvalidSinkSentAction.self)` to debug.")
-                }
-
-                do {
-                    throw InvalidSinkSentAction()
-                } catch {}
+                print("[\(WorkflowType.self)]: ℹ️ Sink sent another action after it was invalidated but before its original action handling was resolved. This new action will be ignored. If this is unexpected, set a Swift error breakpoint on `\(InvalidSinkSentAction.self)` to debug.")
+                do { throw InvalidSinkSentAction() } catch {}
                 #endif
 
+            case .invalid:
                 // If we're invalid and this is the first time `handle()` has
                 // been called, then it's likely we've somehow been inadvertently
                 // retained from the 'outside world'. Fail more loudly in this case.
-                assert(isReentrantCall, """
-                    [\(WorkflowType.self)]: Sink sent an action after it was invalidated. This action will be ignored.
-                """)
+                if !isReentrantCall {
+                    WorkflowLogger.logExternal(as: .warning, """
+                        [\(WorkflowType.self)]: Sink sent an action after it was invalidated. This action will be ignored.
+                    """)
+                    assertionFailure("""
+                        [\(WorkflowType.self)]: Sink sent an action after it was invalidated. This action will be ignored.
+                    """)
+                }
             }
         }
 
diff --git a/Workflow/Sources/WorkflowLogger.swift b/Workflow/Sources/WorkflowLogger.swift
@@ -15,6 +15,7 @@
  */
 
 import Foundation
+import Logging
 import os.signpost
 
 extension OSLog {
@@ -207,3 +208,33 @@ enum WorkflowLogger {
         }
     }
 }
+
+// MARK: - External Logging
+
+typealias EternalLogger = Logging.Logger
+
+extension WorkflowLogger {
+    static func logExternal(
+        as level: EternalLogger.Level,
+        _ message: @autoclosure () -> EternalLogger.Message,
+        metadata: @autoclosure () -> EternalLogger.Metadata? = nil,
+        source: @autoclosure () -> String? = nil,
+        file: String = #fileID,
+        function: String = #function,
+        line: UInt = #line
+    ) {
+        EternalLogger.workflow.log(
+            level: level,
+            message(),
+            metadata: metadata(),
+            source: source(),
+            file: file,
+            function: function,
+            line: line
+        )
+    }
+}
+
+extension EternalLogger {
+    static let workflow = Logger(label: "com.squareup.workflow")
+}
diff --git a/Workflow/Tests/SubtreeManagerTests.swift b/Workflow/Tests/SubtreeManagerTests.swift
@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
+import IssueReporting
 import ReactiveSwift
 import XCTest
+
 @testable import Workflow
 
 final class SubtreeManagerTests: XCTestCase {
@@ -126,16 +128,18 @@ final class SubtreeManagerTests: XCTestCase {
 
         var escapingContext: RenderContext<ParentWorkflow>!
 
-        _ = manager.render { context -> TestViewModel in
-            XCTAssertTrue(context.isValid)
-            escapingContext = context
-            return context.render(
-                workflow: TestWorkflow(),
-                key: "",
-                outputMap: { _ in AnyWorkflowAction.noAction }
-            )
+        withExpectedIssue {
+            _ = manager.render { context -> TestViewModel in
+                XCTAssertTrue(context.isValid)
+                escapingContext = context
+                return context.render(
+                    workflow: TestWorkflow(),
+                    key: "",
+                    outputMap: { _ in AnyWorkflowAction.noAction }
+                )
+            }
+            manager.enableEvents()
         }
-        manager.enableEvents()
 
         XCTAssertFalse(escapingContext.isValid)
     }

Original file line number	Diff line number	Diff line change
`@@ -153,7 +153,10 @@ public class RenderContext<WorkflowType: Workflow>: RenderContextType {`
`153`	`153`	`}`
`154`	`154`
`155`	`155`	`private func assertStillValid() {`
`156`		- assert(isValid, "A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
	`156`	`+ if !isValid {`
	`157`	+ WorkflowLogger.logExternal(as: .error, "A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
	`158`	+ assertionFailure("A `RenderContext` instance was used outside of the workflow's `render` method. It is a programmer error to capture a context in a closure or otherwise cause it to be used outside of the `render` method.")
	`159`	`+ }`
`157`	`160`	`}`
`158`	`161`	`}`
`159`	`162`	`}`