refactor(cls): remove cls support (#10817)

BREAKING CHANGE: All CLS support has been removed. Migration guide: If you do not use CLS, you can ignore this. Check all your usage of the `.transaction` method and make you sure explicitly pass the `transaction` object for each subsequent query.
sequelize · Jun 12, 2019 · c28e034 · c28e034
1 parent a1c6f16
commit c28e034
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 428 deletions.
diff --git a/docs/transactions.md b/docs/transactions.md
@@ -52,68 +52,16 @@ return sequelize.transaction(t => {
 });
 ```
 
-### Automatically pass transactions to all queries
-
-In the examples above, the transaction is still manually passed, by passing `{ transaction: t }` as the second argument. To automatically pass the transaction to all queries you must install the [continuation local storage](https://github.com/othiym23/node-continuation-local-storage) (CLS) module and instantiate a namespace in your own code:
-
-```js
-const cls = require('continuation-local-storage'),
-    namespace = cls.createNamespace('my-very-own-namespace');
-```
-
-To enable CLS you must tell sequelize which namespace to use by using a static method of the sequelize constructor:
-
-```js
-const Sequelize = require('sequelize');
-Sequelize.useCLS(namespace);
-
-new Sequelize(....);
-```
-
-Notice, that the `useCLS()` method is on the *constructor*, not on an instance of sequelize. This means that all instances will share the same namespace, and that CLS is all-or-nothing - you cannot enable it only for some instances.
-
-CLS works like a thread-local storage for callbacks. What this means in practice is that different callback chains can access local variables by using the CLS namespace. When CLS is enabled sequelize will set the `transaction` property on the namespace when a new transaction is created. Since variables set within a callback chain are private to that chain several concurrent transactions can exist at the same time:
-
-```js
-sequelize.transaction((t1) => {
-  namespace.get('transaction') === t1; // true
-});
-
-sequelize.transaction((t2) => {
-  namespace.get('transaction') === t2; // true
-});
-```
-
-In most case you won't need to access `namespace.get('transaction')` directly, since all queries will automatically look for a transaction on the namespace:
-
-```js
-sequelize.transaction((t1) => {
-  // With CLS enabled, the user will be created inside the transaction
-  return User.create({ name: 'Alice' });
-});
-```
-
-After you've used `Sequelize.useCLS()` all promises returned from sequelize will be patched to maintain CLS context. CLS is a complicated subject - more details in the docs for [cls-bluebird](https://www.npmjs.com/package/cls-bluebird), the patch used to make bluebird promises work with CLS.
-
-**Note:** _[CLS only supports async/await, at the moment, when using cls-hooked package](https://github.com/othiym23/node-continuation-local-storage/issues/98#issuecomment-323503807). Although, [cls-hooked](https://github.com/Jeff-Lewis/cls-hooked/blob/master/README.md) relies on *experimental API* [async_hooks](https://github.com/nodejs/node/blob/master/doc/api/async_hooks.md)_
-
-## Concurrent/Partial transactions
-
-You can have concurrent transactions within a sequence of queries or have some of them excluded from any transactions. Use the `{transaction: }` option to control which transaction a query belong to:
-
-**Warning:** _SQLite does not support more than one transaction at the same time._
-
-### Without CLS enabled
+### Example
 
 ```js
 sequelize.transaction((t1) => {
   return sequelize.transaction((t2) => {
-    // With CLS enable, queries here will by default use t2
     // Pass in the `transaction` option to define/alter the transaction they belong to.
     return Promise.all([
         User.create({ name: 'Bob' }, { transaction: null }),
         User.create({ name: 'Mallory' }, { transaction: t1 }),
-        User.create({ name: 'John' }) // this would default to t2
+        User.create({ name: 'John' }) // No transaction
     ]);
   });
 });

diff --git a/docs/upgrade-to-v6.md b/docs/upgrade-to-v6.md
@@ -24,3 +24,36 @@ If you have relied on accessing sequelize operators via `Symbol.for('gt')` etc.
 `Model.build` has been acting as proxy for `bulkBuild` and `new Model` for a while.
 
 Use `Model.bulkBuild` or `new Model` instead.
+
+### Removal of CLS
+
+CLS allowed implicit passing of the `transaction` property to query options inside of a transaction.
+This feature was removed for 3 reasons.
+
+- It required hooking the promise implementation which is not sustainable for the future of sequelize.
+- It's generally unsafe due to it's implicit nature.
+- It wasn't always reliable when mixed promise implementations were used.
+
+Check all your usage of the `.transaction` method and make sure to explicitly pass the `transaction` object for each subsequent query.
+
+#### Example
+
+```js
+db.transaction(async transaction => {
+  const mdl = await myModel.findByPk(1);
+  await mdl.update({
+    a: 1;
+  });
+});
+```
+
+should now be:
+
+```js
+db.transaction(async transaction => {
+  const mdl = await myModel.findByPk(1, { transaction });
+  await mdl.update({
+    a: 1;
+  }, { transaction });
+});
+```
diff --git a/lib/model.js b/lib/model.js
@@ -2286,13 +2286,6 @@ class Model {
       }
     }
 
-    if (options.transaction === undefined && this.sequelize.constructor._cls) {
-      const t = this.sequelize.constructor._cls.get('transaction');
-      if (t) {
-        options.transaction = t;
-      }
-    }
-
     const internalTransaction = !options.transaction;
     let values;
     let transaction;

diff --git a/lib/sequelize.js b/lib/sequelize.js
@@ -3,7 +3,6 @@
 const url = require('url');
 const path = require('path');
 const retry = require('retry-as-promised');
-const clsBluebird = require('cls-bluebird');
 const _ = require('lodash');
 
 const Utils = require('./utils');
@@ -614,9 +613,6 @@ class Sequelize {
       const retryOptions = Object.assign({}, this.options.retry, options.retry || {});
 
       return Promise.resolve(retry(() => Promise.try(() => {
-        if (options.transaction === undefined && Sequelize._cls) {
-          options.transaction = Sequelize._cls.get('transaction');
-        }
         if (options.transaction && options.transaction.finished) {
           const error = new Error(`${options.transaction.finished} has been called on this transaction(${options.transaction.id}), you can no longer use it. (The rejected query is attached as the 'sql' property of this error)`);
           error.sql = sql;
@@ -1040,8 +1036,6 @@ class Sequelize {
   /**
    * Start a transaction. When using transactions, you should pass the transaction in the options argument in order for the query to happen under that transaction @see {@link Transaction}
    *
-   * If you have [CLS](https://github.com/othiym23/node-continuation-local-storage) enabled, the transaction will automatically be passed to any query that runs within the callback
-   *
    * @example
    * sequelize.transaction().then(transaction => {
    *   return User.findOne(..., {transaction})
@@ -1050,27 +1044,6 @@ class Sequelize {
    *     .catch(() => transaction.rollback());
    * })
    *
-   * @example <caption>A syntax for automatically committing or rolling back based on the promise chain resolution is also supported</caption>
-   *
-   * sequelize.transaction(transaction => { // Note that we use a callback rather than a promise.then()
-   *   return User.findOne(..., {transaction})
-   *     .then(user => user.update(..., {transaction}))
-   * }).then(() => {
-   *   // Committed
-   * }).catch(err => {
-   *   // Rolled back
-   *   console.error(err);
-   * });
-   *
-   * @example <caption>To enable CLS, add it do your project, create a namespace and set it on the sequelize constructor:</caption>
-   *
-   * const cls = require('continuation-local-storage');
-   * const ns = cls.createNamespace('....');
-   * const Sequelize = require('sequelize');
-   * Sequelize.useCLS(ns);
-   *
-   * // Note, that CLS is enabled for all sequelize instances, and all instances will share the same namespace
-   *
    * @param {Object}   [options] Transaction options
    * @param {string}   [options.type='DEFERRED'] See `Sequelize.Transaction.TYPES` for possible options. Sqlite only.
    * @param {string}   [options.isolationLevel] See `Sequelize.Transaction.ISOLATION_LEVELS` for possible options
@@ -1090,57 +1063,16 @@ class Sequelize {
     if (!autoCallback) return transaction.prepareEnvironment(false).return(transaction);
 
     // autoCallback provided
-    return Sequelize._clsRun(() => {
-      return transaction.prepareEnvironment()
-        .then(() => autoCallback(transaction))
-        .tap(() => transaction.commit())
-        .catch(err => {
-          // Rollback transaction if not already finished (commit, rollback, etc)
-          // and reject with original error (ignore any error in rollback)
-          return Promise.try(() => {
-            if (!transaction.finished) return transaction.rollback().catch(() => {});
-          }).throw(err);
-        });
-    });
-  }
-
-  /**
-   * Use CLS with Sequelize.
-   * CLS namespace provided is stored as `Sequelize._cls`
-   * and bluebird Promise is patched to use the namespace, using `cls-bluebird` module.
-   *
-   * @param {Object} ns CLS namespace
-   * @returns {Object} Sequelize constructor
-   */
-  static useCLS(ns) {
-    // check `ns` is valid CLS namespace
-    if (!ns || typeof ns !== 'object' || typeof ns.bind !== 'function' || typeof ns.run !== 'function') throw new Error('Must provide CLS namespace');
-
-    // save namespace as `Sequelize._cls`
-    this._cls = ns;
-
-    // patch bluebird to bind all promise callbacks to CLS namespace
-    clsBluebird(ns, Promise);
-
-    // return Sequelize for chaining
-    return this;
-  }
-
-  /**
-   * Run function in CLS context.
-   * If no CLS context in use, just runs the function normally
-   *
-   * @private
-   * @param {Function} fn Function to run
-   * @returns {*} Return value of function
-   */
-  static _clsRun(fn) {
-    const ns = Sequelize._cls;
-    if (!ns) return fn();
-
-    let res;
-    ns.run(context => res = fn(context));
-    return res;
+    return transaction.prepareEnvironment()
+      .then(() => autoCallback(transaction))
+      .tap(() => transaction.commit())
+      .catch(err => {
+        // Rollback transaction if not already finished (commit, rollback, etc)
+        // and reject with original error (ignore any error in rollback)
+        return Promise.try(() => {
+          if (!transaction.finished) return transaction.rollback().catch(() => {});
+        }).throw(err);
+      });
   }
 
   log(...args) {

diff --git a/lib/transaction.js b/lib/transaction.js
@@ -58,8 +58,6 @@ class Transaction {
       return Promise.reject(new Error(`Transaction cannot be committed because it has been finished with state: ${this.finished}`));
     }
 
-    this._clearCls();
-
     return this
       .sequelize
       .getQueryInterface()
@@ -91,8 +89,6 @@ class Transaction {
       return Promise.reject(new Error('Transaction cannot be rolled back because it never started'));
     }
 
-    this._clearCls();
-
     return this
       .sequelize
       .getQueryInterface()
@@ -105,13 +101,9 @@ class Transaction {
       });
   }
 
-  prepareEnvironment(useCLS) {
+  prepareEnvironment() {
     let connectionPromise;
 
-    if (useCLS === undefined) {
-      useCLS = true;
-    }
-
     if (this.parent) {
       connectionPromise = Promise.resolve(this.parent.connection);
     } else {
@@ -134,12 +126,6 @@ class Transaction {
           .catch(setupErr => this.rollback().finally(() => {
             throw setupErr;
           }));
-      })
-      .tap(() => {
-        if (useCLS && this.sequelize.constructor._cls) {
-          this.sequelize.constructor._cls.set('transaction', this);
-        }
-        return null;
       });
   }
 
@@ -172,16 +158,6 @@ class Transaction {
     return res;
   }
 
-  _clearCls() {
-    const cls = this.sequelize.constructor._cls;
-
-    if (cls) {
-      if (cls.get('transaction') === this) {
-        cls.set('transaction', null);
-      }
-    }
-  }
-
   /**
    * A hook that is run after a transaction is committed
    *

diff --git a/package.json b/package.json
@@ -31,7 +31,6 @@
   "license": "MIT",
   "dependencies": {
     "bluebird": "^3.5.0",
-    "cls-bluebird": "^2.1.0",
     "debug": "^4.1.1",
     "dottie": "^2.0.0",
     "inflection": "1.12.0",
@@ -57,7 +56,6 @@
     "chai-as-promised": "^7.x",
     "chai-datetime": "^1.x",
     "chai-spies": "^1.x",
-    "continuation-local-storage": "^3.x",
     "cross-env": "^5.2.0",
     "env-cmd": "^8.0.0",
     "esdoc": "^1.1.0",