Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the new URLSession async/await APIs #113

Open
vdka opened this issue Sep 17, 2022 · 17 comments
Open

Add support for the new URLSession async/await APIs #113

vdka opened this issue Sep 17, 2022 · 17 comments
Labels
enhancement New feature or request

Comments

@vdka
Copy link
Contributor

vdka commented Sep 17, 2022

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
        }
    }
}
@kean
Copy link
Owner

kean commented Sep 17, 2022

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 kean added the enhancement New feature or request label Sep 17, 2022
@kean kean changed the title Requests stuck Pending Add support for the new URLSession async/await APIs Sep 17, 2022
@vdka
Copy link
Contributor Author

vdka commented Sep 17, 2022

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()
    }
}

@stremblayiOS
Copy link

+1

@kean
Copy link
Owner

kean commented Oct 25, 2022

@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.

@stremblayiOS
Copy link

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/

@alexchernokun
Copy link

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?

@vdka
Copy link
Contributor Author

vdka commented Oct 31, 2022

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

@ffittschen
Copy link

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.

@tahirmt
Copy link
Contributor

tahirmt commented Feb 2, 2023

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

@jgodonQobuz
Copy link

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

@lukeredpath
Copy link

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
}

@MrAsterisco
Copy link

MrAsterisco commented Apr 14, 2023

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()
  }
}

@alexdremov
Copy link

alexdremov commented May 10, 2023

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!
    }
}

@salmin-skelia
Copy link

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
Copy link

salmin-skelia commented Oct 16, 2023

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()
        })
    }
}

@vitys96
Copy link

vitys96 commented Dec 5, 2023

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

@salmin-skelia
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests