♻️ Extract util.callbackOrEmit() for synchronous error paths

lib/backend.js

@@ -106,10 +106,7 @@ Backend.prototype.close = function(callback) {
   var wait = 4;
   var backend = this;
   function finish(err) {
-    if (err) {
-      if (callback) return callback(err);
-      return backend.emit('error', err);
-    }
+    if (err) return util.callbackOrEmit(backend, err, callback);
     if (--wait) return;
     if (callback) callback();
   }

lib/client/doc.js

@@ -131,10 +131,7 @@ Doc.prototype.destroy = function(callback) {
   doc.whenNothingPending(function() {
     if (doc.wantSubscribe) {
       doc.unsubscribe(function(err) {
-        if (err) {
-          if (callback) return callback(err);
-          return doc.emit('error', err);
-        }
+        if (err) return util.callbackOrEmit(doc, err, callback);
         doc.connection._destroyDoc(doc);
         if (callback) callback();
       });
@@ -191,8 +188,7 @@ Doc.prototype.ingestSnapshot = function(snapshot, callback) {
       ERROR_CODE.ERR_INGESTED_SNAPSHOT_HAS_NO_VERSION,
       'Missing version in ingested snapshot. ' + this.collection + '.' + this.id
     );
-    if (callback) return callback(err);
-    return this.emit('error', err);
+    return util.callbackOrEmit(this, err, callback);
   }
 
   // If the doc is already created or there are ops pending, we cannot use the
@@ -214,8 +210,7 @@ Doc.prototype.ingestSnapshot = function(snapshot, callback) {
         ERROR_CODE.ERR_DOC_MISSING_VERSION,
         'Cannot ingest snapshot in doc with null version. ' + this.collection + '.' + this.id
       );
-      if (callback) return callback(err);
-      return this.emit('error', err);
+      return util.callbackOrEmit(this, err, callback);
     }
     // If we got a snapshot for a version further along than the document is
     // currently, issue a fetch to get the latest ops and catch us up
@@ -764,19 +759,15 @@ Doc.prototype._submit = function(op, source, callback) {
         );
       }
 
-      if (callback) return callback(err);
-      return this.emit('error', err);
+      return util.callbackOrEmit(this, err, callback);
     }
     // Try to normalize the op. This removes trailing skip:0's and things like that.
     if (this.type.normalize) op.op = this.type.normalize(op.op);
 
     // This has to happen before _pushOp(), because _tryCompose() applies the op
     // to a pending create, well before _otApply() gets a chance to check it
     var pathError = ot.checkOpPathsForType(this.type, op);
-    if (pathError) {
-      if (callback) return callback(pathError);
-      return this.emit('error', pathError);
-    }
+    if (pathError) return util.callbackOrEmit(this, pathError, callback);
   }
 
   try {
@@ -913,8 +904,7 @@ Doc.prototype.create = function(data, type, options, callback) {
   }
   if (this.type) {
     var err = new ShareDBError(ERROR_CODE.ERR_DOC_ALREADY_CREATED, 'Document already exists');
-    if (callback) return callback(err);
-    return this.emit('error', err);
+    return util.callbackOrEmit(this, err, callback);
   }
   var op = {create: {type: type, data: data}};
   var source = options && options.source;
@@ -935,8 +925,7 @@ Doc.prototype.del = function(options, callback) {
   }
   if (!this.type) {
     var err = new ShareDBError(ERROR_CODE.ERR_DOC_DOES_NOT_EXIST, 'Document does not exist');
-    if (callback) return callback(err);
-    return this.emit('error', err);
+    return util.callbackOrEmit(this, err, callback);
   }
   var op = {del: true};
   var source = options && options.source;

lib/client/query.js

@@ -149,8 +149,7 @@ Query.prototype._finishResponse = function(err, callback) {
   this.ready = true;
   if (err) {
     this.connection._destroyQuery(this);
-    if (callback) return callback(err);
-    return this.emit('error', err);
+    return util.callbackOrEmit(this, err, callback);
   }
   if (callback) callback(null, this.results, this.extra);
 };

lib/util.js

@@ -84,6 +84,11 @@ exports.callEach = function(callbacks, error) {
   return called;
 };
 
+exports.callbackOrEmit = function(emitter, error, callback) {
+  if (callback) return callback(error);
+  if (error) emitter.emit('error', error);
+};
+
 exports.truthy = function(arg) {
   return !!arg;
 };

test/lib-util.js

@@ -0,0 +1,56 @@
+var EventEmitter = require('../lib/emitter').EventEmitter;
+var util = require('../lib/util');
+var expect = require('chai').expect;
+
+describe('util.callbackOrEmit', function() {
+  var emitter;
+  var error;
+
+  beforeEach(function() {
+    emitter = new EventEmitter();
+    error = new Error('some error');
+  });
+
+  it('calls back synchronously with the error', function() {
+    var calledWith = 'not called';
+    util.callbackOrEmit(emitter, error, function(err) {
+      calledWith = err;
+    });
+    expect(calledWith).to.equal(error);
+  });
+
+  it('calls back synchronously when there is no error', function() {
+    var calledWith = 'not called';
+    util.callbackOrEmit(emitter, null, function(err) {
+      calledWith = err;
+    });
+    expect(calledWith).to.equal(null);
+  });
+
+  it('does not emit when a callback is provided', function() {
+    var emitted = false;
+    emitter.on('error', function() {
+      emitted = true;
+    });
+    util.callbackOrEmit(emitter, error, function() {});
+    expect(emitted).to.equal(false);
+  });
+
+  it('emits the error when there is no callback', function() {
+    var emittedWith = 'not emitted';
+    emitter.on('error', function(err) {
+      emittedWith = err;
+    });
+    util.callbackOrEmit(emitter, error);
+    expect(emittedWith).to.equal(error);
+  });
+
+  it('does nothing when there is no error and no callback', function() {
+    var emitted = false;
+    emitter.on('error', function() {
+      emitted = true;
+    });
+    util.callbackOrEmit(emitter, null);
+    expect(emitted).to.equal(false);
+  });
+});