Skip to content

Commit d03823f

Browse files
authored
Merge pull request #37 from Mordil/error-handling
Standardize Error Handling
2 parents f89e64d + 46e96bd commit d03823f

13 files changed

+67
-60
lines changed

Sources/NIORedis/Commands/BasicCommands.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,7 @@ extension RedisClient {
134134
let value = result[0].string,
135135
let position = Int(value)
136136
else {
137-
throw RedisError(
138-
identifier: #function,
139-
reason: "Unexpected value in response: \(result[0])"
140-
)
137+
throw NIORedisError.assertionFailure(message: "Unexpected value in response: \(result[0])")
141138
}
142139
return position
143140
}

Sources/NIORedis/Commands/SortedSetCommands.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ extension RedisClient {
1717
let scoreItem = response[scoreIsFirst ? index : index + 1]
1818

1919
guard let score = Double(scoreItem) else {
20-
throw RedisError(identifier: #function, reason: "Unexpected response \"\(scoreItem)\"")
20+
throw NIORedisError.assertionFailure(message: "Unexpected response: '\(scoreItem)'")
2121
}
2222

2323
let elementIndex = scoreIsFirst ? index + 1 : index
@@ -48,9 +48,9 @@ extension RedisClient {
4848
options: Set<String> = []
4949
) -> EventLoopFuture<Int> {
5050
guard !options.contains("INCR") else {
51-
return self.eventLoop.makeFailedFuture(RedisError(
52-
identifier: #function,
53-
reason: "INCR option is unsupported. Use zincrby(_:element:in:) instead."
51+
return self.eventLoop.makeFailedFuture(NIORedisError.unsupportedOperation(
52+
method: #function,
53+
message: "INCR option is unsupported. Use zincrby(_:element:in:) instead."
5454
))
5555
}
5656

Sources/NIORedis/Extensions/NIO/EventLoopFuture.swift

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,7 @@ extension EventLoopFuture where Value == RESPValue {
1414
) -> EventLoopFuture<T> where T: RESPValueConvertible
1515
{
1616
return self.flatMapThrowing {
17-
guard let value = T($0) else {
18-
throw RedisError(
19-
identifier: #function,
20-
reason: "Failed to convert RESP to \(String(describing: type))",
21-
file: file,
22-
function: function,
23-
line: line
24-
)
25-
}
17+
guard let value = T($0) else { throw NIORedisError.responseConversion(to: type) }
2618
return value
2719
}
2820
}

Sources/NIORedis/RESP/RESPDecoder.swift

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import protocol Foundation.LocalizedError
12
import NIO
23

34
extension UInt8 {
@@ -21,6 +22,16 @@ public final class RESPDecoder {
2122
case parsed(RESPValue)
2223
}
2324

25+
/// Representation of an `Swift.Error` found during RESP decoding.
26+
public enum Error: LocalizedError {
27+
case invalidToken
28+
case arrayRecursion
29+
30+
public var errorDescription: String? {
31+
return "RESPDecoding: \(self)"
32+
}
33+
}
34+
2435
public init() { }
2536

2637
/// Attempts to parse the `ByteBuffer`, starting at the specified position, following the RESP specification.
@@ -58,9 +69,9 @@ public final class RESPDecoder {
5869
let stringBuffer = parseSimpleString(&slice, &position),
5970
let message = stringBuffer.getString(at: 0, length: stringBuffer.readableBytes)
6071
else { return .notYetParsed }
61-
return .parsed(.error(RedisError(identifier: "serverSide", reason: message)))
72+
return .parsed(.error(RedisError(reason: message)))
6273

63-
default: throw RedisError(identifier: "invalidTokenType", reason: "Unexpected error while parsing Redis RESP.")
74+
default: throw Error.invalidToken
6475
}
6576
}
6677
}
@@ -173,9 +184,7 @@ extension RESPDecoder {
173184
}
174185

175186
let values = try results.map { state -> RESPValue in
176-
guard case let .parsed(value) = state else {
177-
throw RedisError(identifier: "parseArray", reason: "Unexpected error while parsing RESP.")
178-
}
187+
guard case let .parsed(value) = state else { throw Error.arrayRecursion }
179188
return value
180189
}
181190
return .parsed(.array(ContiguousArray<RESPValue>(values)))

Sources/NIORedis/RESP/RESPEncoder.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public final class RESPEncoder {
4545

4646
case .error(let error):
4747
out.writeStaticString("-")
48-
out.writeString(error.description)
48+
out.writeString(error.message)
4949
out.writeStaticString("\r\n")
5050

5151
case .array(let array):

Sources/NIORedis/RedisClient.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ public final class RedisConnection: RedisClient {
130130
with arguments: [RESPValueConvertible]
131131
) -> EventLoopFuture<RESPValue> {
132132
guard isConnected else {
133-
logger.error("Received command when connection was closed.")
134-
return channel.eventLoop.makeFailedFuture(RedisError.connectionClosed)
133+
logger.error("\(NIORedisError.connectionClosed.localizedDescription)")
134+
return channel.eventLoop.makeFailedFuture(NIORedisError.connectionClosed)
135135
}
136136

137137
let args = arguments.map { $0.convertedToRESPValue() }
@@ -144,7 +144,7 @@ public final class RedisConnection: RedisClient {
144144

145145
promise.futureResult.whenComplete { result in
146146
guard case let .failure(error) = result else { return }
147-
self.logger.error("\(error)")
147+
self.logger.error("\(error.localizedDescription)")
148148
}
149149
logger.debug("Sending command \"\(command)\" with \(arguments) encoded as \(args)")
150150

Sources/NIORedis/RedisError.swift

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,33 @@
11
import protocol Foundation.LocalizedError
2-
import class Foundation.Thread
32

4-
/// Errors thrown while working with Redis.
5-
public struct RedisError: CustomDebugStringConvertible, CustomStringConvertible, LocalizedError {
6-
public let description: String
7-
public let debugDescription: String
3+
/// When working with NIORedis, several errors are thrown to indicate problems
4+
/// with state, assertions, or otherwise.
5+
public enum NIORedisError: LocalizedError {
6+
case connectionClosed
7+
case responseConversion(to: Any.Type)
8+
case unsupportedOperation(method: StaticString, message: String)
9+
case assertionFailure(message: String)
810

9-
public init(
10-
identifier: String,
11-
reason: String,
12-
file: StaticString = #file,
13-
function: StaticString = #function,
14-
line: UInt = #line
15-
) {
16-
let name = String(describing: type(of: self))
17-
description = "⚠️ [\(name).\(identifier): \(reason)]"
18-
debugDescription = "⚠️ Redis Error: \(reason)\n- id: \(name).\(identifier)\n\n\(file): L\(line) - \(function)\n\n\(Thread.callStackSymbols)"
11+
public var errorDescription: String? {
12+
let message: String
13+
switch self {
14+
case .connectionClosed: message = "Connection was closed while trying to send command."
15+
case let .responseConversion(type): message = "Failed to convert RESP to \(type)"
16+
case let .unsupportedOperation(method, helpText): message = "\(method) - \(helpText)"
17+
case let .assertionFailure(text): message = text
18+
}
19+
return "NIORedis: \(message)"
1920
}
2021
}
2122

22-
extension RedisError {
23-
internal static var connectionClosed: RedisError {
24-
return RedisError(identifier: "connection", reason: "Connection was closed while trying to execute.")
25-
}
23+
/// When sending commands to a Redis server, errors caught will be returned as an error message.
24+
/// These messages are represented by `RedisError` instances.
25+
public struct RedisError: LocalizedError {
26+
public let message: String
27+
28+
public var errorDescription: String? { return message }
2629

27-
internal static func respConversion<T>(to dest: T.Type) -> RedisError {
28-
return RedisError(identifier: "respConversion", reason: "Failed to convert RESP to \(String(describing: dest))")
30+
public init(reason: String) {
31+
message = "Redis: \(reason)"
2932
}
3033
}

Tests/NIORedisTests/ChannelHandlers/RESPDecoder+B2MDTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ final class RESPDecoderByteToMessageDecoderTests: XCTestCase {
1919
do {
2020
_ = try decodeTest("&3\r\n").0
2121
XCTFail("Failed to properly throw error")
22-
} catch { XCTAssertTrue(error is RedisError) }
22+
} catch { XCTAssertTrue(error is RESPDecoder.Error) }
2323
}
2424

2525
private static let completeMessages = [

Tests/NIORedisTests/ChannelHandlers/RESPDecoder+ParsingTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ final class RESPDecoderParsingTests: XCTestCase {
9696
_ = try decoder.parse(from: &buffer, index: &position)
9797
XCTFail("parse(at:from:) did not throw an expected error!")
9898
}
99-
catch { XCTAssertTrue(error is RedisError) }
99+
catch { XCTAssertTrue(error is RESPDecoder.Error) }
100100

101101
return nil
102102
}
@@ -115,7 +115,7 @@ final class RESPDecoderParsingTests: XCTestCase {
115115
return data
116116
}
117117
XCTAssertNotNil(result)
118-
XCTAssertEqual(result?.error?.description.contains(expectedContent), true)
118+
XCTAssertEqual(result?.error?.message.contains(expectedContent), true)
119119
}
120120

121121
/// See parse_Test_singleValue(input:) String

Tests/NIORedisTests/ChannelHandlers/RESPDecoderTests.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ final class RESPDecoderTests: XCTestCase {
99
func test_error() throws {
1010
XCTAssertNil(try runTest("-ERR"))
1111
XCTAssertNil(try runTest("-ERR\r"))
12-
XCTAssertEqual(try runTest("-ERROR\r\n")?.error?.description.contains("ERROR"), true)
12+
XCTAssertEqual(try runTest("-ERROR\r\n")?.error?.message.contains("ERROR"), true)
1313

1414
let multiError: (RESPValue?, RESPValue?) = try runTest("-ERROR\r\n-OTHER ERROR\r\n")
15-
XCTAssertEqual(multiError.0?.error?.description.contains("ERROR"), true)
16-
XCTAssertEqual(multiError.1?.error?.description.contains("OTHER ERROR"), true)
15+
XCTAssertEqual(multiError.0?.error?.message.contains("ERROR"), true)
16+
XCTAssertEqual(multiError.1?.error?.message.contains("OTHER ERROR"), true)
1717
}
1818

1919
func test_simpleString() throws {
@@ -191,7 +191,7 @@ extension RESPDecoderTests {
191191

192192
XCTAssertEqual(results[0]?.string, AllData.expectedString)
193193
XCTAssertEqual(results[1]?.int, AllData.expectedInteger)
194-
XCTAssertEqual(results[2]?.error?.description.contains(AllData.expectedError), true)
194+
XCTAssertEqual(results[2]?.error?.message.contains(AllData.expectedError), true)
195195

196196
XCTAssertEqual(results[3]?.string, AllData.expectedBulkString)
197197
XCTAssertEqual(results[3]?.bytes, AllData.expectedBulkString.bytes)
@@ -235,6 +235,6 @@ extension RESPDecoderTests {
235235

236236
extension RedisError: Equatable {
237237
public static func == (lhs: RedisError, rhs: RedisError) -> Bool {
238-
return lhs.description == rhs.description
238+
return lhs.message == rhs.message
239239
}
240240
}

0 commit comments

Comments
 (0)