Commit 46e96bd9 authored by Nathan Harris's avatar Nathan Harris

Standardize Error Handling

Motivation:

`RedisError` was being used in many places in an ad-hoc fashion that made it unclear as to what source of errors it exactly represents.

In addition, it doesn't follow community conventions of handling known "classes" of errors with enums rather than static struct instances.

Results:

All errors that are generated and thrown within the library are specified as either `RESPDecoder.Error` or `NIORedisError`.

`RedisError` is simplified to represent an error message sent by a Redis instance that conforms to `LocalizedError`.
parent f89e64de
......@@ -134,10 +134,7 @@ extension RedisClient {
let value = result[0].string,
let position = Int(value)
else {
throw RedisError(
identifier: #function,
reason: "Unexpected value in response: \(result[0])"
)
throw NIORedisError.assertionFailure(message: "Unexpected value in response: \(result[0])")
}
return position
}
......
......@@ -17,7 +17,7 @@ extension RedisClient {
let scoreItem = response[scoreIsFirst ? index : index + 1]
guard let score = Double(scoreItem) else {
throw RedisError(identifier: #function, reason: "Unexpected response \"\(scoreItem)\"")
throw NIORedisError.assertionFailure(message: "Unexpected response: '\(scoreItem)'")
}
let elementIndex = scoreIsFirst ? index + 1 : index
......@@ -48,9 +48,9 @@ extension RedisClient {
options: Set<String> = []
) -> EventLoopFuture<Int> {
guard !options.contains("INCR") else {
return self.eventLoop.makeFailedFuture(RedisError(
identifier: #function,
reason: "INCR option is unsupported. Use zincrby(_:element:in:) instead."
return self.eventLoop.makeFailedFuture(NIORedisError.unsupportedOperation(
method: #function,
message: "INCR option is unsupported. Use zincrby(_:element:in:) instead."
))
}
......
......@@ -14,15 +14,7 @@ extension EventLoopFuture where Value == RESPValue {
) -> EventLoopFuture<T> where T: RESPValueConvertible
{
return self.flatMapThrowing {
guard let value = T($0) else {
throw RedisError(
identifier: #function,
reason: "Failed to convert RESP to \(String(describing: type))",
file: file,
function: function,
line: line
)
}
guard let value = T($0) else { throw NIORedisError.responseConversion(to: type) }
return value
}
}
......
import protocol Foundation.LocalizedError
import NIO
extension UInt8 {
......@@ -21,6 +22,16 @@ public final class RESPDecoder {
case parsed(RESPValue)
}
/// Representation of an `Swift.Error` found during RESP decoding.
public enum Error: LocalizedError {
case invalidToken
case arrayRecursion
public var errorDescription: String? {
return "RESPDecoding: \(self)"
}
}
public init() { }
/// Attempts to parse the `ByteBuffer`, starting at the specified position, following the RESP specification.
......@@ -58,9 +69,9 @@ public final class RESPDecoder {
let stringBuffer = parseSimpleString(&slice, &position),
let message = stringBuffer.getString(at: 0, length: stringBuffer.readableBytes)
else { return .notYetParsed }
return .parsed(.error(RedisError(identifier: "serverSide", reason: message)))
return .parsed(.error(RedisError(reason: message)))
default: throw RedisError(identifier: "invalidTokenType", reason: "Unexpected error while parsing Redis RESP.")
default: throw Error.invalidToken
}
}
}
......@@ -173,9 +184,7 @@ extension RESPDecoder {
}
let values = try results.map { state -> RESPValue in
guard case let .parsed(value) = state else {
throw RedisError(identifier: "parseArray", reason: "Unexpected error while parsing RESP.")
}
guard case let .parsed(value) = state else { throw Error.arrayRecursion }
return value
}
return .parsed(.array(ContiguousArray<RESPValue>(values)))
......
......@@ -45,7 +45,7 @@ public final class RESPEncoder {
case .error(let error):
out.writeStaticString("-")
out.writeString(error.description)
out.writeString(error.message)
out.writeStaticString("\r\n")
case .array(let array):
......
......@@ -130,8 +130,8 @@ public final class RedisConnection: RedisClient {
with arguments: [RESPValueConvertible]
) -> EventLoopFuture<RESPValue> {
guard isConnected else {
logger.error("Received command when connection was closed.")
return channel.eventLoop.makeFailedFuture(RedisError.connectionClosed)
logger.error("\(NIORedisError.connectionClosed.localizedDescription)")
return channel.eventLoop.makeFailedFuture(NIORedisError.connectionClosed)
}
let args = arguments.map { $0.convertedToRESPValue() }
......@@ -144,7 +144,7 @@ public final class RedisConnection: RedisClient {
promise.futureResult.whenComplete { result in
guard case let .failure(error) = result else { return }
self.logger.error("\(error)")
self.logger.error("\(error.localizedDescription)")
}
logger.debug("Sending command \"\(command)\" with \(arguments) encoded as \(args)")
......
import protocol Foundation.LocalizedError
import class Foundation.Thread
/// Errors thrown while working with Redis.
public struct RedisError: CustomDebugStringConvertible, CustomStringConvertible, LocalizedError {
public let description: String
public let debugDescription: String
/// When working with NIORedis, several errors are thrown to indicate problems
/// with state, assertions, or otherwise.
public enum NIORedisError: LocalizedError {
case connectionClosed
case responseConversion(to: Any.Type)
case unsupportedOperation(method: StaticString, message: String)
case assertionFailure(message: String)
public init(
identifier: String,
reason: String,
file: StaticString = #file,
function: StaticString = #function,
line: UInt = #line
) {
let name = String(describing: type(of: self))
description = "⚠️ [\(name).\(identifier): \(reason)]"
debugDescription = "⚠️ Redis Error: \(reason)\n- id: \(name).\(identifier)\n\n\(file): L\(line) - \(function)\n\n\(Thread.callStackSymbols)"
public var errorDescription: String? {
let message: String
switch self {
case .connectionClosed: message = "Connection was closed while trying to send command."
case let .responseConversion(type): message = "Failed to convert RESP to \(type)"
case let .unsupportedOperation(method, helpText): message = "\(method) - \(helpText)"
case let .assertionFailure(text): message = text
}
return "NIORedis: \(message)"
}
}
extension RedisError {
internal static var connectionClosed: RedisError {
return RedisError(identifier: "connection", reason: "Connection was closed while trying to execute.")
}
/// When sending commands to a Redis server, errors caught will be returned as an error message.
/// These messages are represented by `RedisError` instances.
public struct RedisError: LocalizedError {
public let message: String
public var errorDescription: String? { return message }
internal static func respConversion<T>(to dest: T.Type) -> RedisError {
return RedisError(identifier: "respConversion", reason: "Failed to convert RESP to \(String(describing: dest))")
public init(reason: String) {
message = "Redis: \(reason)"
}
}
......@@ -19,7 +19,7 @@ final class RESPDecoderByteToMessageDecoderTests: XCTestCase {
do {
_ = try decodeTest("&3\r\n").0
XCTFail("Failed to properly throw error")
} catch { XCTAssertTrue(error is RedisError) }
} catch { XCTAssertTrue(error is RESPDecoder.Error) }
}
private static let completeMessages = [
......
......@@ -96,7 +96,7 @@ final class RESPDecoderParsingTests: XCTestCase {
_ = try decoder.parse(from: &buffer, index: &position)
XCTFail("parse(at:from:) did not throw an expected error!")
}
catch { XCTAssertTrue(error is RedisError) }
catch { XCTAssertTrue(error is RESPDecoder.Error) }
return nil
}
......@@ -115,7 +115,7 @@ final class RESPDecoderParsingTests: XCTestCase {
return data
}
XCTAssertNotNil(result)
XCTAssertEqual(result?.error?.description.contains(expectedContent), true)
XCTAssertEqual(result?.error?.message.contains(expectedContent), true)
}
/// See parse_Test_singleValue(input:) String
......
......@@ -9,11 +9,11 @@ final class RESPDecoderTests: XCTestCase {
func test_error() throws {
XCTAssertNil(try runTest("-ERR"))
XCTAssertNil(try runTest("-ERR\r"))
XCTAssertEqual(try runTest("-ERROR\r\n")?.error?.description.contains("ERROR"), true)
XCTAssertEqual(try runTest("-ERROR\r\n")?.error?.message.contains("ERROR"), true)
let multiError: (RESPValue?, RESPValue?) = try runTest("-ERROR\r\n-OTHER ERROR\r\n")
XCTAssertEqual(multiError.0?.error?.description.contains("ERROR"), true)
XCTAssertEqual(multiError.1?.error?.description.contains("OTHER ERROR"), true)
XCTAssertEqual(multiError.0?.error?.message.contains("ERROR"), true)
XCTAssertEqual(multiError.1?.error?.message.contains("OTHER ERROR"), true)
}
func test_simpleString() throws {
......@@ -191,7 +191,7 @@ extension RESPDecoderTests {
XCTAssertEqual(results[0]?.string, AllData.expectedString)
XCTAssertEqual(results[1]?.int, AllData.expectedInteger)
XCTAssertEqual(results[2]?.error?.description.contains(AllData.expectedError), true)
XCTAssertEqual(results[2]?.error?.message.contains(AllData.expectedError), true)
XCTAssertEqual(results[3]?.string, AllData.expectedBulkString)
XCTAssertEqual(results[3]?.bytes, AllData.expectedBulkString.bytes)
......@@ -235,6 +235,6 @@ extension RESPDecoderTests {
extension RedisError: Equatable {
public static func == (lhs: RedisError, rhs: RedisError) -> Bool {
return lhs.description == rhs.description
return lhs.message == rhs.message
}
}
......@@ -42,8 +42,8 @@ final class RESPEncoderParsingTests: XCTestCase {
}
func testError() {
let error = RedisError(identifier: "testError", reason: "Manual error")
XCTAssertTrue(testPass(input: .error(error), expected: "-\(error.description)\r\n"))
let error = RedisError(reason: "Manual error")
XCTAssertTrue(testPass(input: .error(error), expected: "-\(error.message)\r\n"))
}
func testNull() {
......
......@@ -79,10 +79,10 @@ final class RESPEncoderTests: XCTestCase {
}
func testError() throws {
let error = RedisError(identifier: "testError", reason: "Manual error")
let error = RedisError(reason: "Manual error")
let data = RESPValue.error(error)
try runEncodePass(with: data) {
XCTAssertEqual($0.readableBytes, "-\(error.description)\r\n".bytes.count)
XCTAssertEqual($0.readableBytes, "-\(error.message)\r\n".bytes.count)
}
XCTAssertNoThrow(try channel.writeOutbound(data))
}
......
......@@ -22,8 +22,10 @@ final class BasicCommandsTests: XCTestCase {
XCTAssertNoThrow(try connection.select(database: 3).wait())
}
func test_delete() throws {
func test_delete() {
do {
let keys = [ #function + "1", #function + "2", #function + "3" ]
try connection.close().wait()
try connection.set(keys[0], to: "value").wait()
try connection.set(keys[1], to: "value").wait()
try connection.set(keys[2], to: "value").wait()
......@@ -36,6 +38,10 @@ final class BasicCommandsTests: XCTestCase {
let third = try connection.delete([keys[1], keys[2]]).wait()
XCTAssertEqual(third, 2)
}
catch {
print("failed")
}
}
func test_expire() throws {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment