geocodeWithOptions:completionHandler: crash on features unwrap

[friedbunny]

geocodeWithOptions:completionHandler: in v0.6.1 can fail to force-unwrap the returned JSON feature collection — MBGeocoder.swift#L179.

Thread 0 Crashed:
0   SomeApp		0x00000001001bca14 function signature specialization <Arg[1] = Owned To Guaranteed> of SomeApp.Geocoder.(geocode (SomeApp.GeocodeOptions, completionHandler : ([SomeApp.GeocodedPlacemark]?, Swift.String?, __ObjC.NSError?) -> ()) -> __ObjC.URLSessionDataTask).(closure #1) (MBGeocoder.swift:179)
1   SomeApp		0x00000001001b8b54 SomeApp.Geocoder.(geocode (SomeApp.GeocodeOptions, completionHandler : ([SomeApp.GeocodedPlacemark]?, Swift.String?, __ObjC.NSError?) -> ()) -> __ObjC.URLSessionDataTask).(closure #1) (MBGeocoder.swift:0)
2   SomeApp		0x00000001001b9078 function signature specialization <Arg[1] = Value Promoted from Box> of SomeApp.Geocoder.((dataTaskWithURL in _5EEF66B4503CFF193FAFED8963214E26) (Foundation.URL, completionHandler : (protocol<>) -> (), errorHandler : (__ObjC.NSError) -> ()) -> __ObjC.URLSessionDataTask).(closure #1).(closure #2) (MBGeocoder.swift:252)
3   libdispatch.dylib	0x00000001914d21fc _dispatch_call_block_and_release + 20
...

/cc @1ec5

[1ec5]

@ingalls @mattficke, are there any situations where the Geocoding API would return a non-FeatureCollection or a FeatureCollection that lacks a features array? (There's an assertion that we're dealing with a FeatureCollection, but assertions don't run in Release builds.)

[ingalls]

@1ec5 Not a valid reason no - On no results we simply return something like

{
    "type": "FeatureCollection",
    "features": []
}

If you run into someplace where this is not the case I would love to see the URL that generated it.

[mattficke]

@1ec5 not sure if this is relevant here, but batch queries to the permanent endpoint return an array of FeatureCollections.

[1ec5]

batch queries to the permanent endpoint return an array of FeatureCollections

We do account for that difference in batchGeocode(_:completionHandler:).

I’ve tried a number of possibilities but was unable to get the normal Geocoding API to return something that Swift couldn’t parse as a feature collection. If we remove the force-cast here, we’d still have to produce an error since it would be invalid data coming back from the API. (Optionally casting could obscure the error and cause an issue that’s more difficult to debug later.)

[postmechanical]

@1ec5 We've pulled the code into our project manually, adjusted the code to avoid force-unwraps since we don't allow force-unwraps in our code base (except in rare cases), and invoke the completion blocks with a new "failed to parse json" error instead.

[1ec5]

That sounds like a good solution for this project. Please let us know if you continued to run into that error in the wild (with details about the request, if possible).

[1ec5]

The crash in #106 (comment) was fixed by the migration to Codable in #127: now a malformed GeoJSON response will cause the error handler to be invoked instead of crashing. Please open separate issues if you encounter another situation where this library crashes on malformed input.