From d961b13fa524900588cbff0f8458be32f12c007b Mon Sep 17 00:00:00 2001 From: tkausch Date: Tue, 11 Nov 2025 11:48:43 +0100 Subject: [PATCH 1/2] RestApi Refactoring with named Parameters and better Localization. --- Sources/SwiftRestRequests/RestApiCaller.swift | 14 +- Sources/SwiftRestRequests/RestError.swift | 130 ++++++++++++++---- 2 files changed, 109 insertions(+), 35 deletions(-) diff --git a/Sources/SwiftRestRequests/RestApiCaller.swift b/Sources/SwiftRestRequests/RestApiCaller.swift index d8d899d..4c0ca87 100644 --- a/Sources/SwiftRestRequests/RestApiCaller.swift +++ b/Sources/SwiftRestRequests/RestApiCaller.swift @@ -203,7 +203,7 @@ open class RestApiCaller : NSObject { @inline(__always) private func validateResponseStatusCodes(_ expectedStatusCodes: [HTTPStatusCode]?, _ httpResponse: HTTPURLResponse) throws { if let expectedStatusCodes, !expectedStatusCodes.contains(httpResponse.status) { - throw RestError.unexpectedHttpStatusCode(httpResponse.statusCode) + throw RestError.unexpectedHttpStatusCode(statusCode: httpResponse.statusCode) } } @@ -251,7 +251,7 @@ open class RestApiCaller : NSObject { // check http response has a supported type guard let httpResponse = response as? HTTPURLResponse else { - throw RestError.badResponse(response, data) + throw RestError.badResponse(response: response, data: data) } callReceiveInterceptors(data, httpResponse) @@ -283,7 +283,7 @@ open class RestApiCaller : NSObject { } guard !data.isEmpty else { - throw RestError.failedRestCall(httpResponse, httpStatus, error: nil) + throw RestError.failedRestCall(response: httpResponse, status: httpStatus, errorPayload: nil) } // Postcondition: We have a response object or error that needs to be parsed! @@ -308,7 +308,7 @@ open class RestApiCaller : NSObject { guard let firstContentMimeType, let mimeType = MimeType(rawValue: firstContentMimeType) else { - throw RestError.invalidMimeType(contentType) + throw RestError.invalidMimeType(mimeType: contentType) } return mimeType } @@ -318,7 +318,7 @@ open class RestApiCaller : NSObject { do { return try deserializer.deserialize(data) } catch { - throw RestError.malformedResponse(response, data, error) + throw RestError.malformedResponse(response: response, data: data, underlying: error) } } return nil @@ -327,9 +327,9 @@ open class RestApiCaller : NSObject { private func buildErrorResponse(data: Data, response: HTTPURLResponse, status: HTTPStatusCode) throws -> RestError { do { let errorPayload = try errorDeserializer?.deserialize(data) - return RestError.failedRestCall(response, status, error: errorPayload) + return RestError.failedRestCall(response: response, status: status, errorPayload: errorPayload) } catch { - throw RestError.malformedResponse(response, data, error) + throw RestError.malformedResponse(response: response, data: data, underlying: error) } } diff --git a/Sources/SwiftRestRequests/RestError.swift b/Sources/SwiftRestRequests/RestError.swift index 870845f..9ca78e4 100644 --- a/Sources/SwiftRestRequests/RestError.swift +++ b/Sources/SwiftRestRequests/RestError.swift @@ -31,37 +31,111 @@ import FoundationNetworking /// or attempt to recover (for example when a deserializer reports malformed data). public enum RestError: Error { - /// Indicates that the server responded using an unknown protocol. + /// The server responded with a non-HTTP response or an unsupported protocol. /// - Parameters: - /// - URLResponse: The response returned form the server. - /// - Data: The raw returned data from the server. - case badResponse(URLResponse, Data) - - /// Indicates that the server responded with an unexpected MIME type. - /// - Parameter String: The returned MIME type. - case invalidMimeType(String?) - - /// Indicates that query parameters with key could not be encoded using percent encoding. + /// - response: The raw `URLResponse` returned by the loading system. + /// - data: The raw response body bytes (may be empty). + case badResponse(response: URLResponse, data: Data) + + /// The response `Content-Type` did not match expected MIME types. + /// - Parameter mimeType: The value of the `Content-Type` header, if any. + case invalidMimeType(mimeType: String?) + + /// One or more query parameters could not be encoded (percent-encoding failure). case invalidQueryParameter - /// Indicates the server's response could not be deserialized using the given Deserializer. + /// The response body could not be deserialized into the expected model. /// - Parameters: - /// - HTTPURLResponse: The HTTPURLResponse from the server. - /// - Data: The raw returned data from the server. - /// - Error: The original system error (like a `DecodingError`) that triggered the failure. - case malformedResponse(HTTPURLResponse, Data, any Error) - - /// Indicates the API call failed and optionally surfaces the parsed error payload. + /// - response: The `HTTPURLResponse` that accompanied the body. + /// - data: The raw response body. + /// - underlying: The underlying parsing/decoding error (commonly `DecodingError`). + case malformedResponse(response: HTTPURLResponse, data: Data, underlying: any Error) + + /// The API returned an error HTTP status and, optionally, a parsed error payload. /// - Parameters: - /// - HTTPURLResponse: The HTTPURLResponse from the server. - /// - HTTPStatusCode: The returned HTTP status. - /// - error: The deserialized error payload, if available. - case failedRestCall(HTTPURLResponse, HTTPStatusCode, error: (any Sendable)?) - - /// Indicates that the response contained an unexpected HTTP status code. - /// - /// Configure `RestOptions.expectedStatusCodes` to mark additional status codes as valid. - /// - Parameter Int: The HTTP status returned from the server. - case unexpectedHttpStatusCode(Int) - + /// - response: The `HTTPURLResponse` returned by the server. + /// - status: The `HTTPStatusCode` for the response. + /// - errorPayload: An optional deserialized error object returned by the API. + case failedRestCall(response: HTTPURLResponse, status: HTTPStatusCode, errorPayload: (any Sendable)?) + + /// The HTTP status code was not in the set of expected codes. + /// Configure `RestOptions.expectedStatusCodes` to accept additional codes. + /// - Parameter statusCode: The unexpected HTTP status code. + case unexpectedHttpStatusCode(statusCode: Int) + + +} + +// MARK: - LocalizedError and debug helpers + +extension RestError: LocalizedError { + public var errorDescription: String? { + switch self { + case .badResponse: + return "Received an unsupported or invalid response from the server." + case .invalidMimeType(let mime): + return "Unexpected content type: \(mime ?? "unknown")." + case .invalidQueryParameter: + return "Failed to encode query parameters." + case .malformedResponse(_, _, let underlying): + return "Failed to decode response: \(underlying.localizedDescription)" + case .failedRestCall(_, let status, _): + return "Server returned an error (status: \(status))." + case .unexpectedHttpStatusCode(let code): + return "Unexpected HTTP status code: \(code)." + } + } + + public var recoverySuggestion: String? { + switch self { + case .invalidQueryParameter: + return "Verify parameter values and percent-encode reserved characters." + case .malformedResponse: + return "Confirm the response schema matches the expected model and enable payload logging in debug builds." + default: + return nil + } + } +} + +extension RestError: CustomDebugStringConvertible { + public var debugDescription: String { + switch self { + case .badResponse(let response, let data): + return "RestError.badResponse(url: \(response.url?.absoluteString ?? "n/a"), size: \(data.count))" + case .invalidMimeType(let mime): + return "RestError.invalidMimeType(mime: \(mime ?? "nil"))" + case .invalidQueryParameter: + return "RestError.invalidQueryParameter" + case .malformedResponse(let response, let data, let underlying): + return "RestError.malformedResponse(status: \(response.statusCode), size: \(data.count), underlying: \(underlying))" + case .failedRestCall(let response, let status, let payload): + return "RestError.failedRestCall(status: \(status), url: \(response.url?.absoluteString ?? "n/a"), payload: \(String(describing: payload)))" + case .unexpectedHttpStatusCode(let code): + return "RestError.unexpectedHttpStatusCode(\(code))" + } + } +} + +// MARK: - Equatable (useful for tests) + +extension RestError: Equatable { + public static func == (lhs: RestError, rhs: RestError) -> Bool { + switch (lhs, rhs) { + case (.invalidQueryParameter, .invalidQueryParameter): + return true + case (.invalidMimeType(let a), .invalidMimeType(let b)): + return a == b + case (.unexpectedHttpStatusCode(let a), .unexpectedHttpStatusCode(let b)): + return a == b + case (.badResponse(let la, let ld), .badResponse(let ra, let rd)): + return la.url?.absoluteString == ra.url?.absoluteString && ld == rd + case (.malformedResponse(let la, _, _), .malformedResponse(let ra, _, _)): + return la.statusCode == ra.statusCode + case (.failedRestCall(_, let aStatus, _), .failedRestCall(_, let bStatus, _)): + return aStatus == bStatus + default: + return false + } + } } From 9af77f47938c5057f8edd2c440fcecec50056539 Mon Sep 17 00:00:00 2001 From: tkausch Date: Tue, 11 Nov 2025 14:33:13 +0100 Subject: [PATCH 2/2] Explicit logging to make REST calls reproducible. --- Sources/SwiftRestRequests/RestApiCaller.swift | 42 +++++++++++++------ Sources/SwiftRestRequests/http/HTTPUtil.swift | 2 +- .../AbstractRestApiCallerTests.swift | 7 +++- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/Sources/SwiftRestRequests/RestApiCaller.swift b/Sources/SwiftRestRequests/RestApiCaller.swift index 4c0ca87..2dde6f7 100644 --- a/Sources/SwiftRestRequests/RestApiCaller.swift +++ b/Sources/SwiftRestRequests/RestApiCaller.swift @@ -271,25 +271,32 @@ open class RestApiCaller : NSObject { /// - options: Rest options to use for the data task i.e. timeout /// - Returns: The data returned by server and the corresponding `HTTPURLResponse` private func makeCall(_ relativePath: String?, httpMethod: HTTPMethod, payload: Data?, responseDeserializer: T, options: RestOptions) async throws -> (T.ResponseType?, HTTPStatusCode) { - let (data, httpResponse) = try await dataTask(relativePath: relativePath, httpMethod: httpMethod.rawValue, accept: responseDeserializer.acceptHeader, payload: payload, options: options) - let httpStatus = httpResponse.status - + + // Log receipt for auditing + logger.debug("Received response: method=\(httpMethod.rawValue) path=\(relativePath ?? "/") status=\(httpStatus) size=\(data.count)") + // For requests without deserialization and no error just return the status if shouldBypassDeserialization(responseDeserializer, status: httpStatus), httpStatus.type == .success { + logger.debug("Bypassing deserialization for status=\(httpStatus) at path=\(relativePath ?? "/")") return (nil, httpStatus) } - - guard !data.isEmpty else { + + // If there's no body but the status indicates an error, surface a failedRestCall so callers + // can handle API-level errors. Log the condition for audit. + if data.isEmpty { + logger.warning("Empty response body received for status=\(httpStatus) at path=\(relativePath ?? "/"). Treating as failedRestCall.") throw RestError.failedRestCall(response: httpResponse, status: httpStatus, errorPayload: nil) } - - // Postcondition: We have a response object or error that needs to be parsed! - + + // Postcondition: We have a response body that needs validation/parsing. + + _ = try validatedMimeType(from: httpResponse) + if httpStatus.type == .success { let transformedResponse = try decodeSuccessfulResponse(data: data, response: httpResponse, deserializer: responseDeserializer) return (transformedResponse, httpStatus) @@ -303,12 +310,16 @@ open class RestApiCaller : NSObject { } private func validatedMimeType(from response: HTTPURLResponse) throws -> MimeType { + let contentType = response.value(forHTTPHeaderField: HTTPHeaderKeys.ContentType.rawValue) let firstContentMimeType = contentType?.components(separatedBy: ";").first guard let firstContentMimeType, let mimeType = MimeType(rawValue: firstContentMimeType) else { - throw RestError.invalidMimeType(mimeType: contentType) + + let mimeTypeError = RestError.invalidMimeType(mimeType: contentType) + logger.error("Throw mimetype error: \(String(describing: mimeTypeError.errorDescription))") + throw mimeTypeError } return mimeType } @@ -318,7 +329,10 @@ open class RestApiCaller : NSObject { do { return try deserializer.deserialize(data) } catch { - throw RestError.malformedResponse(response: response, data: data, underlying: error) + // Log decoding failure with diagnostics and rethrow a RestError + let malformedResponseError = RestError.malformedResponse(response: response, data: data, underlying: error) + logger.error("Failed to deserialize successful response throwing error: \(String(describing: malformedResponseError.errorDescription)).") + throw malformedResponseError } } return nil @@ -327,9 +341,13 @@ open class RestApiCaller : NSObject { private func buildErrorResponse(data: Data, response: HTTPURLResponse, status: HTTPStatusCode) throws -> RestError { do { let errorPayload = try errorDeserializer?.deserialize(data) - return RestError.failedRestCall(response: response, status: status, errorPayload: errorPayload) + let failedRestCall = RestError.failedRestCall(response: response, status: status, errorPayload: errorPayload) + logger.error("Failing rest call throw error: \(String(describing: failedRestCall.errorDescription))") + return failedRestCall } catch { - throw RestError.malformedResponse(response: response, data: data, underlying: error) + let malformedResponseError = RestError.malformedResponse(response: response, data: data, underlying: error) + logger.error("Error deserialing response throwing error: \(String(describing: malformedResponseError.errorDescription))") + throw malformedResponseError } } diff --git a/Sources/SwiftRestRequests/http/HTTPUtil.swift b/Sources/SwiftRestRequests/http/HTTPUtil.swift index 928ab65..0d2374a 100644 --- a/Sources/SwiftRestRequests/http/HTTPUtil.swift +++ b/Sources/SwiftRestRequests/http/HTTPUtil.swift @@ -33,7 +33,7 @@ internal enum HTTPMethod: String { } /// MIME types referenced throughout the client. -internal enum MimeType: String { +internal enum MimeType: String, CaseIterable { case ApplicationJson = "application/json" case TextPlain = "text/plain" case ApplicationOctetStream = "application/octet-stream" diff --git a/Tests/SwiftRestRequestsTests/AbstractRestApiCallerTests.swift b/Tests/SwiftRestRequestsTests/AbstractRestApiCallerTests.swift index c33110b..f70d92e 100644 --- a/Tests/SwiftRestRequestsTests/AbstractRestApiCallerTests.swift +++ b/Tests/SwiftRestRequestsTests/AbstractRestApiCallerTests.swift @@ -18,6 +18,11 @@ class AbstractRestApiCallerTests: XCTestCase { override class func setUp() { super.setUp() + // *********************************************************** + // IMPORTANT NOTE: You must run httpbin locally for testing!!! + // docker run -p 80:80 kennethreitz/httpbin + // ************************************************************ + // Synchronize access to global logger state loggingLock.lock() defer { loggingLock.unlock() } @@ -26,7 +31,7 @@ class AbstractRestApiCallerTests: XCTestCase { // Configure `swift-log` default logger #else /// Configure `swift-log` logging system to use OSLog backend - LoggingSystem.bootstrap(OSLogHandler.init) + // LoggingSystem.bootstrap(OSLogHandler.init) #endif Logger.SwiftRestRequests.security.logLevel = .trace