Skip to content
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

fix: backport emitWarning helper #2743

Merged
merged 5 commits into from
Mar 2, 2021
Merged

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Feb 17, 2021

Unify the way we print out messages relating to upcoming changes
or potential misuse of the driver. Using the emitWarning API allows
users to configure when and how messages are shown.

(code from #2740)

NODE-2317, NODE-3114

Unify the way we print out messages relating to upcoming changes
or potential misuse of the driver. Using the emitWarning API allows
users to configure when and how messages are shown.

NODE-2317
mergeOptionsAndWriteConcern was not updated to use the new format of
writeConcern and was causing the the warning about the deprecation to be
emitted regardless of user usage.

NODE-3114
@nbbeeken nbbeeken requested review from durran and emadum March 1, 2021 20:06
@nbbeeken nbbeeken marked this pull request as ready for review March 1, 2021 20:06
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

Added a suggestion about throwing a warning on type 1 op_msg payloads, like in 4.0. Otherwise LGTM!

@@ -196,7 +197,7 @@ class BinMsg {
while (this.index < this.data.length) {
const payloadType = this.data.readUInt8(this.index++);
if (payloadType === 1) {
console.error('TYPE 1');
emitWarning('TYPE 1'); // ???
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
emitWarning('TYPE 1'); // ???
// It was decided that no driver makes use of payload type 1
throw new MongoError('OP_MSG Payload Type 1 detected unsupported protocol');

Copy link
Member

@durran durran left a comment

Choose a reason for hiding this comment

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

LGTM

@wayahead
Copy link

#2744 is serious impact to many projects, so when the new 3.6 version will be released to npm registry? Thanks.

@nbbeeken
Copy link
Contributor Author

We hope to have a release out tomorrow! (March 16)

@wayahead
Copy link

@nbbeeken appreciate your guys effort!

This was referenced Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants