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

Bugfix - Session Delegate Precondition #2783

Merged
merged 1 commit into from Apr 6, 2019

Conversation

Projects
None yet
3 participants
@cnoon
Copy link
Member

commented Apr 4, 2019

This PR removes the URL session delegate precondition check.

Goals ⚽️

Users should be able to create Session instances alongside performance monitoring frameworks such as NewRelic that are swizzling our URLSessionDelegate APIs by proxying.

Implementation Details 🚧

While the session delegate precondition check is a good idea, enforcing the check causes Alamofire to be unusable with other frameworks that are swizzling out URL session delegate APIs. In cases where say Alamofire and NewRelic are being used within the same application, the precondition check will always fail.

Screen Shot 2019-04-02 at 5 34 42 PM

The above example demonstrates what you get when you po the session.delegate and delegate inside the precondition removed in this PR. The session.delegate is an NRMAURLSessionTaskDelegate (courtesy of NewRelic proxy swizzling) and the delegate is an Alamofire.SessionDelegate.

I'm open to other ideas here as well. We could add a flag to disable the precondition that's false by default, but I thought it might be overkill to expose this precondition controls through the public API.

Testing Details 🔍

N/A.

@cnoon cnoon requested a review from jshier Apr 4, 2019

@cnoon cnoon self-assigned this Apr 4, 2019

@cnoon cnoon added this to the 5.0.0-beta.5 milestone Apr 4, 2019

@jshier

jshier approved these changes Apr 4, 2019

Copy link
Contributor

left a comment

As we discussed, there's no good solution here. I'm okay just removing it for now and seeing if it becomes a problem before implement one of the bad workarounds.

@cnoon cnoon merged commit 8efd2d9 into master Apr 6, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

@cnoon cnoon deleted the bugfix/session-delegate-precondition branch Apr 6, 2019

@tariq235

This comment has been minimized.

Copy link

commented Apr 11, 2019

My 2 year old code started crashing on Alamofire 5.
I am using AlamofireObjectMapper "6.0.0" which install Alamofire at "5.0.0-beta.2"

Screen Shot 2019-04-11 at 1 00 41 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.