RestApi Refactoring with named Parameters and better Localization.

[tkausch]

PR Type

Enhancement

---

Description

• Refactor RestError enum cases to use named parameters for clarity

• Add LocalizedError conformance with localized descriptions and recovery suggestions

• Implement CustomDebugStringConvertible for improved debugging output

• Add Equatable conformance to enable error comparison in tests

---

Diagram Walkthrough

flowchart LR
  A["RestError enum"] -->|"Add named parameters"| B["Improved API clarity"]
  A -->|"Implement LocalizedError"| C["User-friendly messages"]
  A -->|"Add CustomDebugStringConvertible"| D["Better debugging"]
  A -->|"Implement Equatable"| E["Test support"]
  B --> F["Enhanced RestError"]
  C --> F
  D --> F
  E --> F

Loading

File Walkthrough

Relevant files

Enhancement

RestApiCaller.swift Update RestError instantiations with named parameters        Sources/SwiftRestRequests/RestApiCaller.swift Update all RestError case instantiations to use named parameters Change unexpectedHttpStatusCode(httpResponse.statusCode) to unexpectedHttpStatusCode(statusCode: httpResponse.statusCode) Change badResponse(response, data) to badResponse(response: response, data: data) Change failedRestCall(httpResponse, httpStatus, error: nil) to failedRestCall(response: httpResponse, status: httpStatus, errorPayload: nil) Change invalidMimeType(contentType) to invalidMimeType(mimeType: contentType) Change malformedResponse(response, data, error) to malformedResponse(response: response, data: data, underlying: error) +7/-7      RestError.swift Add localization and debugging support to RestError            Sources/SwiftRestRequests/RestError.swift Refactor all RestError cases to use named parameters instead of positional arguments Update documentation comments to describe parameters with improved clarity and detail Add LocalizedError extension with errorDescription and recoverySuggestion properties for user-friendly error messages Add CustomDebugStringConvertible extension providing detailed debug output for each error case Add Equatable extension enabling error comparison for testing purposes +102/-28

---

[qodo-code-review]

PR Compliance Guide 🔍

(Compliance updated until commit 8dac925)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Audit Logging: The new logging added records request method, path, status, and response size but does not clearly include a user identifier or guarantee structured logs for reconstructing critical actions. Referred Code // 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) } // 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 body that needs validation/parsing. Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error Detail Exposure: Logger calls emit errorDescription and underlying decoding errors which may surface internal details if routed to user-facing logs. Referred Code let mimeTypeError = RestError.invalidMimeType(mimeType: contentType) logger.error("Throw mimetype error: \(String(describing: mimeTypeError.errorDescription))") throw mimeTypeError } return mimeType } private func decodeSuccessfulResponse<T: Deserializer>(data: Data, response: HTTPURLResponse, deserializer: T) throws -> T.ResponseType? { if response.status == .ok { do { return try deserializer.deserialize(data) } catch { // 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 Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Potential PII Logs: Logs include path and status and may include payload-derived details via errorDescription which could inadvertently contain sensitive data depending on upstream configuration. Referred Code // 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) } // 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 body that needs validation/parsing. _ = try validatedMimeType(from: httpResponse) ... (clipped 54 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

---

Previous compliance checks

Compliance check up to commit d961b13

Security Compliance
⚪	Sensitive information exposure Description: Debug descriptions interpolate untrusted response data (URLs, payload sizes) into strings that could be logged; if logs include sensitive URLs or query parameters, this may expose sensitive information in logs. RestError.swift [101-118] Referred Code 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))" } } }
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing Logging: New error paths throw RestError but do not log critical request/response context, which may limit auditability of failed REST calls. Referred Code // make request and install interceptor hooks callInvokeInterceptors(&request) let (data, response) = try await session.data(for: request) // check http response has a supported type guard let httpResponse = response as? HTTPURLResponse else { throw RestError.badResponse(response: response, data: data) } callReceiveInterceptors(data, httpResponse) try validateResponseStatusCodes(options.expectedStatusCodes, httpResponse) return (data, httpResponse) } /// Make a REST call and deserialize the response with the given deserializer from JSON to object. For successful calls the httpStatus is returned as well. Note: /// Some REST services do not always return /// - Parameters: ... (clipped 67 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Error Detail Exposure: The LocalizedError descriptions include underlying error messages and status codes which may be user-facing depending on usage, potentially exposing internal details. Referred Code 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: ... (clipped 27 lines) Learn more about managing compliance generic rules or creating your own custom rules

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Re-evaluate the incomplete Equatable implementation The Equatable implementation for RestError is incomplete, as it only compares a subset of associated values for certain cases. This should be fixed by either implementing a more thorough check or by removing the conformance and using test helpers instead. Examples: Sources/SwiftRestRequests/RestError.swift [122-141] 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)): ... (clipped 10 lines) Solution Walkthrough: Before: extension RestError: Equatable { public static func == (lhs: RestError, rhs: RestError) -> Bool { switch (lhs, rhs) { // ... case (.malformedResponse(let la, _, _), .malformedResponse(let ra, _, _)): // Only status code is compared. Data and underlying error are ignored. return la.statusCode == ra.statusCode case (.failedRestCall(_, let aStatus, _), .failedRestCall(_, let bStatus, _)): // Only status is compared. Response and payload are ignored. return aStatus == bStatus default: return false } } } After: // Option 1: More complete Equatable (conceptual) extension RestError: Equatable { public static func == (lhs: RestError, rhs: RestError) -> Bool { switch (lhs, rhs) { // ... case (.malformedResponse(let lResp, let lData, _), .malformedResponse(let rResp, let rData, _)): // Compare more fields, e.g., data. // Note: `any Error` cannot be compared directly. return lResp.statusCode == rResp.statusCode && lData == rData case (.failedRestCall(let lResp, let lStatus, _), .failedRestCall(let rResp, let rStatus, _)): // Compare more fields. // Note: `any Sendable` cannot be compared directly. return lStatus == rStatus && lResp.url == rResp.url // ... } } } // Option 2: Remove Equatable and use test helpers // func isMalformedResponse(error: Error, withStatusCode: Int) -> Bool { ... } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the new Equatable conformance for RestError is incomplete for malformedResponse and failedRestCall, which could lead to incorrect test outcomes.	Medium
General	Improve Equatable conformance for malformed responses Improve the Equatable implementation for the .malformedResponse case by also comparing the response data, in addition to the status code. Sources/SwiftRestRequests/RestError.swift [133-134] -case (.malformedResponse(let la, _, _), .malformedResponse(let ra, _, _)): - return la.statusCode == ra.statusCode +case (.malformedResponse(let la, let lData, _), .malformedResponse(let ra, let rData, _)): + return la.statusCode == ra.statusCode && lData == rData Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the Equatable implementation for .malformedResponse is incomplete and proposes a valid improvement by comparing the response data, which enhances the accuracy of equality checks for testing.	Medium
Update