mirror of
https://codeberg.org/secana/ForgejoKit.git
synced 2026-06-16 05:13:53 -07:00
feat: classify auth and service errors (#2)
Problem - Callers currently need to inspect raw status codes to distinguish authentication, permission, not-found, and server failures. Change - Adds a small HTTPErrorCategory for 401, 403, 404, and 5xx responses. - Exposes httpStatusCode and httpErrorCategory on ServiceError and AuthenticationError. - Keeps the existing AuthenticationError cases intact and centralizes status-to-auth-error mapping. Tests - DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer swift test Co-authored-by: Piotr Durlej <pdurlej@users.noreply.github.com> Reviewed-on: https://codeberg.org/secana/ForgejoKit/pulls/2 Reviewed-by: secana <secana@noreply.codeberg.org>
This commit is contained in:
parent
b87e4980a0
commit
8a5e513c50
2 changed files with 131 additions and 11 deletions
|
|
@ -115,13 +115,9 @@ public final class ForgejoClient: Sendable {
|
||||||
let decoder = JSONDecoder()
|
let decoder = JSONDecoder()
|
||||||
decoder.dateDecodingStrategy = forgejoDateDecodingStrategy
|
decoder.dateDecodingStrategy = forgejoDateDecodingStrategy
|
||||||
return try decoder.decode(User.self, from: data)
|
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:
|
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) {
|
} catch let error as URLError where Self.certificateErrorCodes.contains(error.code) {
|
||||||
throw AuthenticationError.certificateError
|
throw AuthenticationError.certificateError
|
||||||
|
|
@ -187,17 +183,18 @@ public final class ForgejoClient: Sendable {
|
||||||
// Token name already taken — Forgejo returns 400, some versions use 422
|
// Token name already taken — Forgejo returns 400, some versions use 422
|
||||||
let body = String(data: data, encoding: .utf8) ?? ""
|
let body = String(data: data, encoding: .utf8) ?? ""
|
||||||
guard body.contains("name") else {
|
guard body.contains("name") else {
|
||||||
throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode)
|
throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body)
|
||||||
}
|
}
|
||||||
guard retryCount < 3 else {
|
guard retryCount < 3 else {
|
||||||
throw AuthenticationError.unknownError(statusCode: httpResponse.statusCode)
|
throw AuthenticationError.from(statusCode: httpResponse.statusCode, body: body)
|
||||||
}
|
}
|
||||||
let retryName = "\(name)-\(Int(Date().timeIntervalSince1970))"
|
let retryName = "\(name)-\(Int(Date().timeIntervalSince1970))"
|
||||||
return try await postTokenRequest(
|
return try await postTokenRequest(
|
||||||
url: url, name: retryName, otp: otp, retryCount: retryCount + 1,
|
url: url, name: retryName, otp: otp, retryCount: retryCount + 1,
|
||||||
)
|
)
|
||||||
default:
|
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 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 invalidURL
|
||||||
case invalidCredentials
|
case invalidCredentials
|
||||||
case otpRequired
|
case otpRequired
|
||||||
|
|
@ -378,6 +398,37 @@ public enum AuthenticationError: LocalizedError, Sendable {
|
||||||
case certificateError
|
case certificateError
|
||||||
case unknownError(statusCode: Int)
|
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? {
|
public var errorDescription: String? {
|
||||||
switch self {
|
switch self {
|
||||||
case .invalidURL:
|
case .invalidURL:
|
||||||
|
|
@ -393,7 +444,14 @@ public enum AuthenticationError: LocalizedError, Sendable {
|
||||||
case .certificateError:
|
case .certificateError:
|
||||||
"SSL certificate error. Try enabling 'Accept Self-Signed Certificates' if your server uses one."
|
"SSL certificate error. Try enabling 'Accept Self-Signed Certificates' if your server uses one."
|
||||||
case let .unknownError(statusCode):
|
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 mergeConflict
|
||||||
case decodingFailed(detail: String)
|
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? {
|
public var errorDescription: String? {
|
||||||
switch self {
|
switch self {
|
||||||
case .noActiveInstance:
|
case .noActiveInstance:
|
||||||
|
|
|
||||||
48
Tests/ForgejoKitTests/ErrorMappingTests.swift
Normal file
48
Tests/ForgejoKitTests/ErrorMappingTests.swift
Normal file
|
|
@ -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)
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue