From d54c407c862eee325d17cd467772464fa7b82f20 Mon Sep 17 00:00:00 2001 From: Alex Azarov Date: Wed, 27 Mar 2024 14:21:41 +0100 Subject: [PATCH 1/5] Improve error handing in case of invalid route responses --- .../LegacyRouteController.swift | 9 +++---- .../{Result+Expected.swift => Result++.swift} | 9 +++++++ .../RouteController.swift | 24 +++++++++++-------- Sources/MapboxCoreNavigation/Router.swift | 2 +- 4 files changed, 29 insertions(+), 15 deletions(-) rename Sources/MapboxCoreNavigation/{Result+Expected.swift => Result++.swift} (83%) diff --git a/Sources/MapboxCoreNavigation/LegacyRouteController.swift b/Sources/MapboxCoreNavigation/LegacyRouteController.swift index 249f620f920..9d3ce9ff09e 100644 --- a/Sources/MapboxCoreNavigation/LegacyRouteController.swift +++ b/Sources/MapboxCoreNavigation/LegacyRouteController.swift @@ -151,15 +151,16 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa updateRoute(with: indexedRouteResponse, routeOptions: routeOptions, isProactive: false, - isAlternative: false, - completion: completion) + isAlternative: false) { result in + completion?(result.isSuccess) + } } func updateRoute(with indexedRouteResponse: IndexedRouteResponse, routeOptions: RouteOptions?, isProactive: Bool, isAlternative: Bool, - completion: ((Bool) -> Void)?) { + completion: ((Result) -> Void)?) { guard let route = indexedRouteResponse.currentRoute else { preconditionFailure("`indexedRouteResponse` does not contain route for index `\(indexedRouteResponse.routeIndex)` when updating route.") } @@ -167,7 +168,7 @@ open class LegacyRouteController: NSObject, Router, InternalRouter, CLLocationMa routeProgress = RouteProgress(route: route, options: routeOptions) self.indexedRouteResponse = indexedRouteResponse announce(reroute: route, at: location, proactive: isProactive) - completion?(true) + completion?(.success(())) } diff --git a/Sources/MapboxCoreNavigation/Result+Expected.swift b/Sources/MapboxCoreNavigation/Result++.swift similarity index 83% rename from Sources/MapboxCoreNavigation/Result+Expected.swift rename to Sources/MapboxCoreNavigation/Result++.swift index 0fb0d4e76f5..25fe6eb47e6 100644 --- a/Sources/MapboxCoreNavigation/Result+Expected.swift +++ b/Sources/MapboxCoreNavigation/Result++.swift @@ -18,4 +18,13 @@ extension Result { preconditionFailure("Expected type is neither a value nor an error.") } } + + var isSuccess: Bool { + switch self { + case .success: + return true + case .failure: + return false + } + } } diff --git a/Sources/MapboxCoreNavigation/RouteController.swift b/Sources/MapboxCoreNavigation/RouteController.swift index 00704820f1b..042b6d76125 100644 --- a/Sources/MapboxCoreNavigation/RouteController.swift +++ b/Sources/MapboxCoreNavigation/RouteController.swift @@ -867,14 +867,17 @@ extension RouteController: Router { case let .success(indexedResponse): let response = indexedResponse.routeResponse guard case let .route(routeOptions) = response.options else { - //TODO: Can a match hit this codepoint? + announceReroutingError(with: ReroutingError.routeError) self.isRerouting = false; return } self.updateRoute(with: indexedResponse, routeOptions: routeOptions, isProactive: false, - isAlternative: false) { [weak self] success in + isAlternative: false) { [weak self] result in self?.isRerouting = false + if case let .failure(error) = result { + self?.announceReroutingError(with: error) + } } case let .failure(error): self.announceReroutingError(with: error) @@ -890,15 +893,16 @@ extension RouteController: Router { updateRoute(with: indexedRouteResponse, routeOptions: routeOptions, isProactive: false, - isAlternative: false, - completion: completion) + isAlternative: false) { result in + completion?(result.isSuccess) + } } func updateRoute(with indexedRouteResponse: IndexedRouteResponse, routeOptions: RouteOptions?, isProactive: Bool, isAlternative: Bool, - completion: ((Bool) -> Void)?) { + completion: ((Result) -> Void)?) { guard let route = indexedRouteResponse.currentRoute else { preconditionFailure("`indexedRouteResponse` does not contain route for index `\(indexedRouteResponse.routeIndex)` when updating route.") } @@ -926,9 +930,9 @@ extension RouteController: Router { self.announce(reroute: route, at: self.location, proactive: isProactive) self.indexedRouteResponse = indexedRouteResponse self.didProactiveReroute = isProactive - completion?(true) - case .failure: - completion?(false) + completion?(.success(())) + case .failure(let error): + completion?(.failure(error)) } } } @@ -1014,11 +1018,11 @@ extension RouteController: ReroutingControllerDelegate { routeOptions: options, isProactive: false, isAlternative: true, - completion: { [weak self] success in + completion: { [weak self] result in guard let self = self else { return } var userInfo = [RouteController.NotificationUserInfoKey: Any]() userInfo[.locationKey] = self.location - if success { + if result.isSuccess { NotificationCenter.default.post(name: .routeControllerDidTakeAlternativeRoute, object: self, userInfo: userInfo) diff --git a/Sources/MapboxCoreNavigation/Router.swift b/Sources/MapboxCoreNavigation/Router.swift index 48c429722be..dc892ca3ce5 100644 --- a/Sources/MapboxCoreNavigation/Router.swift +++ b/Sources/MapboxCoreNavigation/Router.swift @@ -318,7 +318,7 @@ protocol InternalRouter: AnyObject { routeOptions: RouteOptions?, isProactive: Bool, isAlternative: Bool, - completion: ((Bool) -> Void)?) + completion: ((Result) -> Void)?) } extension InternalRouter where Self: Router { From befcd2f455a1318754ffc6b66448aa3b2e8fc363 Mon Sep 17 00:00:00 2001 From: Alex Azarov Date: Wed, 27 Mar 2024 14:41:53 +0100 Subject: [PATCH 2/5] Fix the order in which delegate is notified about failed reroute attempt --- Sources/MapboxCoreNavigation/RouteController.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/MapboxCoreNavigation/RouteController.swift b/Sources/MapboxCoreNavigation/RouteController.swift index 042b6d76125..062596b0641 100644 --- a/Sources/MapboxCoreNavigation/RouteController.swift +++ b/Sources/MapboxCoreNavigation/RouteController.swift @@ -867,8 +867,8 @@ extension RouteController: Router { case let .success(indexedResponse): let response = indexedResponse.routeResponse guard case let .route(routeOptions) = response.options else { - announceReroutingError(with: ReroutingError.routeError) - self.isRerouting = false; return + self.isRerouting = false + return announceReroutingError(with: ReroutingError.routeError) } self.updateRoute(with: indexedResponse, routeOptions: routeOptions, From 9fc81fe31a2ed45c1767da8bc78de4ee885df586 Mon Sep 17 00:00:00 2001 From: Alex Azarov Date: Wed, 27 Mar 2024 15:36:17 +0100 Subject: [PATCH 3/5] Fix for old Swift compiler versions --- Sources/MapboxCoreNavigation/RouteController.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/MapboxCoreNavigation/RouteController.swift b/Sources/MapboxCoreNavigation/RouteController.swift index 062596b0641..e2c4f2ef425 100644 --- a/Sources/MapboxCoreNavigation/RouteController.swift +++ b/Sources/MapboxCoreNavigation/RouteController.swift @@ -868,7 +868,8 @@ extension RouteController: Router { let response = indexedResponse.routeResponse guard case let .route(routeOptions) = response.options else { self.isRerouting = false - return announceReroutingError(with: ReroutingError.routeError) + self.announceReroutingError(with: ReroutingError.routeError) + return } self.updateRoute(with: indexedResponse, routeOptions: routeOptions, From a2290664aaeb2f94783a6811257b07e62198430c Mon Sep 17 00:00:00 2001 From: Alex Azarov Date: Wed, 27 Mar 2024 15:43:35 +0100 Subject: [PATCH 4/5] Fixed project structure in Carthage integration --- MapboxNavigation.xcodeproj/project.pbxproj | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MapboxNavigation.xcodeproj/project.pbxproj b/MapboxNavigation.xcodeproj/project.pbxproj index f02598f3e2e..5ce3764df68 100644 --- a/MapboxNavigation.xcodeproj/project.pbxproj +++ b/MapboxNavigation.xcodeproj/project.pbxproj @@ -169,7 +169,6 @@ 2E5ACEE8288E877A00300ECA /* EventsAPI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E5ACEE6288E877A00300ECA /* EventsAPI.swift */; }; 2E5ACEE9288E877A00300ECA /* EventConstants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E5ACEE7288E877A00300ECA /* EventConstants.swift */; }; 2E5ACEEB288E888500300ECA /* EventsAPIMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E5ACEEA288E888500300ECA /* EventsAPIMock.swift */; }; - 2E6656F9264EC912009463EE /* Result+Expected.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E6656F8264EC912009463EE /* Result+Expected.swift */; }; 2E82B9DC26E61F4600B7837F /* CongestionLevel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E82B9DB26E61F4600B7837F /* CongestionLevel.swift */; }; 2E82B9DE26E6237600B7837F /* CongestionLevelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E82B9DD26E6237600B7837F /* CongestionLevelTests.swift */; }; 2EBF20AE25D6F89000DB7BF2 /* Utils.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2EBF20AD25D6F89000DB7BF2 /* Utils.swift */; }; @@ -566,6 +565,7 @@ DAFA92071F01735000A7FB09 /* DistanceFormatter.swift in Sources */ = {isa = PBXBuildFile; fileRef = 351BEC0B1E5BCC72006FE110 /* DistanceFormatter.swift */; }; DF345E4129437EE800EC77A7 /* MBXInfo.plist in Resources */ = {isa = PBXBuildFile; fileRef = DF345E4029437EE800EC77A7 /* MBXInfo.plist */; }; DF345E4329437F1000EC77A7 /* Bundle+SDKVersion.swift in Sources */ = {isa = PBXBuildFile; fileRef = DF345E4229437F1000EC77A7 /* Bundle+SDKVersion.swift */; }; + DFBE62C82BB467E900CF830D /* Result++.swift in Sources */ = {isa = PBXBuildFile; fileRef = DFBE62C72BB467E900CF830D /* Result++.swift */; }; E20E3AC526C50AC6002E13EE /* BillingServiceMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = E20E3AC426C50AC5002E13EE /* BillingServiceMock.swift */; }; E20F43C326BBD0A600346E71 /* RouterDelegateSpy.swift in Sources */ = {isa = PBXBuildFile; fileRef = E20F43C226BBD0A600346E71 /* RouterDelegateSpy.swift */; }; E2108B30285866C200CB0875 /* NavigationLog.swift in Sources */ = {isa = PBXBuildFile; fileRef = E2108B2F285866C200CB0875 /* NavigationLog.swift */; }; @@ -859,7 +859,6 @@ 2E5ACEE6288E877A00300ECA /* EventsAPI.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EventsAPI.swift; sourceTree = ""; }; 2E5ACEE7288E877A00300ECA /* EventConstants.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EventConstants.swift; sourceTree = ""; }; 2E5ACEEA288E888500300ECA /* EventsAPIMock.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = EventsAPIMock.swift; sourceTree = ""; }; - 2E6656F8264EC912009463EE /* Result+Expected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Result+Expected.swift"; sourceTree = ""; }; 2E82B9DB26E61F4600B7837F /* CongestionLevel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CongestionLevel.swift; sourceTree = ""; }; 2E82B9DD26E6237600B7837F /* CongestionLevelTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CongestionLevelTests.swift; sourceTree = ""; }; 2EBF20AD25D6F89000DB7BF2 /* Utils.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Utils.swift; sourceTree = ""; }; @@ -1358,6 +1357,7 @@ DAFEB36F2093A3EF00A86A83 /* ko */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = ko; path = Resources/ko.lproj/Localizable.stringsdict; sourceTree = ""; }; DF345E4029437EE800EC77A7 /* MBXInfo.plist */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.xml; name = MBXInfo.plist; path = Resources/MBXInfo.plist; sourceTree = ""; }; DF345E4229437F1000EC77A7 /* Bundle+SDKVersion.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Bundle+SDKVersion.swift"; sourceTree = ""; }; + DFBE62C72BB467E900CF830D /* Result++.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "Result++.swift"; sourceTree = ""; }; E20E3AC426C50AC5002E13EE /* BillingServiceMock.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BillingServiceMock.swift; sourceTree = ""; }; E20F43C226BBD0A600346E71 /* RouterDelegateSpy.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RouterDelegateSpy.swift; sourceTree = ""; }; E2108B2F285866C200CB0875 /* NavigationLog.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NavigationLog.swift; sourceTree = ""; }; @@ -2498,7 +2498,7 @@ 5A39B9272498F9890026DFD1 /* PassiveLocationManager.swift */, 8A3A218F25EEC00200EDA999 /* CoreNavigationNavigator.swift */, E2805A5726CB994500165DB9 /* NSLock+MapboxInternal.swift */, - 2E6656F8264EC912009463EE /* Result+Expected.swift */, + DFBE62C72BB467E900CF830D /* Result++.swift */, E2C623A626CBFCE1005769FA /* OnMainQueue.swift */, E2DAFAB927BCF3C200BA12BD /* RoutesCoordinator.swift */, ); @@ -3553,7 +3553,6 @@ 4303A3992332CD6200B5737D /* UnimplementedLogging.swift in Sources */, 2C4D0A2E29F6DC900063BF52 /* NavigationSessionManager.swift in Sources */, 2C295ACA299BF76000FC74E8 /* Junction.swift in Sources */, - 2E6656F9264EC912009463EE /* Result+Expected.swift in Sources */, 8A7B69922947AA0F00FFC3F5 /* AmenityType.swift in Sources */, 11D1F8A02696048D0053A93F /* Dictionary+DeepMerge.swift in Sources */, 3A163AE3249901D000D66A0D /* FixLocation.swift in Sources */, @@ -3606,6 +3605,7 @@ DA5F44C825F07AB700F573EC /* MapboxStreetsRoadClass.swift in Sources */, 2B5A4AC728099FA900170A2B /* AlternativeRoute.swift in Sources */, 2E50E0C0264E35CA009D3848 /* RoadObjectMatcher.swift in Sources */, + DFBE62C82BB467E900CF830D /* Result++.swift in Sources */, C51DF8661F38C31C006C6A15 /* Locale.swift in Sources */, DA5F44F625F07D3B00F573EC /* RoadObjectStore.swift in Sources */, 2C85644F297AD64C006EFCBB /* NavigationNativeEventsManager.swift in Sources */, From a49433231a35561543a2262ac6287d5d24b3622d Mon Sep 17 00:00:00 2001 From: Alex Azarov Date: Wed, 27 Mar 2024 15:43:42 +0100 Subject: [PATCH 5/5] Added changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10b91bc5a7f..bc1c5546ee2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ * Fixed a rare issue that could lead to memory corruption under specific conditions. This was resolved by replacing the internal image downloader with brand new actor-based implementation. ([#4588](https://github.com/mapbox/mapbox-navigation-ios/pull/4588)) * Fixed the possible situation when the upcoming route leg is rendered above the active route leg. ([#4588](https://github.com/mapbox/mapbox-navigation-ios/pull/4588)) * Fixed a main thread hang on NavigationViewController creation. ([#4617](https://github.com/mapbox/mapbox-navigation-ios/pull/4617)) +* Fixed error reporting in `RouteController/reroute(from:along:)`. ([#4618](https://github.com/mapbox/mapbox-navigation-ios/pull/4618)) ## v2.17.0