pouchdb / pouchdb-server

CouchDB-compatible server built on PouchDB and Node
Apache License 2.0
952 stars 154 forks source link

document update conflict when used repeatedly within async tests #226

Open jmatsushita opened 7 years ago

jmatsushita commented 7 years ago

Hi there,

Great project!

I am however encountering a problem while using pouchdb-server in a mocha test suite whereby using asynchronous tests yields document update conflict errors that only appear in stdout and are not anywhere else as far as I can tell.

I have boiled down the problem to this test case which has a synchronous version that works, and an asynchronous version which displays 409 errors on the console:

import PouchDB from 'pouchdb';
const app = require('express')();

let memdown = require('memdown');

let pouch, server, designdoc_rev, pouchdb_express;

const setup = done => {
  memdown.clearGlobalStore();

  const InMemPouchDB = require('pouchdb').defaults({ db: memdown });

  pouch = new InMemPouchDB('db');

  pouchdb_express = require('express-pouchdb')(InMemPouchDB);

  pouch.put(
    {
      _id: '_design/entities',
      language: 'javascript'
    },
    function(err, res) {
      designdoc_rev = res.rev;
      done();
    }
  );
};

const check = done => {
  pouch.remove('_design/entities', designdoc_rev, err => {
    if (err) return done(err);
    pouch.destroy(err => {
      if (err) return done(err);
      memdown.clearGlobalStore();
      server.close();
      done();
    });
  });
};

describe('inmem sync works', () => {
  beforeEach(done =>
    setup(() => {
      app.use('', pouchdb_express);
      server = app.listen(15984, done);
    })
  );

  afterEach(done => check(done));

  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  // no problem
});

describe('inmem async conflicts', () => {
  beforeEach(done =>
    setup(() => {
      app.use('', pouchdb_express);
      server = app.listen(15984, done);
    })
  );

  afterEach(done => check(done));

  const time = 0;

  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  // this gets shown in the console:
  //
  // { [conflict: Document update conflict]
  // status: 409,
  // name: 'conflict',
  // message: 'Document update conflict',
  // error: true,
  // id: 'db_db' }
  it('go', done => {
    setTimeout(done, time);
  });
  // and the same console error there.
});

I'm probably missing something with these global variables and the pouch and express teardowns or maybe mocha but I can't figure out what it is!

Help much appreciated!

Cheers,

Jun

jmatsushita commented 7 years ago

I'm using the following versions

├── mocha@3.4.1
└─┬ pouchdb-server@2.3.7
  ├── express@4.15.3
  └─┬ http-pouchdb@2.1.2
    └── pouchdb@6.2.0
jmatsushita commented 7 years ago

Increasing the timeout makes for some fun times. It seem that in fact the stdout errors from the sync suite where somehow swallowed, and they all come out when increasing the time constant to say 100. Still don't get what's happening and I have a nagging feeling that it's my code :)

jmatsushita commented 7 years ago

Some progress by doing a get remove query in the beforeEach hook, but there's still some weirdness with a 409 now in the sync part of the suite and related to db__users.

import PouchDB from 'pouchdb';
const app = require('express')();

let memdown = require('memdown');

let pouch, server, pouchdb_express;

const setup = done => {
  memdown.clearGlobalStore();

  const InMemPouchDB = require('pouchdb').defaults({ db: memdown });

  pouch = new InMemPouchDB('db');

  pouchdb_express = require('express-pouchdb')(InMemPouchDB);

  const put = cb =>
    pouch.put(
      {
        _id: '_design/entities',
        language: 'javascript'
      },
      function(err, res) {
        cb();
      }
    );

  pouch.get('_design/entities', (err, res) => {
    if (err) {
      put(done);
    } else {
      pouch.remove(res._id, res._rev, err => {
        put(done);
      });
    }
  });
};

const check = done => {
  // pouch.destroy(err => {
  // console.log('destroy.err', err);
  // if (err) return done(err);
  // memdown.clearGlobalStore();
  setImmediate(function() {
    server.close();
    done();
  });
  // });
};

describe('inmem sync works', () => {
  beforeEach(done =>
    setup(() => {
      app.use('', pouchdb_express);
      server = app.listen(15984, done);
    })
  );

  afterEach(done => check(done));

  it('go', () => {});
  it('go', () => {});
  // There is a document update conflict here now.
  //
  // { [conflict: Document update conflict]
  // status: 409,
  // name: 'conflict',
  // message: 'Document update conflict',
  // error: true,
  // id: 'db__users' }
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  it('go', () => {});
  // no problem
});

describe('inmem async conflicts', () => {
  beforeEach(done =>
    setup(() => {
      app.use('', pouchdb_express);
      server = app.listen(15984, done);
    })
  );

  afterEach(done => check(done));

  const time = 0;

  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
  it('go', done => {
    setTimeout(done, time);
  });
});
jmatsushita commented 7 years ago

I've tracked the error message. It's coming from this line in pouchdb-all-dbs which mentions istanbul and has a comment //shouldn't happen :)

@nolanlawson I'm running this within mocha (not with istanbul for this run).

I think this is related to destroying the db correctly, it seems that memdown.clearGlobalStore() is not enough. I've also tried with pouch.destroy() but still something is causing conflict, probably in combination with express-pouch. I'll investigate further with the debugger.

jmatsushita commented 7 years ago

Ah, I think the difficulty troubleshooting this comes from the asynchronous creation of the database. I'll try to setup my code to listen to the created event.

jmatsushita commented 7 years ago

Oh my, that's really confusing. Listening to the created event shows that if I pass already created databases to express-pouchdb then actually creates an all-db database and recreate the original ones.

This explains the comment in the pouchdb-all-dbs module for PouchDB.resetAllDbs([callback])

I just use it for the unit tests.

I'll try and follow that lead... Which there was some example doc for using pouch-server for unit -- more like integration tests I guess -- with proper teardown...