Pulse icon indicating copy to clipboard operation
Pulse copied to clipboard

Add support for the new URLSession async/await APIs

Open vdka opened this issue 3 years ago • 17 comments

Simple integration using URLSessionProxyDelegate on iOS 16.0 is creating the request in logs, but they are stuck "Pending". I did a little bit of debugging and it seems urlSession(_:task:didCompleteWithError:) isn't being called by the system.

Reproduction

import SwiftUI
import Pulse
import PulseUI

struct API {
    let session: URLSession = {
        let config = URLSessionConfiguration.default
        let delegate = URLSessionProxyDelegate()
        return URLSession(configuration: config, delegate: delegate, delegateQueue: nil)
    }()

    func fetchTodos() async throws -> Data {
        let url = URL(string: "https://jsonplaceholder.typicode.com/todos/1")!
        let (data, _) = try await session.data(from: url)
        return data
    }
}

struct ContentView: View {
    let api = API()

    @State var isLoading = false
    @State var data: Data?

    @State var showPulse = false

    var body: some View {
        VStack {
            if let data {
                Text("\(data.count) bytes of data")
            } else if isLoading {
                ProgressView("Loading...")
            } else {
                Text("No data")
            }

            Button("Fetch Todos", action: fetchTodos).buttonStyle(.borderedProminent)

            Button("Show PulseUI", action: { showPulse = true }).buttonStyle(.bordered)
        }
        .sheet(isPresented: $showPulse, content: {
            PulseUI.MainView()
        })
        .padding()
    }

    func fetchTodos() {
        isLoading = true
        Task {
            self.data = try? await api.fetchTodos()
            isLoading = false
        }
    }
}

vdka avatar Sep 17 '22 02:09 vdka

I'm pretty sure it used to work in the previous versions, but yes, I just tested it, and the new async/await APIs on URLSession such as data(for:) are not supported. The issue is that almost none of the delegate methods get called for tasks created using these methods. Crucially, urlSession(_:task:didCompleteWithError:) doesn't. I'm not sure if it's possible to capture these automatically.

kean avatar Sep 17 '22 20:09 kean

I thought that might be the case so I tried to work around it with the following but it runs into the same issue, which makes me think the issue is more related to the new concurrency system then URLSession's concurrency methods themselves.

func fetchTodos() async throws -> Data {
    let url = URL(string: "https://jsonplaceholder.typicode.com/todos/1")!

    // let (data, _) = try await session.data(from: url)
    // return data
    return try await withCheckedThrowingContinuation { continuation in
        let task = session.dataTask(with: URLRequest(url: url)) { data, _, error in
            if let error {
                continuation.resume(throwing: error)
                return
            }
            continuation.resume(returning: data ?? Data())
        }
        task.resume()
    }
}

vdka avatar Sep 17 '22 21:09 vdka

+1

stremblayiOS avatar Oct 25 '22 20:10 stremblayiOS

@vdka , the completion-based methods also don't call the delegate. There is probably a way to swizzle some of the URLSession method to make it work automatically.

kean avatar Oct 25 '22 22:10 kean

I believe we have to use URLSessionTaskDelegate in order to get the call back. Some people are discussing the issue here: https://www.reddit.com/r/swift/comments/ptpxf7/swift_55_async_await_issue_with_delegates_using/

stremblayiOS avatar Oct 26 '22 16:10 stremblayiOS

The same "pending" issue appears if you use Combine dataTaskPublisher. It seems like dataTaskPublisher doesn't invoke the delegate methods both for URLSessionTaskDelegate and URLSessionDataDelegate. Maybe someone also have this issue?

alexchernokun avatar Oct 26 '22 21:10 alexchernokun

I'd love to make a PR to fix this, however I am not likely to get the time. If anybody is interested in looking into this, I know ProxymanApp/Atlantis appears to be unaffected. It appears to Swizzle internal Method's and so AppReview would likely not approve, however, could be offered as an option similar to the existing Experimental integration approach

vdka avatar Oct 31 '22 02:10 vdka

I'm assuming the URLSession.DataTaskPublisher internally uses the completionHandler based dataTask method. Apple's Docs state that if a completionHandler is set, the delegate methods will not be called.

ffittschen avatar Dec 02 '22 15:12 ffittschen

For the completion handler case, is it possible to tell the pulse logger after the completion block is called that the request is complete?

tahirmt avatar Feb 02 '23 00:02 tahirmt

Hi @kean

I've exactly the same problem with "new" Swift concurrency URLSession as requests are always shown as "Pending..." when they are done like this:

let (data, _) = try await session.data(from: url)

As this is not possible/acceptable for our iOS team to modify all requests in another way than modern Swift concurrency try await session.data(from:), do you think that any fixes/refacto could be done in Pulse about this in a futur version (v3 ?) of your great and so useful tool ?

Thanks for your answer and again: Big up for all your work 👏

Have a nice day

Jeremie

jgodonQobuz avatar Feb 16 '23 09:02 jgodonQobuz

The experimental URL protocol stuff wasn't working for me so I came up with a different workaround for our API client implementation.

I've only tested this with data tasks, but it looks like it's the final didCompleteWithError call that is missing when you use the async API. To work around this, I created my own URLSessionTaskDelegate/URLSessionDataDelegate proxy that ultimately ends up wrapping the Pulse URLSessionProxyDelegate that I pass in to my API client. The only difference is that this proxy:

  • Has a reference to the URLSession being used by my API client.
  • Stores a reference to the session task it gets given by the urlSession:didCreateTask callback.
  • Expose a single public method called notifyCompletion(error: Error?) that calls the urlSession:task:didCompleteWithError: delegate method on the proxied delegate using the session and task reference it already has and the error provided.

Every time my API client starts a new request, it creates a new instance of my proxy delegate, passing in a reference to the session and the actual delegate. It then passes this to the async function e.g. try await session.data(for: request, delegate: proxyDelegate) - I wrap this in a do/catch block and call notifyCompletion, as a rough example:

do {
  let proxyDelegate = ProxyDelegate(session: session, actualDelegate: actualDelegate)
  let (data, response) = try await session.data(for: request, delegate: proxyDelegate)
  proxyDelegate.notifyCompletion(error: nil)
  return (data, response)
} catch {
  proxyDelegate.notifyCompletion(error: error)
  throw error
}

lukeredpath avatar Mar 27 '23 18:03 lukeredpath

Just adding my workaround here: if you're using URLSession.dataTaskPublisher, you can replace it with this extension and you'll at least get network call logs (no metrics, though).

It relies on creating a fake dataTask and then subscribing to events of the dataTaskPublisher, as the publisher itself doesn't expose its dataTask instance.

extension URLSession {
  func trackedDataRequest(for urlRequest: URLRequest, with networkLogger: NetworkLogger) -> AnyPublisher<DataTaskPublisher.Output, DataTaskPublisher.Failure> {
    let dataTask = dataTask(with: urlRequest)
    
    return dataTaskPublisher(for: urlRequest)
      .handleEvents(
        receiveSubscription: { _ in
          networkLogger.logTaskCreated(dataTask)
        },
        receiveOutput: { output in
          networkLogger.logDataTask(dataTask, didReceive: output.data)
        },
        receiveCompletion: { completion in
          switch completion {
          case .finished:
            networkLogger.logTask(dataTask, didCompleteWithError: nil)
          case let .failure(error):
            networkLogger.logTask(dataTask, didCompleteWithError: error)
          }
        }
      )
      .eraseToAnyPublisher()
  }
}

MrAsterisco avatar Apr 14 '23 16:04 MrAsterisco

For me, solution was to transform codebase to use URLSessionProtocol protocol. It used by default in API generated by openapi-generator, and URLSession conforms to it, so Pulse can be easily removed in production

public protocol URLSessionProtocol {
    func dataTask(with request: URLRequest, completionHandler: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask
}

Then, Pulse can be easily integrated:

class URLSessionPulse: URLSessionProtocol {
    let session: URLSession
    let networkLogger: NetworkLogger
    
    init(session: URLSession) {
        self.session = session
        self.networkLogger = NetworkLogger()
    }
    
    func dataTask(with request: URLRequest, completionHandler: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
        var task: URLSessionDataTask?
        let onReceive: (Data?, URLResponse?, Error?) -> Void = { (data, response, error) in
            if let task {
                if let data {
                    self.networkLogger.logDataTask(task, didReceive: data)
                }
                self.networkLogger.logTask(task, didCompleteWithError: error)
            }
        }
        
        task = session.dataTask(with: request) {data, response, error in
            onReceive(data, response, error)
            completionHandler(data, response, error)
        }
        if let task {
            self.networkLogger.logTaskCreated(task)
        }
        return task!
    }
}

alexdremov avatar May 10 '23 18:05 alexdremov

According to docs, if you're specifying completion handler (use async version that uses it under the hood or use dataTaskPublished that uses completion handler as well) it will ignore data delegate note section of docs. I wonder how it worked before?

salmin-skelia avatar Oct 16 '23 12:10 salmin-skelia

For me, solution was to transform codebase to use URLSessionProtocol protocol. It used by default in API generated by openapi-generator, and URLSession conforms to it, so Pulse can be easily removed in production

public protocol URLSessionProtocol {
    func dataTask(with request: URLRequest, completionHandler: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask
}

Then, Pulse can be easily integrated:

class URLSessionPulse: URLSessionProtocol {
    let session: URLSession
    let networkLogger: NetworkLogger
    
    init(session: URLSession) {
        self.session = session
        self.networkLogger = NetworkLogger()
    }
    
    func dataTask(with request: URLRequest, completionHandler: @escaping @Sendable (Data?, URLResponse?, Error?) -> Void) -> URLSessionDataTask {
        var task: URLSessionDataTask?
        let onReceive: (Data?, URLResponse?, Error?) -> Void = { (data, response, error) in
            if let task {
                if let data {
                    self.networkLogger.logDataTask(task, didReceive: data)
                }
                self.networkLogger.logTask(task, didCompleteWithError: error)
            }
        }
        
        task = session.dataTask(with: request) {data, response, error in
            onReceive(data, response, error)
            completionHandler(data, response, error)
        }
        if let task {
            self.networkLogger.logTaskCreated(task)
        }
        return task!
    }
}

More generic solution that uses URLSessionDataDelegate and checks cancellation that worked for me.

extension URLSession {
    /// Allows to track `URLSessionDataDelegate` using closure based call.
    /// By default if you use async interface or `completionHandler` based interface,
    /// URLSession won't notify `URLSessionDataDelegate`.
    public func dataTask(for request: URLRequest) async throws -> (Data, URLResponse) {
        var dataTask: URLSessionDataTask?

        let onSuccess: (Data, URLResponse) -> Void = { (data, response) in
            guard let dataTask, let dataDelegate = self.delegate as? URLSessionDataDelegate else {
                return
            }
            dataDelegate.urlSession?(self, dataTask: dataTask, didReceive: data)
            dataDelegate.urlSession?(self, task: dataTask, didCompleteWithError: nil)
        }
        let onError: (Error) -> Void = { error in
            guard let dataTask, let dataDelegate = self.delegate as? URLSessionDataDelegate else {
                return
            }
            dataDelegate.urlSession?(self, task: dataTask, didCompleteWithError: error)

        }
        let onCancel = {
            dataTask?.cancel()
        }

        return try await withTaskCancellationHandler(operation: {
            try await withCheckedThrowingContinuation { continuation in
                dataTask = self.dataTask(with: request) { data, response, error in
                    guard let data = data, let response = response else {
                        let error = error ?? URLError(.badServerResponse)
                        onError(error)
                        return continuation.resume(throwing: error)
                    }
                    onSuccess(data, response)
                    continuation.resume(returning: (data, response))
                }
                dataTask?.resume()
            }
        }, onCancel: {
            onCancel()
        })
    }
}

salmin-skelia avatar Oct 16 '23 15:10 salmin-skelia

@salmin-skelia where delegate (URLSessionDataDelegate) is used? can you show all code please?

vitys96 avatar Dec 05 '23 16:12 vitys96

@salmin-skelia where delegate (URLSessionDataDelegate) is used? can you show all code please?

What do you mean by where? You just pass URLSessionDataDelegate as delegate for your session like URLSession(configuration: sessionConfiguration, delegate: sessionDelegate, delegateQueue: nil) and sessionDelegate using snipped I liked will receive appropriate action if sessionDelegate will conform URLSessionDataDelegate.

salmin-skelia avatar Dec 06 '23 10:12 salmin-skelia