[UPDATE] fix error message with full error description. #61
Conversation
WalkthroughUpdates internal error messages in KurrentError: unknown errors and IOError.rethrow now include the usage wrapped in backticks and the phrase "full error" with the original error text. No public APIs or signatures were changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/KurrentDB/Core/Error/KurrentError.swift (1)
94-98: Don't use description for equality — match enum cases/semantic associated valuesComparing lhs.description == rhs.description ties equality to presentation text; changing message strings will break callers/tests (e.g. Tests/StreamsTests/StreamsTests.swift:29 expects KurrentError.resourceNotFound(reason: "...")).
- Replace the Equatable implementation in Sources/KurrentDB/Core/Error/KurrentError.swift (≈ lines 94–98) with explicit case matching that compares the enum case and only the semantic associated values (IDs, keys). Omit or normalize presentation-only "reason" strings unless they are part of the semantic identity.
- Update tests that assert full descriptions (e.g. Tests/StreamsTests/StreamsTests.swift:29) to assert the error case and/or specific associated values instead of exact message text.
Suggested approach (sketch):
switch (lhs, rhs) {
case (.resourceNotFound(let a), .resourceNotFound(let b)): return a == b
case (.notLeaderException, .notLeaderException): return true
/* compare other cases' semantic fields */
default: return false
}
🧹 Nitpick comments (4)
Sources/KurrentDB/Core/Error/KurrentError.swift (4)
157-160: Add period and double‑check leakage of sensitive details in “full error”.
- Consistency nit: end the interpolated error with a period to match other messages.
- Please verify that bubbling up
\(error)won’t surface secrets, tokens, or endpoints in user‑visible contexts.Apply this diff:
- throw .internalClientError(reason: "`\(usage)` failed. full error: \(error)") + throw .internalClientError(reason: "`\(usage)` failed. full error: \(error).")If you want a safer default, consider redacting known patterns (bearer tokens, connection URIs) before interpolation; I can draft a small sanitizer if you’d like.
191-199: Unify punctuation style for “full error” and maintain consistency.Use a period before “full error” (to match lines 157/170) for consistency.
- throw .internalClientError(reason: "`\(usage)` failed, full error: \(origin).") + throw .internalClientError(reason: "`\(usage)` failed. full error: \(origin).")
175-179: Match the new backtick style in the generic Error.rethrow helper.Align message formatting with other sites that now backtick
usage.- throw .internalClientError(reason: "\(usage) failed.") + throw .internalClientError(reason: "`\(usage)` failed.")
74-76: Minor grammar: “fill an issue” → “file an issue”.Small polish in user-facing text.
- "Unexpected internal client error. Please fill an issue on GitHub. reason: \(reason)" + "Unexpected internal client error. Please file an issue on GitHub. reason: \(reason)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Sources/KurrentDB/Core/Error/KurrentError.swift(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Swift 6.0 on ubuntu-latest
- GitHub Check: Swift 6.1 on ubuntu-latest
🔇 Additional comments (1)
Sources/KurrentDB/Core/Error/KurrentError.swift (1)
170-173: Mirror the punctuation fix and leakage check in the sync path.Same comments as the async variant: add a trailing period and verify no sensitive data is exposed.
- throw .internalClientError(reason: "`\(usage)` failed. full error: \(error)") + throw .internalClientError(reason: "`\(usage)` failed. full error: \(error).")
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
=======================================
Coverage 24.83% 24.83%
=======================================
Files 153 153
Lines 19653 19653
=======================================
Hits 4880 4880
Misses 14773 14773 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
PR Type
Bug fix
Description
Improved error message formatting with backticks around usage parameter
Enhanced error descriptions from "error:" to "full error:"
Added complete error information in IOError handling
Diagram Walkthrough
File Walkthrough
KurrentError.swift
Enhanced error message formatting and descriptionsSources/KurrentDB/Core/Error/KurrentError.swift
usageparameter in error messages for betterformatting
Summary by CodeRabbit