-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5688: Bump maxWireVersion for MongoDB 8.3 #1746
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
Conversation
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.
LGTM
@@ -189,7 +194,8 @@ public static ServerVersion ToServerVersion(this int wireVersion) | |||
WireVersion.Server80 => ServerVersion.Server80, | |||
WireVersion.Server81 => ServerVersion.Server81, | |||
WireVersion.Server82 => ServerVersion.Server82, | |||
_ => throw new ArgumentException($"Invalid write version: {wireVersion}.", nameof(wireVersion)) | |||
WireVersion.Server83 => ServerVersion.Server83, | |||
_ => throw new ArgumentException($"Invalid wire version: {wireVersion}.", nameof(wireVersion)) |
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.
I think we need something like a MongoConnectionException
and a better error message telling the user what to do, something like:
This version of the driver is not compatible with the version of the server.
We could add "Either use a newer version of the driver or an older version of the server" but I think the problem could happen at the other end also, so that might not be accurate... it could be "either use an older version of the driver or a newer version of the server".
We can probably tell which error message to add based on whether the unknown wire version is too large or too small.
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.
As discussed on Slack it turns out this exception is NOT being thrown from the connection code, so my comments above are not actually relevant.
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.
LGTM
@@ -189,7 +194,8 @@ public static ServerVersion ToServerVersion(this int wireVersion) | |||
WireVersion.Server80 => ServerVersion.Server80, | |||
WireVersion.Server81 => ServerVersion.Server81, | |||
WireVersion.Server82 => ServerVersion.Server82, | |||
_ => throw new ArgumentException($"Invalid write version: {wireVersion}.", nameof(wireVersion)) | |||
WireVersion.Server83 => ServerVersion.Server83, | |||
_ => throw new ArgumentException($"Invalid wire version: {wireVersion}.", nameof(wireVersion)) |
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.
As discussed on Slack it turns out this exception is NOT being thrown from the connection code, so my comments above are not actually relevant.
No description provided.