fix: the pending status issue

[eligutovsky]

Description

This pull request proposes a solution for requests with 'Pending' status that never ends.

Issue

The issue arises when using dataTask(with:completionHandler:) with a non-nil completionHandler. In this case, the delegate method urlSession(:task:didCompleteWithError:) will not be called. Apple's documentation describes this behavior.

The completion handler will call when the load request is complete. This handler is executed on the delegate queue.

If you pass nil, only the session delegate methods are called when the task completes, making this method equivalent to the dataTask(with:) method.

Source

The description does not explicitly convey the behavior, but it provides some context.

Solution

The idea is to move the completion of a network task to urlSession(:task:didFinishCollecting:) method, by copying the implementation of urlSession(:task:didCompleteWithError:).

As the next step, I would like to kindly request your professional opinion regarding the potential implementation of the urlSession(:task:didCompleteWithError:) method for use in the urlSession(:task:didFinishCollecting:) operation. Your valuable insights on this matter would be greatly appreciated.

Connected issues and discussions:

• https://github.com/kean/Pulse/issues/212
• https://github.com/kean/Pulse/discussions/112

Bonus

This solution solves async/await support

• https://github.com/kean/Pulse/issues/113

[kean]

Hey, @eligutovsky. That's a neat idea, and I appreciate you taking a stab at it! It's been a major pain-point when integrating Pulse.

Are the response data and task.error populated by the time the urlSession(:task:didFinishCollecting:) is called?

let data = context.data

[eligutovsky]

Hey, @eligutovsky. That's a neat idea, and I appreciate you taking a stab at it! It's been a major pain-point when integrating Pulse.

Are the response data and task.error populated by the time the urlSession(:task:didFinishCollecting:) is called?

let data = context.data

Hi @kean,

I wanted to update you regarding the task.error property that you mentioned earlier. I've checked and found that the property is successfully fulfilled with an actual error during the metrics collection stage. However, I'm facing some issues while collecting response data and have tried using URLCache.shared.cachedResponse(for: request). Unfortunately, it's not reliable enough. I also attempted swizzling #selector(URLSession.dataTask(with:completionHandler:)), but it crashed in an unexpected place:

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSMutableURLRequest isFileReferenceURL]: unrecognized selector sent to instance 0x600000020000'

I wanted to update you on the situation. Maybe you have any suggestions for improving this PR to meet the goal with the completionHandler? Otherwise, I would close this PR.

[kean]

I wanted to update you on the situation. Maybe you have any suggestions for improving this PR to meet the goal with the completionHandler?

I haven't invested much time looking into the async/await and/or completion handler support. The recommended approach so far has been using the experimental proxy based on a custom URL protocol or logging manually. I'm not a fan of the solution based on a custom protocol, hence why it never exited the "experimental" status.

There is likely a less invasive way to implement this with more swizzling. Proxyman went a bit further with swizzling in Atlantis – you may want to check it out. I briefly looked into it but ended up not following their approach and preferring the simplicity over the slight inconvenience during the setup process.

[eligutovsky]

I wanted to update you on the situation. Maybe you have any suggestions for improving this PR to meet the goal with the completionHandler?

I haven't invested much time looking into the async/await and/or completion handler support. The recommended approach so far has been using the experimental proxy based on a custom URL protocol or logging manually. I'm not a fan of the solution based on a custom protocol, hence why it never exited the "experimental" status.

There is likely a less invasive way to implement this with more swizzling. Proxyman went a bit further with swizzling in Atlantis – you may want to check it out. I briefly looked into it but ended up not following their approach and preferring the simplicity over the slight inconvenience during the setup process.

Unfortunately, the "experimental solution" does not work well enough for a project that I'm working on 😔 The Atlantis solution with the swizzling works ok. Closing this PR.

Thank you @kean