Improve error messages (swift-server#97)

Author: hamzahrmalik

Sources/RediStack/RedisConnection.swift

@@ -265,7 +265,7 @@ extension RedisConnection {
 
         guard self.isConnected else {
             let error = RedisClientError.connectionClosed
-            logger.warning("\(error.localizedDescription)")
+            logger.warning("\(error.loggableDescription)")
             return self.channel.eventLoop.makeFailedFuture(error)
         }
         logger.trace("received command request")
@@ -293,7 +293,7 @@ extension RedisConnection {
             switch result {
             case let .failure(error):
                 logger.error("command failed", metadata: [
-                    RedisLogging.MetadataKeys.error: "\(error.localizedDescription)"
+                    RedisLogging.MetadataKeys.error: "\(error.loggableDescription)"
                 ])
 
             case let .success(value):
@@ -448,7 +448,7 @@ extension RedisConnection {
                             logger.debug(
                                 "failed to add subscriptions that triggered pubsub mode. removing handler",
                                 metadata: [
-                                    RedisLogging.MetadataKeys.error: "\(error.localizedDescription)"
+                                    RedisLogging.MetadataKeys.error: "\(error.loggableDescription)"
                                 ]
                             )
                             // if there was an error, no subscriptions were made

Sources/RediStack/RedisError.swift

@@ -55,3 +55,16 @@ extension RedisError: RESPValueConvertible {
         return .error(self)
     }
 }
+
+extension Error {
+    /// Provides a description of the error which is suitable for logging
+    /// This uses localizedDescription if it is implemented, otherwise falls back to default string representation
+    /// This avoids hiding details for errors coming from other libraries (e.g. from swift-nio) which don't
+    /// conform to LocalizedError
+    var loggableDescription: String {
+        if self is LocalizedError {
+            return self.localizedDescription
+        }
+        return "\(self)"
+    }
+}

Tests/RediStackTests/Helpers/RedisErrorTests.swift

@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the RediStack open source project
+//
+// Copyright (c) YEARS RediStack project authors
+// Licensed under Apache License v2.0
+//
+// See LICENSE.txt for license information
+// See CONTRIBUTORS.txt for the list of RediStack project authors
+//
+// SPDX-License-Identifier: Apache-2.0
+//
+//===----------------------------------------------------------------------===//
+
+@testable import RediStack
+import XCTest
+
+final class RedisErrorTests: XCTestCase {
+    func testLoggableDescriptionLocalized() {
+        let error = RedisError(reason: "test")
+        XCTAssertEqual(error.loggableDescription, "(Redis) test")
+    }
+
+    func testLoggableDescriptionNotLocalized() {
+        struct MyError: Error, CustomStringConvertible {
+            var field: String
+            var description: String {
+                "description of \(self.field)"
+            }
+        }
+        let error = MyError(field: "test")
+        XCTAssertEqual(error.loggableDescription, "description of test")
+        // Trying to take a localizedDescription would give a less useful message like
+        // "The operation couldn’t be completed. (RediStackTests.RedisErrorTests.(unknown context at $10aa9f334).(unknown context at $10aa9f340).MyError error 1.)"
+        XCTAssertTrue(error.localizedDescription.contains("unknown context"))
+    }
+}