Skip to content

Conversation

@wolrajhti
Copy link

@wolrajhti wolrajhti commented Feb 18, 2021

Description

In 3.6.4 was added a depreciation warning about how writeConcern options should be formatted

console.warn(
`Top-level use of w, wtimeout, j, and fsync is deprecated. Use writeConcern instead.`
);

#2743 solves the problem of multiple emitions of depreciation warning, but digging into the code to find why this warning is triggered so many times, I found that sourceOptions.writeConcern is never merged into targetOptions.writeConcern in utils.mergeOptionsAndWriteConcern

// Write concern keys
var writeConcernKeys = ['w', 'j', 'wtimeout', 'fsync'];
// Merge the write concern options
var mergeOptionsAndWriteConcern = function(targetOptions, sourceOptions, keys, mergeWriteConcern) {
// Mix in any allowed options
for (var i = 0; i < keys.length; i++) {
if (!targetOptions[keys[i]] && sourceOptions[keys[i]] !== undefined) {
targetOptions[keys[i]] = sourceOptions[keys[i]];
}
}
// No merging of write concern
if (!mergeWriteConcern) return targetOptions;
// Found no write Concern options
var found = false;
for (i = 0; i < writeConcernKeys.length; i++) {
if (targetOptions[writeConcernKeys[i]]) {
found = true;
break;
}
}
if (!found) {
for (i = 0; i < writeConcernKeys.length; i++) {
if (sourceOptions[writeConcernKeys[i]]) {
targetOptions[writeConcernKeys[i]] = sourceOptions[writeConcernKeys[i]];
}
}
}
return targetOptions;
};

To fix it, my proposal is to add writeConcern to the collectionKeys in db.js. Also, utils.mergeOptionsAndWriteConcern is not part of the public API and used only 1 time in db.js, so I propose to remove the trailing, always true, argument : mergeWriteConcern

The original bug was reported here: https://jira.mongodb.org/browse/NODE-3114

@nodesocket
Copy link

@wolrajhti thanks so much for this work. Look forward to this fix merged and a new release.

wolrajhti referenced this pull request Mar 3, 2021
Updates all logging statements to use node's process.emitWarning API.
Allows for filtering, capturing and silencing of warnings.

NODE-2317, NODE-3114
@nbbeeken
Copy link
Contributor

nbbeeken commented Mar 3, 2021

Thanks again for the contribution!
Closing as #2743 contained the fix proposed here.

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.

3 participants