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`.

Author: Mordil

Sources/NIORedis/Commands/BasicCommands.swift

@@ -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
         }

Sources/NIORedis/Commands/SortedSetCommands.swift

@@ -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."
             ))
         }
 

Sources/NIORedis/Extensions/NIO/EventLoopFuture.swift

@@ -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
         }
     }

Sources/NIORedis/RESP/RESPDecoder.swift

@@ -1,3 +1,4 @@
+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)))

Sources/NIORedis/RESP/RESPEncoder.swift

@@ -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):

Sources/NIORedis/RedisClient.swift

@@ -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)")
 

Sources/NIORedis/RedisError.swift

@@ -1,30 +1,33 @@
 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)"
     }
 }

Tests/NIORedisTests/ChannelHandlers/RESPDecoder+B2MDTests.swift

@@ -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 = [

Tests/NIORedisTests/ChannelHandlers/RESPDecoder+ParsingTests.swift

@@ -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

Tests/NIORedisTests/ChannelHandlers/RESPDecoderTests.swift

@@ -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
     }
 }