Skip to content

Conversation

hamzahrmalik
Copy link
Contributor

@hamzahrmalik hamzahrmalik commented Nov 14, 2023

Avoid logging localizedDescription for errors which don't conform to LocalizedError

some errors might come from underlying libraries
This results is unuseful messages if those libraries don't conform their errors to LocalizedError
For example, a NIOSSL error gets logged as

The operation could not be completed. (NIOSSL.NIOSSLError error 0.)

We do not get the full underlying error

See the added test case for a further example

Avoid logging localizedDescription for errors which don't conform to LocalizedError
@fabianfett fabianfett requested a review from Joannis November 14, 2023 15:45
@fabianfett fabianfett added the 🔨 semver/patch No public API change. label Nov 14, 2023
@testable import RediStack
import XCTest

class RedisErrorTests: XCTestCase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class RedisErrorTests: XCTestCase {
final class RedisErrorTests: XCTestCase {

@Joannis
Copy link
Member

Joannis commented Nov 14, 2023

Thanks for the PR! Looks good to me

@fabianfett
Copy link
Member

@swift-server-bot test this please

1 similar comment
@fabianfett
Copy link
Member

@swift-server-bot test this please

@fabianfett
Copy link
Member

@swift-server-bot test this please

@fabianfett
Copy link
Member

issue with installing jazzy. retry. @swift-server-bot test this please

@fabianfett
Copy link
Member

@swift-server-bot test this please

@fabianfett fabianfett merged commit 5a81dd7 into swift-server:main Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants