NSInternalConsistencyException thrown when using CLLocationManager.requestLocation in 3.3.0

[jamiepinkham]

Short description of the issue:

Sample project demonstrating the issue can be found here
We are using a DelegateProxy to wrap CLLocationManagerDelegate, found here, and have added support for calling CLLocationManager.requestLocation to get a one time location update as you can see here. In previous versions of RxSwift, this worked as expected. However, upon updating to 3.3.0 this code now crashes with an NSInternalConsistencyException because the DelegateProxy does not implement locationManager:didFailWithError:. Despite the method being declared as optional, the documentation states:

When using this method, the associated delegate must implement the location​Manager(:​did​Update​Locations:​) and location​Manager(:​did​Fail​With​Error:​) methods. Failure to do so is a programmer error.

A quick fix for us was to implement the locationManager:didFailWithError: on the delegate proxy, but I'm not sure how to forward those calls such that the reactive extension's didFailWithError observable is triggered. I suspect it's similar to how it's implemented for UIScrollView's delegate proxy.

I think this is somehow related to the changes made for these issues: #1081 #1087

I don't think this is a bug per se in RxSwift, but a bug in my implementation of the DelegateProxy for CLLocationManager, and is also latent in the implementation in the DelegateProxy found in the example code.

Expected outcome:

The delegate proxy for CLLocationManager should be updated to handle the locationManager:didFailWithError: delegate method.
We should update the examples to demonstrate the correct usage of required delegate methods in delegate proxies.

What actually happens:

Using the delegate proxy in the sample code, and calling requestLocation without first obtaining location permissions crashes with an NSInternalConsistencyException

RxSwift

3.3.0

Platform/Environment

• iOS
• macOS
• tvOS
• watchOS
• playgrounds

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

• easy, 100% repro
• sometimes, 10%-100%
• hard, 2% - 10%
• extremely hard, %0 - 2%

Xcode version:

8.2.1

Installation method:

• CocoaPods
• Carthage
• Git submodules

I have multiple versions of Xcode installed:
(so we can know if this is a potential cause of your issue)

• yes (which ones)
• no

Level of RxSwift knowledge:
(this is so we can understand your level of knowledge
and formulate the response in an appropriate manner)

• just starting
• I have a small code base
• I have a significant code base

[kzaher]

Hi @jamiepinkham ,

A quick fix for us was to implement the locationManager:didFailWithError: on the delegate proxy, but I'm not sure how to forward those calls such that the reactive extension's didFailWithError observable is triggered. I suspect it's similar to how it's implemented for UIScrollView's delegate proxy.

I think that delegate proxy is behaving as designed now. The method is optional and if there aren't any observers registered it doesn't auto implement it. That seems reasonable to me.

I seems to me that this API is exception to the rule. Implementing this method on delegate proxy should should solve this problem.

It's just a matter of adding PublishSubject to delegate proxy like we've done for contentOffsetPublishSubject in RxScrollViewDelegateProxy.

[jamiepinkham]

It's just a matter of adding PublishSubject to delegate proxy like we've done for contentOffsetPublishSubject in RxScrollViewDelegateProxy.

What'd be really nice if there was a way in which we could somehow fold those error events into the didUpdateLocations Observable, such that the errors are delivered as part of that Observable. The way it's structured now seems not idiomatic seeing has you have to subscribe to two observables: locationManager.rx.didUpdateLocations and locationManager.rx.didFailWithError despite those two being related. (failing to update locations should be an error event in the update locations observable).

[kzaher]

Observable.merge(locationManager.rx.didUpdateLocations,  locationManager.rx.didFailWithError.map { e -> [CLLocation]  in throw e })`

[jamiepinkham]

Awesome! Thanks @kzaher!

[tomburns]

@kzaher should we update the RxExample implementation of this delegate proxy? It's currently broken as described above. I was able to update ours to work similarly to the UIScrollView one, but the RxExample code is currently a bit misleading.

[phlippieb]

@tomburns I for one would really appreciate that 😄

[sergdort]

PRs are always welcome guys 😜

[phlippieb]

#1207 #1208 fixes it for me