Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

Can we use new version of express-pouchdb 2.3.6 instead of 1.0.6-thali? #1840

Open
lesn1kk opened this issue Mar 22, 2017 · 3 comments
Open
Assignees

Comments

@lesn1kk
Copy link
Member

lesn1kk commented Mar 22, 2017

Does express-pouchdb 2.3.6 contain fixes that were implemented in 1.0.6-thali version? Because right now it is quite outdated, TeamONE app uses newest version and I am afraid this can cause issues.

@yaronyg
Copy link
Member

yaronyg commented Mar 22, 2017

See #1464. If we can port those fixes to express-pouchdb then we can go back to the mainline. But the fix isn't quite as simple as it looks because when we made those changes there was a plan in place to have a mono-repo based in pouchdb-server that would include express-pouchdb and we made our fixes against the first draft of that plan.

The good news is that the mono repo happened.

The bad news is that it was done using a completely different structure then the one we were working against! So we can't just apply our changes.

Instead we have to go to https://github.com/yaronyg/pouchdb-server/commits/thali-release and take the last 7 changes and manually apply them to the new pouchdb-server monorepo.

The good news is that this really isn't much work. If you walk through the changes you will see that we changed just a handful of lines in just 2 files.

So it should be pretty fast for someone to go through pouchdb-server, check those two files, see if the changes are still needed and if so submit a PR.

@chapko
Copy link
Contributor

chapko commented Mar 22, 2017

Only 2 commits of those 7 contain actual changes, others are just merge commits and version updates.

@yaronyg As far as I understand, the first commit (yaronyg/pouchdb-server@a012907) fixes memory leaks. It is partially fixed here – changes are cancelled when connection is closed. But it fixed only for 'longpoll' feed, and not for 'continuous'. Author of the commit suggests to implement it for the 'continuous' but it wasn't implemented. I believe fixing it will achieve the same result as your commit.

IIRC the second commit (yaronyg/pouchdb-server@31545a0) by @andrew-aladev fixes the following scenario:

  1. We start replication, and server instantly replies with '{"results":[\n'.
  2. We somewhere call res.end();
  3. Client parses received JSON ('{"results":[\n') and throws an error.

It looks like the only place, where it could happen is on this line:

            changes = req.db.changes(req.query)
              .on('change', function (change) {
                utils.writeJSON(res, change);
                res.write('],\n"last_seq":' + change.seq + '}\n');
                cleanup();
              }).on('error', function (err) {
                // shouldn't happen
                console.log(err);
/* here -> */   cleanup();
              });

And in the cleanup we call res.end(). I need to run more test without this commit to see where it happens exactly. After that it probably will be clear if these changes are required for new express-pouchdb.

@yaronyg
Copy link
Member

yaronyg commented Mar 23, 2017

@chapko What you said. :) As I said, this is all super minor stuff. If we put it into mainline express then we can get off our custom version.

@chapko chapko removed their assignment Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants