Wait for TaskQueue to be ready before calling wrapper chain #3
Conversation
If we are not waiting for the TaskQueue, of the currently active pouchdb instance to be ready, before calling the handler, we might end up calling it twice. Once due to the call from the outside, which initiated the chain and once from the queuing itself, which calls it again, once the pouchdb instance is ready. This race condition does not always happen, but I found it to happen more often with PouchDB >6.1.1 and the idb-adapter. This is most likely due to optimizations in the setup process of this adapter.
|
@nolanlawson @daleharvey @garrensmith @marten-de-vries can one of you merge and release the new version? Or add me as npm collaborator to the package? This looks solid to me, it goes together with pouchdb-community/transform-pouch#46 (review) |
| }); | ||
| } else { | ||
| promise = method(); | ||
| } |
There was a problem hiding this comment.
another pattern and maybe more readable would be
var promise = Promise.resolve()
if (!args.base.taskqueue.isReady) {
promise = promise.then(function () {
new Promise(function (resolve, reject) {
args.base.taskqueue.addTask(function (failed) {
if (failed) {
return reject(failed);
}
resolve();
});
});
})
}
promise = promise.then(function () {
return method()
})There was a problem hiding this comment.
You are right. I usually use this pattern as well. I have got no idea, why I didn't do it here. Should I change it and push a new commit, or do you want to stay with the original implementation?
There was a problem hiding this comment.
if you agree that it’s more readable, send another commit, yeah. We can squash & merge the commits once we merge. Thanks
|
Requested changes added. |
This reverts commit 7e10695.
|
I reverted the commit, when I realized it doesn't behave correctly in correspondence with the application I am using it in. The original version seems to work quite fine. I am not sure, where the difference between the two implementations is. (Expect a missing However I wanted to make sure no "broken" version is merged. The given one does function properly, so I would suggest to stay with it, as I currently don't have the time to find out why the new version is not identical to the first one. |
|
Howdy! This bug is still a huge problem! Specifically it shows up in the test suite for It looks like the Travis CI build failed over Ruby arcana. Is there anything a passerby can do to help this PR get merged? |
|
Turns out this repo isn’t really used in transform-pouch (where we need it), but there is a variant of this published as part of pouchdb-server (https://github.com/pouchdb/pouchdb-server/tree/master/packages/node_modules/pouchdb-wrappers). |
If we are not waiting for the TaskQueue, of the currently active pouchdb instance to be ready, before calling the handler, we might end up calling it twice. Once due to the call from the outside, which initiated the chain and once from the queuing itself, which calls it again, once the pouchdb instance is ready.
This race condition does not always happen, but I found it to happen more often with PouchDB >6.1.1 and the idb-adapter. This is most likely due to optimizations in the setup process of this adapter.