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.

Sign up for GitHub

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

Jump to bottom

No Response Fix #314

Merged
merged 8 commits into from
Nov 21, 2018
Merged

No Response Fix #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
696f413
Fixing issue where erroneous (such as 413) responses from API can res…
Nov 20, 2018
107f8b7
Adding 413 scenario to "Informative error" generating logic.
Nov 20, 2018
8b90a2a
Changing MBDirectionsErrorDomain string over to Apple-suggested schema.
Nov 20, 2018
b07cf0d
oops
Nov 21, 2018
1ba0e64
Adding tests that ensure handling known and unknown bad responses.
Nov 21, 2018
9addf1e
Reversing now-unneeded sanity check, as root cause was fixed deeper down
Nov 21, 2018
a0b9a3c
Copyedited failure messages
1ec5 Nov 21, 2018
188e546
No need to return the raw response
1ec5 Nov 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions MapboxDirections/MBDirections.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
typealias JSONDictionary = [String: Any]

/// Indicates that an error occurred in MapboxDirections.
public let MBDirectionsErrorDomain = "MBDirectionsErrorDomain"
public let MBDirectionsErrorDomain = "com.mapbox.directions.ErrorDomain"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before anyone asks, this is a great (and rather rare) example of "Do as I say, not as I do" from Apple:

You can create your own error domains and error codes for use in your own frameworks, or even in your own applications. It is recommended that the string constant for the domain be of the form com.company.framework_or_app.ErrorDomain.

Doc Link


/// The Mapbox access token specified in the main application bundle’s Info.plist.
let defaultAccessToken = Bundle.main.object(forInfoDictionaryKey: "MGLMapboxAccessToken") as? String
Expand Down Expand Up @@ -249,7 +249,7 @@ open class Directions: NSObject {

let apiStatusCode = json["code"] as? String
let apiMessage = json["message"] as? String
guard data != nil && error == nil && ((apiStatusCode == nil && apiMessage == nil) || apiStatusCode == "Ok") else {
guard !json.isEmpty, data != nil, error == nil && ((apiStatusCode == nil && apiMessage == nil) || apiStatusCode == "Ok") else {
let apiError = Directions.informativeError(describing: json, response: response, underlyingError: error as NSError?)
DispatchQueue.main.async {
errorHandler(apiError)
Expand Down Expand Up @@ -299,6 +299,11 @@ open class Directions: NSObject {
case (404, "ProfileNotFound"):
failureReason = "Unrecognized profile identifier."
recoverySuggestion = "Make sure the profileIdentifier option is set to one of the provided constants, such as MBDirectionsProfileIdentifierAutomobile."

case (413, _):
failureReason = "The request is too large."
recoverySuggestion = "Try specifying fewer waypoints or giving the waypoints shorter names."

case (429, _):
if let timeInterval = response.rateLimitInterval, let maximumCountOfRequests = response.rateLimit {
let intervalFormatter = DateComponentsFormatter()
Expand Down
70 changes: 69 additions & 1 deletion MapboxDirectionsTests/DirectionsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,32 @@ import OHHTTPStubs
@testable import MapboxDirections

let BogusToken = "pk.feedCafeDadeDeadBeef-BadeBede.FadeCafeDadeDeed-BadeBede"
let BadResponse = """
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<HTML><HEAD><META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=iso-8859-1">
<TITLE>ERROR: The request could not be satisfied</TITLE>
</HEAD><BODY>
<H1>413 ERROR</H1>
<H2>The request could not be satisfied.</H2>
<HR noshade size="1px">
Bad request.

<BR clear="all">
<HR noshade size="1px">
<PRE>
Generated by cloudfront (CloudFront)
Request ID: RAf2XH13mMVxQ96Z1cVQMPrd-hJoVA6LfaWVFDbdN2j-J1VkzaPvZg==
</PRE>
<ADDRESS>
</ADDRESS>
</BODY></HTML>
"""

class DirectionsTests: XCTestCase {
override func setUp() {
// Make sure tests run in all time zones
NSTimeZone.default = TimeZone(secondsFromGMT: 0)!
}

override func tearDown() {
OHHTTPStubs.removeAllStubs()
super.tearDown()
Expand All @@ -21,6 +40,55 @@ class DirectionsTests: XCTestCase {
XCTAssertEqual(directions.apiEndpoint.absoluteString, "https://api.mapbox.com")
}

func testKnownBadResponse() {
let pass = "The operation couldn’t be completed. The request is too large."

OHHTTPStubs.stubRequests(passingTest: { (request) -> Bool in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testRateLimitErrorParsing() is a lot more focused, testing just Directions.informativeError(describing:response:underlyingError:) without need to stub out anything.

return request.url!.absoluteString.contains("https://api.mapbox.com/directions")
}) { (_) -> OHHTTPStubsResponse in
return OHHTTPStubsResponse(data: BadResponse.data(using: .utf8)!, statusCode: 413, headers: ["Content-Type" : "text/html"])
}
let expectation = XCTestExpectation(description: "Async callback")
let one = CLLocation(coordinate: CLLocationCoordinate2D(latitude: 0.0, longitude: 0.0))
let two = CLLocation(coordinate: CLLocationCoordinate2D(latitude: 2.0, longitude: 2.0))

let directions = Directions(accessToken: BogusToken)
let opts = RouteOptions(locations: [one, two])
directions.calculate(opts, completionHandler: { (waypoints, routes, error) in
expectation.fulfill()
XCTAssertNil(routes, "Unexpected route response")
XCTAssertNotNil(error, "No error returned")
XCTAssertNil(error?.userInfo[NSUnderlyingErrorKey])
XCTAssertEqual(error?.localizedDescription, pass, "Wrong type of error received")
})
wait(for: [expectation], timeout: 2.0)
}

func testUnknownBadResponse() {
let pass = "The operation couldn’t be completed. server error"

OHHTTPStubs.stubRequests(passingTest: { (request) -> Bool in
return request.url!.absoluteString.contains("https://api.mapbox.com/directions")
}) { (_) -> OHHTTPStubsResponse in
let message = "Enhance your calm, John Spartan."
return OHHTTPStubsResponse(data: message.data(using: .utf8)!, statusCode: 420, headers: ["Content-Type" : "text/plain"])
}
let expectation = XCTestExpectation(description: "Async callback")
let one = CLLocation(coordinate: CLLocationCoordinate2D(latitude: 0.0, longitude: 0.0))
let two = CLLocation(coordinate: CLLocationCoordinate2D(latitude: 2.0, longitude: 2.0))

let directions = Directions(accessToken: BogusToken)
let opts = RouteOptions(locations: [one, two])
directions.calculate(opts, completionHandler: { (waypoints, routes, error) in
expectation.fulfill()
XCTAssertNil(routes, "Unexpected route response")
XCTAssertNotNil(error, "No error returned")
XCTAssertNil(error?.userInfo[NSUnderlyingErrorKey])
XCTAssertEqual(error?.localizedDescription, pass, "Wrong type of error received")
})
wait(for: [expectation], timeout: 2.0)
}

func testRateLimitErrorParsing() {
let json = ["message" : "Hit rate limit"]

Expand Down