From 2d4fdc3378ce8711f386cc7a96b56263a06fbce4 Mon Sep 17 00:00:00 2001 From: Piotr Durlej Date: Sat, 9 May 2026 22:47:40 +0200 Subject: [PATCH] fix: classify auth and service errors --- .../ForgejoKit/Services/ForgejoClient.swift | 94 ++++++++++++++++--- Tests/ForgejoKitTests/ErrorMappingTests.swift | 48 ++++++++++ 2 files changed, 131 insertions(+), 11 deletions(-) create mode 100644 Tests/ForgejoKitTests/ErrorMappingTests.swift diff --git a/Sources/ForgejoKit/Services/ForgejoClient.swift b/Sources/ForgejoKit/Services/ForgejoClient.swift index 70a29c4..19d6135 100644 --- a/Sources/ForgejoKit/Services/ForgejoClient.swift +++ b/Sources/ForgejoKit/Services/ForgejoClient.swift @@ -115,13 +115,9 @@ public final class ForgejoClient: Sendable { let decoder = JSONDecoder() decoder.dateDecodingStrategy = forgejoDateDecodingStrategy return try decoder.decode(User.self, from: data) - case 401: - let body = String(data: data, encoding: .utf8) ?? "" - throw body.contains("OTP") ? AuthenticationError.otpRequired : .invalidCredentials - case 404: - throw AuthenticationError.serverNotFound default: - throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode) + let body = String(data: data, encoding: .utf8) ?? "" + throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body) } } catch let error as URLError where Self.certificateErrorCodes.contains(error.code) { throw AuthenticationError.certificateError @@ -187,17 +183,18 @@ public final class ForgejoClient: Sendable { // Token name already taken — Forgejo returns 400, some versions use 422 let body = String(data: data, encoding: .utf8) ?? "" guard body.contains("name") else { - throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode) + throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body) } guard retryCount < 3 else { - throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode) + throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body) } let retryName = "\(name)-\(Int(Date().timeIntervalSince1970))" return try await postTokenRequest( url: url, name: retryName, otp: otp, retryCount: retryCount + 1, ) default: - throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode) + let body = String(data: data, encoding: .utf8) ?? "" + throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body) } } @@ -369,7 +366,30 @@ public struct LoginResult: Sendable { public let token: String } -public enum AuthenticationError: LocalizedError, Sendable { +public enum HTTPErrorCategory: Equatable, Sendable { + case authentication + case permissionDenied + case notFound + case server + case other + + public init(statusCode: Int) { + switch statusCode { + case 401: + self = .authentication + case 403: + self = .permissionDenied + case 404: + self = .notFound + case 500 ... 599: + self = .server + default: + self = .other + } + } +} + +public enum AuthenticationError: LocalizedError, Sendable, Equatable { case invalidURL case invalidCredentials case otpRequired @@ -378,6 +398,37 @@ public enum AuthenticationError: LocalizedError, Sendable { case certificateError case unknownError(statusCode: Int) + static func from(statusCode: Int, body: String = "") -> AuthenticationError { + switch statusCode { + case 401: + body.localizedCaseInsensitiveContains("OTP") ? .otpRequired : .invalidCredentials + case 404: + .serverNotFound + default: + .unknownError(statusCode: statusCode) + } + } + + public var httpStatusCode: Int? { + switch self { + case .invalidCredentials, .otpRequired: + 401 + case .serverNotFound: + 404 + case let .unknownError(statusCode): + statusCode + case .invalidURL, .invalidResponse, .certificateError: + nil + } + } + + public var httpErrorCategory: HTTPErrorCategory? { + guard let httpStatusCode else { + return nil + } + return HTTPErrorCategory(statusCode: httpStatusCode) + } + public var errorDescription: String? { switch self { case .invalidURL: @@ -393,7 +444,14 @@ public enum AuthenticationError: LocalizedError, Sendable { case .certificateError: "SSL certificate error. Try enabling 'Accept Self-Signed Certificates' if your server uses one." case let .unknownError(statusCode): - "Unknown error occurred (Status: \(statusCode))" + switch HTTPErrorCategory(statusCode: statusCode) { + case .permissionDenied: + "Permission denied (Status: \(statusCode))" + case .server: + "Server error occurred (Status: \(statusCode))" + case .authentication, .notFound, .other: + "Unknown error occurred (Status: \(statusCode))" + } } } } @@ -407,6 +465,20 @@ public enum ServiceError: LocalizedError, Sendable { case mergeConflict case decodingFailed(detail: String) + public var httpStatusCode: Int? { + guard case let .httpError(statusCode, _) = self else { + return nil + } + return statusCode + } + + public var httpErrorCategory: HTTPErrorCategory? { + guard let httpStatusCode else { + return nil + } + return HTTPErrorCategory(statusCode: httpStatusCode) + } + public var errorDescription: String? { switch self { case .noActiveInstance: diff --git a/Tests/ForgejoKitTests/ErrorMappingTests.swift b/Tests/ForgejoKitTests/ErrorMappingTests.swift new file mode 100644 index 0000000..9af12e4 --- /dev/null +++ b/Tests/ForgejoKitTests/ErrorMappingTests.swift @@ -0,0 +1,48 @@ +import Testing +@testable import ForgejoKit + +struct ErrorMappingTests { + + @Test func classifiesHTTPStatusCodes() { + #expect(HTTPErrorCategory(statusCode: 401) == .authentication) + #expect(HTTPErrorCategory(statusCode: 403) == .permissionDenied) + #expect(HTTPErrorCategory(statusCode: 404) == .notFound) + #expect(HTTPErrorCategory(statusCode: 500) == .server) + #expect(HTTPErrorCategory(statusCode: 503) == .server) + #expect(HTTPErrorCategory(statusCode: 400) == .other) + } + + @Test func exposesServiceErrorStatusAndCategory() { + let forbidden = ServiceError.httpError(statusCode: 403, message: "missing scope") + #expect(forbidden.httpStatusCode == 403) + #expect(forbidden.httpErrorCategory == .permissionDenied) + + let serverError = ServiceError.httpError(statusCode: 502) + #expect(serverError.httpStatusCode == 502) + #expect(serverError.httpErrorCategory == .server) + + #expect(ServiceError.invalidURL.httpStatusCode == nil) + #expect(ServiceError.invalidURL.httpErrorCategory == nil) + } + + @Test func mapsAuthenticationStatusCodes() { + #expect(AuthenticationError.from(statusCode: 401) == .invalidCredentials) + #expect(AuthenticationError.from(statusCode: 401, body: "OTP required") == .otpRequired) + #expect(AuthenticationError.from(statusCode: 403) == .unknownError(statusCode: 403)) + #expect(AuthenticationError.from(statusCode: 404) == .serverNotFound) + #expect(AuthenticationError.from(statusCode: 500) == .unknownError(statusCode: 500)) + #expect(AuthenticationError.from(statusCode: 418) == .unknownError(statusCode: 418)) + } + + @Test func exposesAuthenticationStatusAndCategory() { + #expect(AuthenticationError.invalidCredentials.httpStatusCode == 401) + #expect(AuthenticationError.invalidCredentials.httpErrorCategory == .authentication) + #expect(AuthenticationError.serverNotFound.httpStatusCode == 404) + #expect(AuthenticationError.serverNotFound.httpErrorCategory == .notFound) + #expect(AuthenticationError.unknownError(statusCode: 403).httpStatusCode == 403) + #expect(AuthenticationError.unknownError(statusCode: 403).httpErrorCategory == .permissionDenied) + #expect(AuthenticationError.unknownError(statusCode: 502).httpErrorCategory == .server) + #expect(AuthenticationError.certificateError.httpStatusCode == nil) + #expect(AuthenticationError.certificateError.httpErrorCategory == nil) + } +}