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(connection): fix promise chaining for openUri #9960

Conversation

lantw44
Copy link
Contributor

@lantw44 lantw44 commented Feb 22, 2021

Summary

When a user calls openUri and attaches more than one callback to its return value with 'then' method, it should be able to pass the result of the first callback to the second one. Currently, openUri always pass 'undefined' to the second callback, making it behave differently from a regular promise. This is unlikely to be an expected behaviour.

To fix it, modify openUri to propagate the return value of the user-provided callback to the 'then' method of the promise.

Examples

const mongoose = require('mongoose')

mongoose.createConnection('mongodb://localhost:27017/test', {
  useNewUrlParser: true,
  useUnifiedTopology: true,
}).then((conn) => {
  console.log('callback 1', typeof conn);
  return 2;
}).then((x) => {
  console.log('callback 2', x);
  return 3;
}).then((x) => {
  console.log('callback 3', x);
  return 4;
}).catch((err) => {
  console.log('error', err);
})

Current behaviour:

callback 1 object
callback 2 undefined
callback 3 3

Expected behaviour:

callback 1 object
callback 2 2
callback 3 3

When a user calls openUri and attaches more than one callback to its
return value with 'then' method, it should be able to pass the result of
the first callback to the second one. Currently, openUri always pass
'undefined' to the second callback, making it behave differently from a
regular promise. This is unlikely to be an expected behaviour.

To fix it, modify openUri to propagate the return value of the
user-provided callback to the 'then' method of the promise.
Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Thanks for finding this issue 👍

@vkarpov15 vkarpov15 added this to the 5.11.18 milestone Feb 22, 2021
@vkarpov15 vkarpov15 added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Feb 22, 2021
@vkarpov15 vkarpov15 merged commit a4410b8 into Automattic:master Feb 22, 2021
This was referenced Mar 5, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants