Flaky tests

[alecgibson]

We've got some flaky tests that were sometimes failing the build before on Travis and are still failing now on GH Actions.

  1) db
       "after each" hook:
     Uncaught Error: Already closed
      at wrapErrorData (node_modules/sharedb/lib/client/connection.js:263:13)
      at Connection.handleMessage (node_modules/sharedb/lib/client/connection.js:191:11)
      at StreamSocket.socket.onmessage (node_modules/sharedb/lib/client/connection.js:142:18)
      at /home/travis/build/share/sharedb-mongo/node_modules/sharedb/lib/stream-socket.js:60:12
      at processTicksAndRejections (internal/process/task_queues.js:79:11)
  2) db
       "before each" hook for "subscribe bulk with readSnapshots rejectSnapshotRead special ignorable error":
     TypeError: Cannot read property 'call' of undefined
      at processImmediate (internal/timers.js:461:21)
  3) getOpsWithoutStrictLinking: true
       "after each" hook for "fetches ops 0-1 without fetching all ops":
     TypeError: Cannot read property 'close' of undefined
      at Context.<anonymous> (test/test_get_ops_without_strict_linking.js:36:13)
      at processImmediate (internal/timers.js:461:21)

[alecgibson]

I wonder if at least the first failure is because of this test: https://github.com/share/sharedb/blob/1038607535f364050f675aeb60e9d5a78f4b0593/test/db.js#L27-L37

    afterEach(function(done) {
      this.backend.close(done);
    });

    it('can call db.close() without callback', function(done) {
      create(function(err, db) {
        if (err) return done(err);
        db.close();
        done();
      });
    });

Here, we call db.close() twice: once in the test, and then again in the afterEach hook.

And in sharedb-mongo, this is what close() looks like:

sharedb-mongo/index.js

Lines 193 to 209 in c6a4395

	ShareDbMongo.prototype.close = function(callback) {
	if (!callback) {
	callback = function(err) {
	if (err) throw err;
	};
	}
	var self = this;
	this.getDbs(function(err) {
	if (err) return callback(err);
	self.closed = true;
	self._mongoClient.close(function(err) {
	if (err) return callback(err);
	if (!self._mongoPollClient) return callback();
	self._mongoPollClient.close(callback);
	});
	});
	};

Note that we call this.getDbs, which checks if we're already closed, and returns an error if so:

sharedb-mongo/index.js

Lines 111 to 114 in c6a4395

	if (this.closed) {
	var err = ShareDbMongo.alreadyClosedError();
	return callback(err);
	}

We could fix the upstream test, but I honestly feel like the real solution is just to check if we're already closed in close(), and early return if so. We could potentially even add a test for this to the upstream test suite, because I feel like you should be able to repeatedly call close() and everything should just be fine. Something like this:

  it('calls close multiple times', function(done) {
    var db = this.db;
    db.close(function(error) {
      if (error) return done(error);
      db.close(function(error) {
        done(error);
      });
    });
  });

[alecgibson]

Looks like that didn't fix it...

https://github.com/share/sharedb-mongo/pull/101/checks?check_run_id=1487922405

[alecgibson]

Hopefully fixed by share/sharedb#416 🤞