Skip to content

Conversation

czechboy0
Copy link
Contributor

@czechboy0 czechboy0 commented Feb 24, 2025

Motivation

Fixes apple/swift-openapi-generator#730.

We were not correctly keeping CustomStringConvertible and LocalizedError conformances separate for wrapper errors like ClientError and ServerError. This lead to some user-thrown errors (in handlers, transports, and middlewares) to print less information than the error was actually providing (using a different method).

Modifications

Properly untangle the two printing codepaths, and only call localizedDescription from the wrapper error's errorDescription.

Also made the localizedDescription strings a bit more user-friendly and less detailed, as in some apps these errors might get directly rendered by a UI component that calls localizedDescription.

Result

Error logging should now match adopter expectations.

Test Plan

Added unit tests for {Client,Server}Error printing methods.

@czechboy0
Copy link
Contributor Author

cc @weissi

@czechboy0 czechboy0 mentioned this pull request Feb 24, 2025
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Feb 24, 2025
Copy link
Collaborator

@simonjbeaumont simonjbeaumont left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @czechboy0!

Took me a minute to confirm that prettyDescription didn't get in the way again, but I believe this resolves the ask and we can avoid making use of localized description in pretty description going forward too.

@simonjbeaumont simonjbeaumont merged commit acb9425 into apple:main Feb 24, 2025
24 of 25 checks passed
@czechboy0 czechboy0 deleted the hd-fix-error-descriptions branch February 24, 2025 11:31
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.

Separate error description vs localizedDescription
2 participants