-
Notifications
You must be signed in to change notification settings - Fork 67
SWIFT-851 Namespace error types #491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SWIFT-851 Namespace error types #491
Conversation
| /// A MongoDB server error code. | ||
| /// - SeeAlso: https://github.com/mongodb/mongo/blob/master/src/mongo/base/error_codes.yml | ||
| public typealias ServerErrorCode = Int | ||
| public protocol MongoErrorProtocol: LocalizedError {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to disambiguate from the namespace, I added the Protocol suffix that I've seen in a few places in the stdlib (e.g. LazySequenceProtocol)
|
|
||
| /// Protocol conformed to by errors that may contain error labels. | ||
| public protocol LabeledError: MongoError { | ||
| public protocol MongoLabeledError: MongoErrorProtocol { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are limitations to this enum-as-namespace approach, namely protocols can't be defined that way. To avoid collisions, I added the Mongo prefix to all these protocols.
| /// Labels that may describe the context in which this error was thrown. | ||
| public let errorLabels: [String]? | ||
| /// Thrown when commands experience errors on the server that prevent execution. | ||
| public struct CommandError: MongoServerError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could even do sub-namespaces here for each of the error categories (e.g. MongoError.ServerError.CommandError), but that felt like overkill.
| public var errorDescription: String? { self.message } | ||
| } | ||
| /// A struct to represent a single write error not resulting from an executed write operation. | ||
| public struct WriteFailure: Codable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these -Failure types aren't errors themselves, but they are contained within different error types. I wasn't sure where to put them so I just left them under MongoError.
Codecov Report
@@ Coverage Diff @@
## master #491 +/- ##
==========================================
- Coverage 76.99% 76.98% -0.02%
==========================================
Files 125 125
Lines 13030 13050 +20
==========================================
+ Hits 10033 10046 +13
- Misses 2997 3004 +7
Continue to review full report at Codecov.
|
kmahar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what you've got here lgtm mod maybe handling the BSON stuff here, but I think probably all of our docstrings listing error types need to be updated now?
4908f6c to
beaac75
Compare
SWIFT-851
This PR moves all the error type definitions under a new empty enum
MongoError.It also renames
MongoErrortoMongoErrorProtocoland prefixes the other error protocols withMongo.