diff --git a/OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift b/OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift index 7f7e9c4..42c8b26 100644 --- a/OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift +++ b/OTPKit/Sources/OTPKit/Core/Models/OTP/ErrorResponse.swift @@ -15,6 +15,7 @@ */ import Foundation +import OSLog /// `ErrorResponse` represents an error structure used across the application to handle and represent /// OTP errors uniformly. @@ -25,4 +26,103 @@ public struct ErrorResponse: Codable, Hashable { /// A descriptive message associated with the error, providing more detailed information about what went wrong. /// This message can be presented to the user or used in debugging to provide context about the error. public let message: String + + /// A message key identifying the error type. + /// This value maps directly to a key in `Messages.properties` and can be used + /// for control flow, localization, or client-side error mapping. + public let messageCode: ErrorResponseCode + + public enum CodingKeys: String, CodingKey { + case id + case message = "msg" + case messageCode = "message" + } +} + +public enum ErrorResponseCode: String, Codable, Hashable { + case systemError = "SYSTEM_ERROR" + case graphUnavailable = "GRAPH_UNAVAILABLE" + case outsideBounds = "OUTSIDE_BOUNDS" + case pathNotFound = "PATH_NOT_FOUND" + case noTransitTimes = "NO_TRANSIT_TIMES" + case requestTimeout = "REQUEST_TIMEOUT" + case bogusParameter = "BOGUS_PARAMETER" + case geocodeFromNotFound = "GEOCODE_FROM_NOT_FOUND" + case geocodeToNotFound = "GEOCODE_TO_NOT_FOUND" + case geocodeFromToNotFound = "GEOCODE_FROM_TO_NOT_FOUND" + case tooClose = "TOO_CLOSE" + case locationNotAccessible = "LOCATION_NOT_ACCESSIBLE" + case geocodeFromAmbiguous = "GEOCODE_FROM_AMBIGUOUS" + case geocodeToAmbiguous = "GEOCODE_TO_AMBIGUOUS" + case geocodeFromToAmbiguous = "GEOCODE_FROM_TO_AMBIGUOUS" + case underspecifiedTriangle = "UNDERSPECIFIED_TRIANGLE" + case triangleNotAffine = "TRIANGLE_NOT_AFFINE" + case triangleOptimizeTypeNotSet = "TRIANGLE_OPTIMIZE_TYPE_NOT_SET" + case triangleValuesNotSet = "TRIANGLE_VALUES_NOT_SET" + + /// Fallback for unknown or future OTP message keys + case unknown + + public init(from decoder: Decoder) throws { + let container = try decoder.singleValueContainer() + let value = try container.decode(String.self) + + if let known = ErrorResponseCode(rawValue: value) { + self = known + } else { + Logger.main.warning("Unrecognized OTP error code: \(value)") + self = .unknown + } + } + + public func encode(to encoder: Encoder) throws { + var container = encoder.singleValueContainer() + try container.encode(rawValue) + } + + /// Localization key used to resolve a user-facing message + public var displayMessage: String { + switch self { + case .systemError: + return OTPLoc("error.system_error") + case .graphUnavailable: + return OTPLoc("error.graph_unavailable") + case .outsideBounds: + return OTPLoc("error.outside_bounds") + case .pathNotFound: + return OTPLoc("error.path_not_found") + case .noTransitTimes: + return OTPLoc("error.no_transit_times") + case .requestTimeout: + return OTPLoc("error.request_timeout") + case .bogusParameter: + return OTPLoc("error.bogus_parameter") + case .geocodeFromNotFound: + return OTPLoc("error.geocode_from_not_found") + case .geocodeToNotFound: + return OTPLoc("error.geocode_to_not_found") + case .geocodeFromToNotFound: + return OTPLoc("error.geocode_from_to_not_found") + case .tooClose: + return OTPLoc("error.too_close") + case .locationNotAccessible: + return OTPLoc("error.location_not_accessible") + case .geocodeFromAmbiguous: + return OTPLoc("error.geocode_from_ambiguous") + case .geocodeToAmbiguous: + return OTPLoc("error.geocode_to_ambiguous") + case .geocodeFromToAmbiguous: + return OTPLoc("error.geocode_from_to_ambiguous") + case .underspecifiedTriangle: + return OTPLoc("error.underspecified_triangle") + case .triangleNotAffine: + return OTPLoc("error.triangle_not_affine") + case .triangleOptimizeTypeNotSet: + return OTPLoc("error.triangle_optimize_type_not_set") + case .triangleValuesNotSet: + return OTPLoc("error.triangle_values_not_set") + case .unknown: + return OTPLoc("error.unknown") + } + } } diff --git a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift index 6d78a28..609093f 100644 --- a/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift +++ b/OTPKit/Sources/OTPKit/Presentation/ViewModel/TripPlannerViewModel.swift @@ -11,6 +11,7 @@ import MapKit import SwiftUI import OSLog +// swiftlint:disable file_length /// Main view model for handling trip planning functionality /// Manages location selection, transport modes, API calls, and UI state @MainActor @@ -177,7 +178,7 @@ public class TripPlannerViewModel: ObservableObject { func planTrip() { // Validate that we have required locations guard let origin = selectedOrigin, let destination = selectedDestination else { - showError(OTPKitError.missingOriginOrDestination) + showError(OTPKitError.missingOriginOrDestination.displayMessage) return } @@ -205,7 +206,7 @@ public class TripPlannerViewModel: ObservableObject { do { let response = try await apiService.fetchPlan(request) await MainActor.run { - self.handleSuccess(response) + self.handlePlanResponse(response) } } catch { await MainActor.run { @@ -246,22 +247,31 @@ public class TripPlannerViewModel: ObservableObject { } } - private func handleSuccess(_ response: OTPResponse) { - tripPlanResponse = response + private func handlePlanResponse(_ response: OTPResponse) { isLoading = false - HapticManager.shared.success() - notificationCenter.post(name: Notifications.itinerariesUpdated, object: nil) + + if let error = response.error { + Logger.main.error("Planner Error: \(error.message)") + Logger.main.error("Planner Error Code: \(error.messageCode.rawValue)") + + tripPlanResponse = nil + showError(error.messageCode.displayMessage) + } else { + tripPlanResponse = response + HapticManager.shared.success() + notificationCenter.post(name: Notifications.itinerariesUpdated, object: nil) + } } private func handleError(_ error: Error) { let otpError = error as? OTPKitError ?? OTPKitError.tripPlanningFailed(error.localizedDescription) - showError(otpError) + isLoading = false + showError(otpError.displayMessage) } - private func showError(_ error: OTPKitError) { - errorMessage = error.displayMessage + private func showError(_ message: String) { + errorMessage = message showingError = true - isLoading = false } // MARK: - Preview Management @@ -395,3 +405,4 @@ public class TripPlannerViewModel: ObservableObject { activeSheet = nil } } +// swiftlint:enable file_length diff --git a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings index d27e444..c00001c 100644 --- a/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings +++ b/OTPKit/Sources/OTPKit/Resources/en.lproj/Localizable.strings @@ -16,6 +16,33 @@ "error.missing_locations.message" = "Please select both origin and destination locations."; "error.unknown" = "An unknown error occurred."; +/* Planning */ +"error.system_error" = "We couldn’t plan your trip right now. Please try again."; +"error.graph_unavailable" = "Trip planning is temporarily unavailable. Try again shortly."; +"error.outside_bounds" = "This location isn’t supported yet."; +"error.path_not_found" = "No route is available for this trip."; +"error.no_transit_times" = "No transit options are available right now."; +"error.request_timeout" = "This took longer than expected. Please try again."; +"error.bogus_parameter" = "Something went wrong with the request. Please try again."; + +/* Geocoding */ +"error.geocode_from_not_found" = "We couldn’t find the starting location."; +"error.geocode_to_not_found" = "We couldn’t find the destination."; +"error.geocode_from_to_not_found" = "We couldn’t find either the starting point or the destination."; +"error.geocode_from_ambiguous" = "The starting location isn’t clear. Try being more specific."; +"error.geocode_to_ambiguous" = "The destination isn’t clear. Try being more specific."; +"error.geocode_from_to_ambiguous" = "Both locations need more detail. Try refining them."; + +/* Validation */ +"error.too_close" = "The start and destination are too close."; +"error.location_not_accessible" = "This route isn’t accessible with the selected option."; + +/* Triangle routing (advanced) */ +"error.underspecified_triangle" = "Some route options are missing."; +"error.triangle_not_affine" = "The selected route options don’t work together."; +"error.triangle_optimize_type_not_set" = "Please choose a route preference."; +"error.triangle_values_not_set" = "Please adjust the route preferences."; + /* Map */ "map.origin" = "Origin"; "map.destination" = "Destination"; diff --git a/OTPKit/Tests/Fixtures/otp_response_error.json b/OTPKit/Tests/Fixtures/otp_response_error.json index 22efe1e..ec4ec4d 100644 --- a/OTPKit/Tests/Fixtures/otp_response_error.json +++ b/OTPKit/Tests/Fixtures/otp_response_error.json @@ -13,7 +13,7 @@ "error": { "id": 404, "msg": "Trip not found", - "message": "No trip found that satisfies the requested parameters.", + "message": "PATH_NOT_FOUND", "missing": [], "noPath": true } diff --git a/OTPKit/Tests/Fixtures/otp_response_unknown_error.json b/OTPKit/Tests/Fixtures/otp_response_unknown_error.json new file mode 100644 index 0000000..c61f26f --- /dev/null +++ b/OTPKit/Tests/Fixtures/otp_response_unknown_error.json @@ -0,0 +1,20 @@ +{ + "requestParameters": { + "fromPlace": "47.6097,-122.3331", + "toPlace": "47.6154,-122.3208", + "time": "08:00:00", + "date": "2024-05-10", + "mode": "TRANSIT,WALK", + "arriveBy": "false", + "maxWalkDistance": "800", + "wheelchair": "false" + }, + "plan": null, + "error": { + "id": 404, + "msg": "Trip not found", + "message": "Unknown Error Type", + "missing": [], + "noPath": true + } +} diff --git a/OTPKit/Tests/Helpers/TestHelpers.swift b/OTPKit/Tests/Helpers/TestHelpers.swift index 56c5cc0..f354a0e 100644 --- a/OTPKit/Tests/Helpers/TestHelpers.swift +++ b/OTPKit/Tests/Helpers/TestHelpers.swift @@ -68,7 +68,7 @@ enum TestHelpers { } /// Wrap itineraries into an `OTPResponse` the view model expects - static func response(with itineraries: [Itinerary]) -> OTPResponse { + static func response(with itineraries: [Itinerary] = [], error: ErrorResponse? = nil) -> OTPResponse { let plan = Plan( date: Date(), from: Place(name: "A", lon: -122.0, lat: 47.0, vertexType: "NORMAL"), @@ -85,6 +85,6 @@ enum TestHelpers { maxWalkDistance: "1000", wheelchair: "false" ) - return OTPResponse(requestParameters: params, plan: plan, error: nil) + return OTPResponse(requestParameters: params, plan: plan, error: error) } } diff --git a/OTPKit/Tests/RestAPIServiceComprehensiveTests.swift b/OTPKit/Tests/RestAPIServiceComprehensiveTests.swift index 60dac88..456ed2a 100644 --- a/OTPKit/Tests/RestAPIServiceComprehensiveTests.swift +++ b/OTPKit/Tests/RestAPIServiceComprehensiveTests.swift @@ -313,7 +313,34 @@ struct RestAPIServiceComprehensiveTests { #expect(response.plan == nil) #expect(response.error != nil) - #expect(response.error?.message == "No trip found that satisfies the requested parameters.") + #expect(response.error?.message == "Trip not found") + #expect(response.error?.messageCode == .pathNotFound) + } + + @Test("fetchPlan with OTP unknown error response") + func fetchPlanWithOTPUnknownErrorResponse() async throws { + let mockLoader = MockDataLoader() + let fixtureData = Fixtures.loadData(file: "otp_response_unknown_error.json") + mockLoader.mockResponse(data: fixtureData) + + let service = RestAPIService( + baseURL: URL(string: "https://otp.example.com")!, + dataLoader: mockLoader + ) + + let request = TripPlanRequest( + origin: CLLocationCoordinate2D(latitude: 47.6097, longitude: -122.3331), + destination: CLLocationCoordinate2D(latitude: 47.6154, longitude: -122.3208), + date: Date(), + time: Date() + ) + + let response = try await service.fetchPlan(request) + + #expect(response.plan == nil) + #expect(response.error != nil) + #expect(response.error?.message == "Trip not found") + #expect(response.error?.messageCode == .unknown) } // MARK: - HTTP Error Tests diff --git a/OTPKit/Tests/TripPlannerViewModelTests.swift b/OTPKit/Tests/TripPlannerViewModelTests.swift index 0cd8296..0b9d33d 100644 --- a/OTPKit/Tests/TripPlannerViewModelTests.swift +++ b/OTPKit/Tests/TripPlannerViewModelTests.swift @@ -33,7 +33,7 @@ struct TripPlannerViewModelTests { ) async throws { let start = Date() while viewModel.isLoading && Date().timeIntervalSince(start) < timeout { - try await Task.sleep(nanoseconds: 50_000_000) // Poll every 0.05 seconds + try await Task.sleep(nanoseconds: 100_000_000) // Poll every 0.1 seconds } } @@ -453,6 +453,126 @@ struct TripPlannerViewModelTests { #expect(viewModel.activeSheet != nil) } + // MARK: - Plan Error Response Handling Test + @Test("planTrip handles OTP error in successful HTTP response") + func planTripHandlesOTPErrorInSuccessfulResponse() async throws { + let mockAPIService = TestFixtures.MockAPIService() + + let errorResponse = TestHelpers.response(error: .init(id: 1, message: "Path not found", messageCode: .pathNotFound)) + mockAPIService.mockResponse = errorResponse + + let viewModel = createViewModel(mockAPIService: mockAPIService) + viewModel.selectedOrigin = TestHelpers.location(title: "Origin") + viewModel.selectedDestination = TestHelpers.location(title: "Destination") + + viewModel.planTrip() + + try await waitForLoadingComplete(viewModel) + + #expect(viewModel.showingError == true) + #expect(viewModel.errorMessage != nil) + #expect(viewModel.isLoading == false) + #expect(viewModel.tripPlanResponse == nil) + } + + @Test("planTrip error response clears previous successful trip plan") + func planTripErrorResponseClearsPreviousSuccessfulPlan() async throws { + let mockAPIService = TestFixtures.MockAPIService() + + let viewModel = createViewModel(mockAPIService: mockAPIService) + viewModel.selectedOrigin = TestHelpers.location(title: "Origin") + viewModel.selectedDestination = TestHelpers.location(title: "Destination") + + // First call: successful response with itineraries + let successResponse = TestHelpers.response(with: [TestHelpers.itinerary()]) + mockAPIService.mockResponse = successResponse + + viewModel.planTrip() + try await waitForLoadingComplete(viewModel) + + #expect(viewModel.tripPlanResponse != nil) + #expect(viewModel.showingError == false) + + // Second call: error response + let errorResponse = TestHelpers.response( + error: .init( + id: 1, + message: "Path not found", + messageCode: .pathNotFound + ) + ) + + mockAPIService.mockResponse = errorResponse + + viewModel.planTrip() + try await waitForLoadingComplete(viewModel) + + // Verify previous response is cleared and error is shown + #expect(viewModel.tripPlanResponse == nil) + #expect(viewModel.showingError == true) + #expect(viewModel.errorMessage != nil) + #expect(viewModel.isLoading == false) + } + + @Test("planTrip handles PATH_NOT_FOUND error code with correct message") + func planTripHandlesPathNotFoundError() async throws { + let mockAPIService = TestFixtures.MockAPIService() + + let errorResponse = TestHelpers.response(error: .init(id: 1, message: "Path not found", messageCode: .pathNotFound)) + mockAPIService.mockResponse = errorResponse + + let viewModel = createViewModel(mockAPIService: mockAPIService) + viewModel.selectedOrigin = TestHelpers.location(title: "Origin") + viewModel.selectedDestination = TestHelpers.location(title: "Destination") + + viewModel.planTrip() + try await waitForLoadingComplete(viewModel) + + #expect(viewModel.showingError == true) + #expect(viewModel.errorMessage == errorResponse.error?.messageCode.displayMessage) + #expect(viewModel.isLoading == false) + #expect(viewModel.tripPlanResponse == nil) + } + + @Test("planTrip differentiates between OTP errors and network errors") + func planTripDistinguishesBetweenOTPAndNetworkErrors() async throws { + let mockAPIService = TestFixtures.MockAPIService() + let viewModel = createViewModel(mockAPIService: mockAPIService) + viewModel.selectedOrigin = TestHelpers.location(title: "Origin") + viewModel.selectedDestination = TestHelpers.location(title: "Destination") + + // Test 1: Network error + mockAPIService.shouldThrowError = true + mockAPIService.mockError = OTPKitError.apiError("Network timeout") + + viewModel.planTrip() + try await waitForLoadingComplete(viewModel) + + let networkErrorMessage = viewModel.errorMessage + #expect(viewModel.showingError == true) + #expect(networkErrorMessage != nil) + #expect(viewModel.isLoading == false) + + // Reset error state + viewModel.errorMessage = nil + viewModel.showingError = false + + // Test 2: OTP error + mockAPIService.shouldThrowError = false + let errorResponse = TestHelpers.response(error: .init(id: 1, message: "Path not found", messageCode: .pathNotFound)) + mockAPIService.mockResponse = errorResponse + + viewModel.planTrip() + try await waitForLoadingComplete(viewModel) + + let otpErrorMessage = viewModel.errorMessage + #expect(viewModel.showingError == true) + #expect(otpErrorMessage != nil) + #expect(viewModel.isLoading == false) + + #expect(networkErrorMessage != otpErrorMessage) + } + // MARK: - Reset Tests @Test("resetTripPlanner clears all state")