Merge pull request #54 from GoodRequest/async-semaphore

fix: Fix actor isolation crashes and data races

Author: plajdo

Sources/GoodNetworking/Interception/AuthenticationInterceptor.swift

@@ -14,17 +14,17 @@ import Foundation
 public final class AuthenticationInterceptor<AuthenticatorType: Authenticator>: Interceptor, @unchecked Sendable {
 
     private let authenticator: AuthenticatorType
-    private let lock: AsyncLock
+    private let lock: AsyncSemaphore
 
     public init(authenticator: AuthenticatorType) {
         self.authenticator = authenticator
-        self.lock = AsyncLock()
+        self.lock = AsyncSemaphore(value: 1)
     }
 
     public func adapt(urlRequest: inout URLRequest) async throws(NetworkError) {
-        await lock.lock()
-        defer { lock.unlock() }
-        
+        await lock.wait()
+        defer { lock.signal() }
+
         if let credential = await authenticator.getCredential() {
             if let refreshableCredential = credential as? RefreshableCredential, refreshableCredential.requiresRefresh {
                 try await refresh(credential: credential)
@@ -47,8 +47,8 @@ public final class AuthenticationInterceptor<AuthenticatorType: Authenticator>:
         // Stop further authentication with possibly invalid credential.
         // If a refresh is already in progress, stopping other requests
         // here will ensure further retries will contain the latest credentials.
-        await lock.lock()
-        defer { lock.unlock() }
+        await lock.wait()
+        defer { lock.signal() }
 
         // A credential is available
         guard let credential = await authenticator.getCredential() else {

Sources/GoodNetworking/Logging/DataTaskLogging.swift

@@ -26,7 +26,7 @@ internal extension DataTaskProxy {
         \(prettyPrintMessage(data: task.originalRequest?.httpBody))
         
         📦 Received data:
-        \(prettyPrintMessage(data: receivedData))
+        \(prettyPrintMessage(data: receivedData, mimeType: task.response?.mimeType))
         """
     }
 
@@ -53,9 +53,9 @@ internal extension DataTaskProxy {
         .joined(separator: "\n")
     }
 
-    @NetworkActor private func prettyPrintMessage(data: Data?) -> String {
+    @NetworkActor private func prettyPrintMessage(data: Data?, mimeType: String? = "text/plain") -> String {
         guard let data else { return "" }
-
+        guard plainTextMimeTypeHeuristic(mimeType) else { return "🏞️ Detected MIME type is not plain text" }
         guard data.count < Self.maxLogSizeBytes else {
             return "💡 Data size is too big (\(data.count) bytes), console limit is \(Self.maxLogSizeBytes) bytes"
         }
@@ -67,7 +67,9 @@ internal extension DataTaskProxy {
         }
 
         if let string = String(data: data, encoding: .utf8) {
-            if let jsonData = try? JSONSerialization.jsonObject(with: data, options: []),
+            let mimeContainsJson = mimeType?.contains("json")
+            if mimeContainsJson ?? true,
+               let jsonData = try? JSONSerialization.jsonObject(with: data, options: []),
                let prettyPrintedData = try? JSONSerialization.data(withJSONObject: jsonData, options: serializationOptions),
                let prettyPrintedString = String(data: prettyPrintedData, encoding: .utf8) {
                 return prettyPrintedString
@@ -79,4 +81,25 @@ internal extension DataTaskProxy {
         return "🔍 Couldn't decode data as UTF-8"
     }
 
+    @NetworkActor private func plainTextMimeTypeHeuristic(_ mimeType: String?) -> Bool {
+        guard let mimeType else { return false }
+
+        let knownPlainTextMimeTypes = ["javascript", "yaml", "toml", "sql", "graphql", "markdown", "urlencoded"]
+
+        let isTextMimeType = mimeType.hasPrefix("text/")
+        let isXml = mimeType.hasSuffix("+xml") || mimeType.contains("xml")
+        let isJson = mimeType.hasSuffix("+json") || mimeType.contains("json")
+        let isTextBased = mimeType.containsOneOf(knownPlainTextMimeTypes)
+
+        return isTextMimeType || isXml || isJson || isTextBased
+    }
+
+}
+
+extension String {
+
+    func containsOneOf(_ strings: [String]) -> Bool {
+        strings.contains { self.contains($0) }
+    }
+
 }

Sources/GoodNetworking/Session/DataTaskProxy.swift

@@ -54,12 +54,14 @@ import Foundation
             self.receivedError = URLError(.unknown)
         }
 
-        logger.logNetworkEvent(
-            message: prepareRequestInfo(),
-            level: receivedError == nil ? .debug : .warning,
-            file: #file,
-            line: #line
-        )
+        Task { @NetworkActor in
+            logger.logNetworkEvent(
+                message: prepareRequestInfo(),
+                level: receivedError == nil ? .debug : .warning,
+                file: #file,
+                line: #line
+            )
+        }
 
         continuation?.resume()
         continuation = nil

Sources/GoodNetworking/Session/NetworkSession.swift

@@ -67,7 +67,7 @@ import Foundation
         self.configuration = configuration
         self.delegateQueue = operationQueue
 
-        // create URLSession lazily, isolated on @NetworkActor, when requested first time
+        // create URLSession lazily, isolated to @NetworkActor, when requested first time
 
         super.init()
     }