From 7a0d8e2a8a875ce3511fc9b3854bcf3b27f32149 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 18 Sep 2025 18:09:28 +0100 Subject: [PATCH 1/3] Add RESPDecodeError, somehow broken arrays though Signed-off-by: Adam Fowler --- .../Commands/Custom/GeoCustomCommands.swift | 4 +- .../Commands/Custom/ListCustomCommands.swift | 2 +- .../Custom/ServerCustomCommands.swift | 8 +-- .../Commands/Custom/SetCustomCommands.swift | 3 +- .../Custom/SortedSetCustomCommands.swift | 4 +- .../Custom/StreamCustomCommands.swift | 18 +++--- .../Custom/StringCustomCommands.swift | 7 ++- Sources/Valkey/RESP/RESPDecodeError.swift | 56 +++++++++++++++++ Sources/Valkey/RESP/RESPTokenDecodable.swift | 61 +++++++++++-------- Sources/Valkey/RESP/RESPTypeIdentifier.swift | 2 +- Sources/Valkey/Subscriptions/PushToken.swift | 2 +- Sources/Valkey/ValkeyKey.swift | 2 +- 12 files changed, 117 insertions(+), 52 deletions(-) create mode 100644 Sources/Valkey/RESP/RESPDecodeError.swift diff --git a/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift b/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift index 903fe0a3..d5139387 100644 --- a/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift @@ -44,7 +44,7 @@ extension GEOSEARCH { case .array(let array): var arrayIterator = array.makeIterator() guard let member = arrayIterator.next() else { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.invalidArraySize(array) } self.member = try String(fromRESP: member) self.attributes = array.dropFirst().map { $0 } @@ -54,7 +54,7 @@ extension GEOSEARCH { self.attributes = [] default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array, .bulkString], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/ListCustomCommands.swift b/Sources/Valkey/Commands/Custom/ListCustomCommands.swift index 15c74d25..46e86244 100644 --- a/Sources/Valkey/Commands/Custom/ListCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/ListCustomCommands.swift @@ -25,7 +25,7 @@ extension LMPOP { case .array(let array): (self.key, self.values) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift b/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift index 9b332ee9..d5e5d29a 100644 --- a/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift @@ -39,7 +39,7 @@ extension ROLE { public init(fromRESP token: RESPToken) throws { let string = try String(fromRESP: token) guard let state = State(rawValue: string) else { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedToken(token: token) } self = state } @@ -81,7 +81,7 @@ extension ROLE { do { var iterator = array.makeIterator() guard let roleToken = iterator.next() else { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.invalidArraySize(array) } let role = try String(fromRESP: roleToken) switch role { @@ -98,10 +98,10 @@ extension ROLE { throw DecodeError() } } catch { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedToken(token: token) } default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/SetCustomCommands.swift b/Sources/Valkey/Commands/Custom/SetCustomCommands.swift index a7af9510..f3d76f0c 100644 --- a/Sources/Valkey/Commands/Custom/SetCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/SetCustomCommands.swift @@ -12,8 +12,7 @@ extension SSCAN { public init(fromRESP token: RESPToken) throws { // cursor is encoded as a bulkString, but should be - let (cursorString, elements) = try token.decodeArrayElements(as: (String, RESPToken.Array).self) - guard let cursor = Int(cursorString) else { throw RESPParsingError(code: .unexpectedType, buffer: token.base) } + let (cursor, elements) = try token.decodeArrayElements(as: (Int, RESPToken.Array).self) self.cursor = cursor self.elements = elements } diff --git a/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift b/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift index 6610ab94..49d6987b 100644 --- a/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift @@ -18,7 +18,7 @@ public struct SortedSetEntry: RESPTokenDecodable, Sendable { case .array(let array): (self.value, self.score) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -56,7 +56,7 @@ extension ZMPOP { case .array(let array): (self.key, self.values) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift b/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift index 5609815c..f2e8b4b5 100644 --- a/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift @@ -21,7 +21,7 @@ public struct XREADMessage: RESPTokenDecodable, Sendable { self.id = id self.fields = keyValuePairs default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } @@ -77,7 +77,7 @@ public struct XREADGroupMessage: RESPTokenDecodable, Sendable { self.id = id self.fields = keyValuePairs default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -100,7 +100,7 @@ public struct XREADStreams: RESPTokenDecodable, Sendable where Message: return Stream(key: key, messages: messages) } default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map], token: token) } } } @@ -116,7 +116,7 @@ public struct XAUTOCLAIMResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.streamID, self.messages, self.deletedMessages) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -143,7 +143,7 @@ public enum XCLAIMResponse: RESPTokenDecodable, Sendable { self = try .ids(array.decode()) } default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -164,7 +164,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.consumer, self.count) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -178,7 +178,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.pendingMessageCount, self.minimumID, self.maximumID, self.consumers) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -194,7 +194,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.id, self.consumer, self.millisecondsSinceDelivered, self.numberOfTimesDelivered) = try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } @@ -205,7 +205,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): self.messages = try array.decode(as: [PendingMessage].self) default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/StringCustomCommands.swift b/Sources/Valkey/Commands/Custom/StringCustomCommands.swift index 031e0375..0c43b9b9 100644 --- a/Sources/Valkey/Commands/Custom/StringCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/StringCustomCommands.swift @@ -40,11 +40,12 @@ extension LCS { default: break } } - guard let matches else { throw RESPParsingError(code: .unexpectedType, buffer: token.base) } - guard let length else { throw RESPParsingError(code: .unexpectedType, buffer: token.base) } + guard let matches else { throw RESPDecodeError.missingToken(key: "matches", token: token) } + guard let length else { throw RESPDecodeError.missingToken(key: "length", token: token) } self = .matches(length: numericCast(length), matches: matches) default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.bulkString, .integer, .map], token: token) + } } } diff --git a/Sources/Valkey/RESP/RESPDecodeError.swift b/Sources/Valkey/RESP/RESPDecodeError.swift new file mode 100644 index 00000000..cd0ca886 --- /dev/null +++ b/Sources/Valkey/RESP/RESPDecodeError.swift @@ -0,0 +1,56 @@ +// +// This source file is part of the valkey-swift project +// Copyright (c) 2025 the valkey-swift project authors +// +// See LICENSE.txt for license information +// SPDX-License-Identifier: Apache-2.0 +// +/// Error returned when decoding a RESPToken. +/// Error thrown when decoding RESPTokens +public struct RESPDecodeError: Error, CustomStringConvertible { + @usableFromInline + enum InternalError: Sendable { + case unexpectedTokenIdentifier(expected: [RESPTypeIdentifier]) + case invalidArraySize + case missingToken(key: String) + case cannotParseInteger + case cannotParseDouble + case unexpectedToken + } + @usableFromInline + let error: InternalError + let token: RESPToken.Value + + public static func unexpectedTokenIdentifier(expected: [RESPTypeIdentifier], token: RESPToken) -> Self { + .init(error: .unexpectedTokenIdentifier(expected: expected), token: token.value) + } + public static func invalidArraySize(_ array: RESPToken.Array) -> Self { .init(error: .invalidArraySize, token: .array(array)) } + public static func missingToken(key: String, token: RESPToken) -> Self { .init(error: .missingToken(key: key), token: token.value) } + public static func cannotParseInteger(token: RESPToken) -> Self { .init(error: .cannotParseInteger, token: token.value) } + public static func cannotParseDouble(token: RESPToken) -> Self { .init(error: .cannotParseDouble, token: token.value) } + public static func unexpectedToken(token: RESPToken) -> Self { .init(error: .unexpectedToken, token: token.value) } + + public var description: String { + switch self.error { + case .unexpectedTokenIdentifier(let expected): + if expected.count == 0 { + return "Found unexpected token while decoding \(self.token)" + } else if expected.count == 1 { + return "Expected to find a \(expected[0]) token but found a \(self.token)" + } else { + let expectedTokens = "\(expected.dropLast().map { "\"\($0)\"" }.joined(separator: ", ")) or \"\(expected.last!)\"" + return "Expected to find a \(expectedTokens) token but found a \(self.token)" + } + case .invalidArraySize: + return "Invalid array length while decoding \(self.token)" + case .missingToken(let key): + return "Expected map to contain token with key \"\(key)\" while decoding \(self.token)" + case .cannotParseInteger: + return "Cannot parse integer while decoding \(self.token)" + case .cannotParseDouble: + return "Cannot parse double while decoding \(self.token)" + case .unexpectedToken: + return "Token was unexpected \(self.token)" + } + } +} diff --git a/Sources/Valkey/RESP/RESPTokenDecodable.swift b/Sources/Valkey/RESP/RESPTokenDecodable.swift index 8e58c25a..800d0f62 100644 --- a/Sources/Valkey/RESP/RESPTokenDecodable.swift +++ b/Sources/Valkey/RESP/RESPTokenDecodable.swift @@ -59,7 +59,7 @@ extension RESPToken: RESPTokenDecodable { case .array(let array), .set(let array): try array.decodeElements() default: - throw RESPParsingError(code: .unexpectedType, buffer: self.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: self) } } } @@ -96,7 +96,10 @@ extension ByteBuffer: RESPTokenDecodable { .map, .set, .push: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier( + expected: [.simpleString, .bulkString, .verbatimString, .bigNumber, .simpleError, .bulkError], + token: token + ) } } } @@ -128,7 +131,10 @@ extension String: RESPTokenDecodable { .map, .set, .push: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier( + expected: [.simpleString, .bulkString, .verbatimString, .bigNumber, .simpleError, .bulkError, .double, .integer, .boolean], + token: token + ) } } } @@ -141,7 +147,7 @@ extension Int64: RESPTokenDecodable { case .bulkString(let buffer): guard let value = Int64(String(buffer: buffer)) else { - throw RESPParsingError(code: .canNotParseInteger, buffer: token.base) + throw RESPDecodeError.cannotParseInteger(token: token) } self = value @@ -158,7 +164,7 @@ extension Int64: RESPTokenDecodable { .set, .null, .map: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.integer, .bulkString], token: token) } } } @@ -169,13 +175,13 @@ extension Int: RESPTokenDecodable { switch token.value { case .number(let value): guard let value = Int(exactly: value) else { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.cannotParseInteger(token: token) } self = value case .bulkString(let buffer): guard let value = Int(String(buffer: buffer)) else { - throw RESPParsingError(code: .canNotParseInteger, buffer: token.base) + throw RESPDecodeError.cannotParseInteger(token: token) } self = value @@ -192,7 +198,7 @@ extension Int: RESPTokenDecodable { .set, .null, .map: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.integer, .bulkString], token: token) } } } @@ -206,18 +212,18 @@ extension Double: RESPTokenDecodable { case .number(let value): guard let double = Double(exactly: value) else { - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.cannotParseDouble(token: token) } self = double case .bulkString(let buffer): guard let value = Double(String(buffer: buffer)) else { - throw RESPParsingError(code: .canNotParseDouble, buffer: token.base) + throw RESPDecodeError.cannotParseDouble(token: token) } self = value default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.double, .integer, .bulkString], token: token) } } } @@ -229,7 +235,7 @@ extension Bool: RESPTokenDecodable { case .boolean(let value): self = value default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.boolean], token: token) } } } @@ -258,14 +264,19 @@ extension Array: RESPTokenDecodable where Element: RESPTokenDecodable { array.append(element) } self = array - } catch let error as RESPParsingError where error.code == .unexpectedType { - // if decoding array failed it is possible `Element` is represented by an array and we have a single array - // that represents one element of `Element` instead of Array. We should attempt to decode this as a single element - let value = try Element(fromRESP: token) - self = [value] + } catch let error as RESPDecodeError { + switch error.error { + case .unexpectedToken: + // if decoding array failed it is possible `Element` is represented by an array and we have a single array + // that represents one element of `Element` instead of Array. We should attempt to decode this as a single element + let value = try Element(fromRESP: token) + self = [value] + default: + throw error + } } case .null: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) default: let value = try Element(fromRESP: token) self = [value] @@ -285,7 +296,7 @@ extension Set: RESPTokenDecodable where Element: RESPTokenDecodable { } self = set case .null: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.set], token: token) default: let value = try Element(fromRESP: token) self = [value] @@ -306,7 +317,7 @@ extension Dictionary: RESPTokenDecodable where Value: RESPTokenDecodable, Key: R } self = .init(array) { first, _ in first } default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map], token: token) } } } @@ -325,7 +336,7 @@ extension RESPToken.Array: RESPTokenDecodable { case .array(let respArray), .set(let respArray), .push(let respArray): self = respArray default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array, .set, .push], token: token) } } @@ -351,8 +362,7 @@ extension RESPToken.Array: RESPTokenDecodable { case .some(let value): return try T(fromRESP: value) case .none: - // TODO: Fixup error when we have a decoding error - throw RESPParsingError(code: .unexpectedType, buffer: token?.base ?? .init()) + throw RESPDecodeError.invalidArraySize(self) } } var iterator = self.makeIterator() @@ -372,8 +382,7 @@ extension RESPToken.Array: RESPTokenDecodable { case .some(let value): return value.decodeResult(as: T.self) case .none: - // TODO: Fixup error when we have a decoding error - return .failure(RESPParsingError(code: .unexpectedType, buffer: token?.base ?? .init())) + return .failure(RESPDecodeError.invalidArraySize(self)) } } var iterator = self.makeIterator() @@ -388,7 +397,7 @@ extension RESPToken.Map: RESPTokenDecodable { case .map(let respArray): self = respArray default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map, .attribute], token: token) } } diff --git a/Sources/Valkey/RESP/RESPTypeIdentifier.swift b/Sources/Valkey/RESP/RESPTypeIdentifier.swift index ad8b733e..dac3db85 100644 --- a/Sources/Valkey/RESP/RESPTypeIdentifier.swift +++ b/Sources/Valkey/RESP/RESPTypeIdentifier.swift @@ -6,7 +6,7 @@ // SPDX-License-Identifier: Apache-2.0 // /// A value that represents the response type. -public enum RESPTypeIdentifier: UInt8 { +public enum RESPTypeIdentifier: UInt8, Sendable { /// An integer case integer = 58 // UInt8(ascii: ":") /// A double diff --git a/Sources/Valkey/Subscriptions/PushToken.swift b/Sources/Valkey/Subscriptions/PushToken.swift index f7522060..f6ffe479 100644 --- a/Sources/Valkey/Subscriptions/PushToken.swift +++ b/Sources/Valkey/Subscriptions/PushToken.swift @@ -140,7 +140,7 @@ struct PushToken: RESPTokenDecodable { .map, .set, .attribute: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.push], token: token) } } } diff --git a/Sources/Valkey/ValkeyKey.swift b/Sources/Valkey/ValkeyKey.swift index 221b70c8..d2c81552 100644 --- a/Sources/Valkey/ValkeyKey.swift +++ b/Sources/Valkey/ValkeyKey.swift @@ -61,7 +61,7 @@ extension ValkeyKey: RESPTokenDecodable { case .simpleString(let buffer), .bulkString(let buffer): self._storage = .buffer(buffer) default: - throw RESPParsingError(code: .unexpectedType, buffer: token.base) + throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.simpleString, .bulkString], token: token) } } } From 0a4f85a70a9f10fba5d60957712b13abb02941a3 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 25 Sep 2025 16:08:01 +0000 Subject: [PATCH 2/3] Restructure RESPDecodeError to code, message and token Signed-off-by: Adam Fowler --- .../Commands/Custom/GeoCustomCommands.swift | 2 +- .../Commands/Custom/ListCustomCommands.swift | 2 +- .../Custom/ServerCustomCommands.swift | 18 +-- .../Custom/SortedSetCustomCommands.swift | 4 +- .../Custom/StreamCustomCommands.swift | 18 +-- .../Custom/StringCustomCommands.swift | 2 +- Sources/Valkey/RESP/RESPDecodeError.swift | 105 +++++++++++------- Sources/Valkey/RESP/RESPTokenDecodable.swift | 38 +++---- Sources/Valkey/Subscriptions/PushToken.swift | 2 +- Sources/Valkey/ValkeyKey.swift | 2 +- 10 files changed, 109 insertions(+), 84 deletions(-) diff --git a/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift b/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift index d5139387..33faecbe 100644 --- a/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/GeoCustomCommands.swift @@ -54,7 +54,7 @@ extension GEOSEARCH { self.attributes = [] default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array, .bulkString], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array, .bulkString], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/ListCustomCommands.swift b/Sources/Valkey/Commands/Custom/ListCustomCommands.swift index 46e86244..7fbc6846 100644 --- a/Sources/Valkey/Commands/Custom/ListCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/ListCustomCommands.swift @@ -25,7 +25,7 @@ extension LMPOP { case .array(let array): (self.key, self.values) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift b/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift index d5e5d29a..87758faf 100644 --- a/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/ServerCustomCommands.swift @@ -7,7 +7,7 @@ // extension ROLE { public enum Response: RESPTokenDecodable, Sendable { - struct DecodeError: Error {} + struct MissingValueDecodeError: Error {} public struct Primary: Sendable { public struct Replica: RESPTokenDecodable, Sendable { public let ip: String @@ -23,7 +23,7 @@ extension ROLE { init(arrayIterator: inout RESPToken.Array.Iterator) throws { guard let replicationOffsetToken = arrayIterator.next(), let replicasToken = arrayIterator.next() else { - throw DecodeError() + throw MissingValueDecodeError() } self.replicationOffset = try .init(fromRESP: replicationOffsetToken) self.replicas = try .init(fromRESP: replicasToken) @@ -39,7 +39,7 @@ extension ROLE { public init(fromRESP token: RESPToken) throws { let string = try String(fromRESP: token) guard let state = State(rawValue: string) else { - throw RESPDecodeError.unexpectedToken(token: token) + throw RESPDecodeError(.unexpectedToken, token: token) } self = state } @@ -55,7 +55,7 @@ extension ROLE { let stateToken = arrayIterator.next(), let replicationToken = arrayIterator.next() else { - throw DecodeError() + throw MissingValueDecodeError() } self.primaryIP = try .init(fromRESP: primaryIPToken) self.primaryPort = try .init(fromRESP: primaryPortToken) @@ -67,7 +67,7 @@ extension ROLE { public let primaryNames: [String] init(arrayIterator: inout RESPToken.Array.Iterator) throws { - guard let primaryNamesToken = arrayIterator.next() else { throw DecodeError() } + guard let primaryNamesToken = arrayIterator.next() else { throw MissingValueDecodeError() } self.primaryNames = try .init(fromRESP: primaryNamesToken) } } @@ -95,13 +95,13 @@ extension ROLE { let sentinel = try Sentinel(arrayIterator: &iterator) self = .sentinel(sentinel) default: - throw DecodeError() + throw RESPDecodeError(.unexpectedToken, token: token) } - } catch { - throw RESPDecodeError.unexpectedToken(token: token) + } catch is MissingValueDecodeError { + throw RESPDecodeError.invalidArraySize(array) } default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift b/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift index 49d6987b..efab30c2 100644 --- a/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/SortedSetCustomCommands.swift @@ -18,7 +18,7 @@ public struct SortedSetEntry: RESPTokenDecodable, Sendable { case .array(let array): (self.value, self.score) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -56,7 +56,7 @@ extension ZMPOP { case .array(let array): (self.key, self.values) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift b/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift index f2e8b4b5..38e38e62 100644 --- a/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/StreamCustomCommands.swift @@ -21,7 +21,7 @@ public struct XREADMessage: RESPTokenDecodable, Sendable { self.id = id self.fields = keyValuePairs default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } @@ -77,7 +77,7 @@ public struct XREADGroupMessage: RESPTokenDecodable, Sendable { self.id = id self.fields = keyValuePairs default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -100,7 +100,7 @@ public struct XREADStreams: RESPTokenDecodable, Sendable where Message: return Stream(key: key, messages: messages) } default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.map], token: token) } } } @@ -116,7 +116,7 @@ public struct XAUTOCLAIMResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.streamID, self.messages, self.deletedMessages) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -143,7 +143,7 @@ public enum XCLAIMResponse: RESPTokenDecodable, Sendable { self = try .ids(array.decode()) } default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -164,7 +164,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.consumer, self.count) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -178,7 +178,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.pendingMessageCount, self.minimumID, self.maximumID, self.consumers) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -194,7 +194,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): (self.id, self.consumer, self.millisecondsSinceDelivered, self.numberOfTimesDelivered) = try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } @@ -205,7 +205,7 @@ public enum XPENDINGResponse: RESPTokenDecodable, Sendable { case .array(let array): self.messages = try array.decode(as: [PendingMessage].self) default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) } } } diff --git a/Sources/Valkey/Commands/Custom/StringCustomCommands.swift b/Sources/Valkey/Commands/Custom/StringCustomCommands.swift index 0c43b9b9..b0dff0ff 100644 --- a/Sources/Valkey/Commands/Custom/StringCustomCommands.swift +++ b/Sources/Valkey/Commands/Custom/StringCustomCommands.swift @@ -44,7 +44,7 @@ extension LCS { guard let length else { throw RESPDecodeError.missingToken(key: "length", token: token) } self = .matches(length: numericCast(length), matches: matches) default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.bulkString, .integer, .map], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.bulkString, .integer, .map], token: token) } } diff --git a/Sources/Valkey/RESP/RESPDecodeError.swift b/Sources/Valkey/RESP/RESPDecodeError.swift index cd0ca886..266cd1b2 100644 --- a/Sources/Valkey/RESP/RESPDecodeError.swift +++ b/Sources/Valkey/RESP/RESPDecodeError.swift @@ -7,50 +7,75 @@ // /// Error returned when decoding a RESPToken. /// Error thrown when decoding RESPTokens -public struct RESPDecodeError: Error, CustomStringConvertible { - @usableFromInline - enum InternalError: Sendable { - case unexpectedTokenIdentifier(expected: [RESPTypeIdentifier]) - case invalidArraySize - case missingToken(key: String) - case cannotParseInteger - case cannotParseDouble - case unexpectedToken +public struct RESPDecodeError: Error { + /// Error code for decode error + public struct ErrorCode: Sendable, Equatable, CustomStringConvertible { + fileprivate enum Code: Sendable, Equatable { + case tokenMismatch + case invalidArraySize + case missingToken + case cannotParseInteger + case cannotParseDouble + case unexpectedToken + } + + fileprivate let code: Code + fileprivate init(_ code: Code) { + self.code = code + } + + public var description: String { String(describing: self.code) } + + /// Token does not match one of the expected tokens + public static var tokenMismatch: Self { .init(.tokenMismatch) } + /// Does not match the expected array size + public static var invalidArraySize: Self { .init(.invalidArraySize) } + /// Token is missing + public static var missingToken: Self { .init(.missingToken) } + /// Failed to parse an integer + public static var cannotParseInteger: Self { .init(.cannotParseInteger) } + /// Failed to parse a double + public static var cannotParseDouble: Self { .init(.cannotParseDouble) } + /// Token is not as expected + public static var unexpectedToken: Self { .init(.unexpectedToken) } } - @usableFromInline - let error: InternalError - let token: RESPToken.Value + public let errorCode: ErrorCode + public let message: String? + public let token: RESPToken.Value - public static func unexpectedTokenIdentifier(expected: [RESPTypeIdentifier], token: RESPToken) -> Self { - .init(error: .unexpectedTokenIdentifier(expected: expected), token: token.value) + public init(_ errorCode: ErrorCode, token: RESPToken.Value, message: String? = nil) { + self.errorCode = errorCode + self.token = token + self.message = message } - public static func invalidArraySize(_ array: RESPToken.Array) -> Self { .init(error: .invalidArraySize, token: .array(array)) } - public static func missingToken(key: String, token: RESPToken) -> Self { .init(error: .missingToken(key: key), token: token.value) } - public static func cannotParseInteger(token: RESPToken) -> Self { .init(error: .cannotParseInteger, token: token.value) } - public static func cannotParseDouble(token: RESPToken) -> Self { .init(error: .cannotParseDouble, token: token.value) } - public static func unexpectedToken(token: RESPToken) -> Self { .init(error: .unexpectedToken, token: token.value) } - public var description: String { - switch self.error { - case .unexpectedTokenIdentifier(let expected): - if expected.count == 0 { - return "Found unexpected token while decoding \(self.token)" - } else if expected.count == 1 { - return "Expected to find a \(expected[0]) token but found a \(self.token)" - } else { - let expectedTokens = "\(expected.dropLast().map { "\"\($0)\"" }.joined(separator: ", ")) or \"\(expected.last!)\"" - return "Expected to find a \(expectedTokens) token but found a \(self.token)" - } - case .invalidArraySize: - return "Invalid array length while decoding \(self.token)" - case .missingToken(let key): - return "Expected map to contain token with key \"\(key)\" while decoding \(self.token)" - case .cannotParseInteger: - return "Cannot parse integer while decoding \(self.token)" - case .cannotParseDouble: - return "Cannot parse double while decoding \(self.token)" - case .unexpectedToken: - return "Token was unexpected \(self.token)" + public init(_ errorCode: ErrorCode, token: RESPToken, message: String? = nil) { + self = .init(errorCode, token: token.value, message: message) + } + + /// Token does not match one of the expected tokens + public static func tokenMismatch(expected: [RESPTypeIdentifier], token: RESPToken) -> Self { + if expected.count == 0 { + return .init(.tokenMismatch, token: token, message: "Found unexpected token while decoding") + } else if expected.count == 1 { + return .init(.tokenMismatch, token: token, message: "Expected to find a \(expected[0])") + } else { + let expectedTokens = "\(expected.dropLast().map { "\($0)" }.joined(separator: ", ")) or \(expected.last!)" + return .init(.tokenMismatch, token: token, message: "Expected to find a \(expectedTokens) token") } } + /// Does not match the expected array size + public static func invalidArraySize(_ array: RESPToken.Array) -> Self { + .init(.invalidArraySize, token: .array(array)) + } + /// Token associated with key is missing + public static func missingToken(key: String, token: RESPToken) -> Self { + .init(.missingToken, token: token, message: "Expected map to contain token with key \"\(key)\"") + } +} + +extension RESPDecodeError: CustomStringConvertible { + public var description: String { + "Error: \"\(self.message ?? String(describing: self.errorCode))\", token: \(self.token.debugDescription)" + } } diff --git a/Sources/Valkey/RESP/RESPTokenDecodable.swift b/Sources/Valkey/RESP/RESPTokenDecodable.swift index 800d0f62..d6fe2f1a 100644 --- a/Sources/Valkey/RESP/RESPTokenDecodable.swift +++ b/Sources/Valkey/RESP/RESPTokenDecodable.swift @@ -59,7 +59,7 @@ extension RESPToken: RESPTokenDecodable { case .array(let array), .set(let array): try array.decodeElements() default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: self) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: self) } } } @@ -96,7 +96,7 @@ extension ByteBuffer: RESPTokenDecodable { .map, .set, .push: - throw RESPDecodeError.unexpectedTokenIdentifier( + throw RESPDecodeError.tokenMismatch( expected: [.simpleString, .bulkString, .verbatimString, .bigNumber, .simpleError, .bulkError], token: token ) @@ -131,7 +131,7 @@ extension String: RESPTokenDecodable { .map, .set, .push: - throw RESPDecodeError.unexpectedTokenIdentifier( + throw RESPDecodeError.tokenMismatch( expected: [.simpleString, .bulkString, .verbatimString, .bigNumber, .simpleError, .bulkError, .double, .integer, .boolean], token: token ) @@ -147,7 +147,7 @@ extension Int64: RESPTokenDecodable { case .bulkString(let buffer): guard let value = Int64(String(buffer: buffer)) else { - throw RESPDecodeError.cannotParseInteger(token: token) + throw RESPDecodeError(.cannotParseInteger, token: token) } self = value @@ -164,7 +164,7 @@ extension Int64: RESPTokenDecodable { .set, .null, .map: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.integer, .bulkString], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.integer, .bulkString], token: token) } } } @@ -175,13 +175,13 @@ extension Int: RESPTokenDecodable { switch token.value { case .number(let value): guard let value = Int(exactly: value) else { - throw RESPDecodeError.cannotParseInteger(token: token) + throw RESPDecodeError(.cannotParseInteger, token: token) } self = value case .bulkString(let buffer): guard let value = Int(String(buffer: buffer)) else { - throw RESPDecodeError.cannotParseInteger(token: token) + throw RESPDecodeError(.cannotParseInteger, token: token) } self = value @@ -198,7 +198,7 @@ extension Int: RESPTokenDecodable { .set, .null, .map: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.integer, .bulkString], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.integer, .bulkString], token: token) } } } @@ -212,18 +212,18 @@ extension Double: RESPTokenDecodable { case .number(let value): guard let double = Double(exactly: value) else { - throw RESPDecodeError.cannotParseDouble(token: token) + throw RESPDecodeError(.cannotParseDouble, token: token) } self = double case .bulkString(let buffer): guard let value = Double(String(buffer: buffer)) else { - throw RESPDecodeError.cannotParseDouble(token: token) + throw RESPDecodeError(.cannotParseDouble, token: token) } self = value default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.double, .integer, .bulkString], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.double, .integer, .bulkString], token: token) } } } @@ -235,7 +235,7 @@ extension Bool: RESPTokenDecodable { case .boolean(let value): self = value default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.boolean], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.boolean], token: token) } } } @@ -265,8 +265,8 @@ extension Array: RESPTokenDecodable where Element: RESPTokenDecodable { } self = array } catch let error as RESPDecodeError { - switch error.error { - case .unexpectedToken: + switch error.errorCode { + case .tokenMismatch: // if decoding array failed it is possible `Element` is represented by an array and we have a single array // that represents one element of `Element` instead of Array. We should attempt to decode this as a single element let value = try Element(fromRESP: token) @@ -276,7 +276,7 @@ extension Array: RESPTokenDecodable where Element: RESPTokenDecodable { } } case .null: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array], token: token) default: let value = try Element(fromRESP: token) self = [value] @@ -296,7 +296,7 @@ extension Set: RESPTokenDecodable where Element: RESPTokenDecodable { } self = set case .null: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.set], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.set], token: token) default: let value = try Element(fromRESP: token) self = [value] @@ -317,7 +317,7 @@ extension Dictionary: RESPTokenDecodable where Value: RESPTokenDecodable, Key: R } self = .init(array) { first, _ in first } default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.map], token: token) } } } @@ -336,7 +336,7 @@ extension RESPToken.Array: RESPTokenDecodable { case .array(let respArray), .set(let respArray), .push(let respArray): self = respArray default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.array, .set, .push], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.array, .set, .push], token: token) } } @@ -397,7 +397,7 @@ extension RESPToken.Map: RESPTokenDecodable { case .map(let respArray): self = respArray default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.map, .attribute], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.map, .attribute], token: token) } } diff --git a/Sources/Valkey/Subscriptions/PushToken.swift b/Sources/Valkey/Subscriptions/PushToken.swift index f6ffe479..a58f95b3 100644 --- a/Sources/Valkey/Subscriptions/PushToken.swift +++ b/Sources/Valkey/Subscriptions/PushToken.swift @@ -140,7 +140,7 @@ struct PushToken: RESPTokenDecodable { .map, .set, .attribute: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.push], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.push], token: token) } } } diff --git a/Sources/Valkey/ValkeyKey.swift b/Sources/Valkey/ValkeyKey.swift index d2c81552..fced38dd 100644 --- a/Sources/Valkey/ValkeyKey.swift +++ b/Sources/Valkey/ValkeyKey.swift @@ -61,7 +61,7 @@ extension ValkeyKey: RESPTokenDecodable { case .simpleString(let buffer), .bulkString(let buffer): self._storage = .buffer(buffer) default: - throw RESPDecodeError.unexpectedTokenIdentifier(expected: [.simpleString, .bulkString], token: token) + throw RESPDecodeError.tokenMismatch(expected: [.simpleString, .bulkString], token: token) } } } From b3e2422a99278938d3425e0d6ab4acaec9d52c79 Mon Sep 17 00:00:00 2001 From: Adam Fowler Date: Thu, 25 Sep 2025 16:08:13 +0000 Subject: [PATCH 3/3] Add tests for decode errors Signed-off-by: Adam Fowler --- Tests/ValkeyTests/RESPDecodeErrorTests.swift | 67 ++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 Tests/ValkeyTests/RESPDecodeErrorTests.swift diff --git a/Tests/ValkeyTests/RESPDecodeErrorTests.swift b/Tests/ValkeyTests/RESPDecodeErrorTests.swift new file mode 100644 index 00000000..69ae7d0b --- /dev/null +++ b/Tests/ValkeyTests/RESPDecodeErrorTests.swift @@ -0,0 +1,67 @@ +import Testing +// +// This source file is part of the valkey-swift project +// Copyright (c) 2025 the valkey-swift project authors +// +// See LICENSE.txt for license information +// SPDX-License-Identifier: Apache-2.0 +// +import Valkey + +struct RESPDecodeErrorTests { + @Test + func testTokenMismatchWith() { + let resp = RESPToken(.null) + let error = #expect(throws: RESPDecodeError.self) { + _ = try Bool(fromRESP: resp) + } + #expect(error?.errorCode == .tokenMismatch) + #expect(error?.message == #"Expected to find a boolean"#) + } + + @Test + func testTokenMismatchWithMultipleMatches() { + let resp = RESPToken(.null) + let error = #expect(throws: RESPDecodeError.self) { + _ = try Double(fromRESP: resp) + } + #expect(error?.errorCode == .tokenMismatch) + #expect(error?.message == #"Expected to find a double, integer or bulkString token"#) + print(error!) + } + + @Test + func testInvalidArraySize() { + struct Test: RESPTokenDecodable { + let number: Double + let number2: Double + init(fromRESP token: RESPToken) throws { + (self.number, self.number2) = try token.decodeArrayElements() + } + } + let resp = RESPToken(.array([.double(1.0)])) + let error = #expect(throws: RESPDecodeError.self) { + _ = try Test(fromRESP: resp) + } + #expect(error?.errorCode == .invalidArraySize) + } + + @Test + func testCannotParseInt() { + let resp = RESPToken(.bulkString("1.0")) + let error = #expect(throws: RESPDecodeError.self) { + _ = try Int(fromRESP: resp) + } + #expect(error?.errorCode == .cannotParseInteger) + print(error!) + } + + @Test + func testCannotParseDouble() { + let resp = RESPToken(.bulkString("1.0a")) + let error = #expect(throws: RESPDecodeError.self) { + _ = try Double(fromRESP: resp) + } + #expect(error?.errorCode == .cannotParseDouble) + } +}