Skip to content

Commit

Permalink
Improve error messages (#97)
Browse files Browse the repository at this point in the history
  • Loading branch information
hamzahrmalik authored Nov 15, 2023
1 parent 4cd8a49 commit 5a81dd7
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
6 changes: 3 additions & 3 deletions Sources/RediStack/RedisConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions Sources/RediStack/RedisError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
}
}
37 changes: 37 additions & 0 deletions Tests/RediStackTests/Helpers/RedisErrorTests.swift
Original file line number Diff line number Diff line change
@@ -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"))
}
}

0 comments on commit 5a81dd7

Please sign in to comment.