From 996a49f7da205ca6e4f380d7125daa7b75081d40 Mon Sep 17 00:00:00 2001 From: josieusa Date: Fri, 9 Sep 2016 10:32:57 +0200 Subject: [PATCH] Drop continuation-local-storage, use cls-hooked Rework the module to use the new cls-hooked module (which uses AsyncWrap available since Node v4.5) instead of old continuation-local-storage (based on very unreliable async-listener). --- .travis.yml | 2 -- README.md | 37 ++++++++++++++++++++++++++++++++++--- package.json | 7 ++++--- server/current-context.js | 15 ++------------- test/main.test.js | 31 +++++++++++++++++++++++++++++++ test/mocha.opts | 1 + 6 files changed, 72 insertions(+), 21 deletions(-) create mode 100644 test/mocha.opts diff --git a/.travis.yml b/.travis.yml index ecf6d68..1d0ad98 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,5 @@ sudo: false language: node_js node_js: - - "0.10" - - "0.12" - "4" - "6" diff --git a/README.md b/README.md index 1b8bf4c..9c38bea 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,38 @@ # loopback-context Current context for LoopBack applications, based on -node-continuation-local-storage. +cls-hooked. + +## USAGE WARNING + +**Only if you use this package, it's recommended to not run your app using `slc run` or `node .`** + +Run using (recommended): + +`node -r cls-hooked .` + +This uses the `-r` option in order to require `cls-hooked` before your app (see warnings below for more info). + +If you wish to use `strong-supervisor`, you would need to pass node options to `slc run`, which currently has issues, according to [strong-supervisor#56](https://github.com/strongloop/strong-supervisor/issues/56). + +A less reliable, less recommended way, which instead should be compatible with `strong-supervisor`, would be to add this Javascript line at the first line of your app, and no later than that (**the order of require statements matters**): + +`require('cls-hooked')` + +Warning: to rely on the order of `require` statements is error-prone. + +## INSTALL WARNING + +**Only if you use this package, do NOT install your app using `npm install`.** + +Install using: + +``` +npm config set engine-strict true +npm install +``` + +This keeps you from using Node < v4.5. ## WARNING @@ -13,8 +44,8 @@ As a result, loopback-context does not work in many situations, as can be seen from issues reported in LoopBack's [issue tracker](https://github.com/strongloop/loopback/issues?utf8=%E2%9C%93&q=is%3Aissue%20getCurrentcontext). -If you are running on Node v6, you can try the new alternative -[cls-hooked](https://github.com/Jeff-Lewis/cls-hooked). +The new alternative +[cls-hooked](https://github.com/Jeff-Lewis/cls-hooked) is known to possibly inherit these problems if it's not imported before everything else, that's why you are required to follow the advice above if using this. ## Usage diff --git a/package.json b/package.json index 7e9a7d2..35b97b1 100644 --- a/package.json +++ b/package.json @@ -1,9 +1,9 @@ { "name": "loopback-context", "version": "3.0.0-alpha.1", - "description": "Current context for LoopBack applications, based on node-continuation-local-storage", + "description": "Current context for LoopBack applications, based on cls-hooked", "engines": { - "node": ">=4" + "node": "^4.5 || ^5.10 || ^6.0" }, "keywords": [ "StrongLoop", @@ -23,9 +23,10 @@ }, "license": "MIT", "dependencies": { - "continuation-local-storage": "^3.1.7" + "cls-hooked": "^4.0.1" }, "devDependencies": { + "async": "1.5.2", "chai": "^3.5.0", "dirty-chai": "^1.2.2", "eslint": "^2.13.1", diff --git a/server/current-context.js b/server/current-context.js index 36bec16..a63ee67 100644 --- a/server/current-context.js +++ b/server/current-context.js @@ -5,20 +5,9 @@ 'use strict'; +var cls = require('cls-hooked'); var domain = require('domain'); -// Require CLS only when using the current context feature. -// As soon as this require is done, all the instrumentation/patching -// of async-listener is fired which is not ideal. -// -// Some users observed stack overflows due to promise instrumentation -// and other people have seen similar things: -// https://github.com/othiym23/async-listener/issues/57 -// It all goes away when instrumentation is disabled. -var cls = function() { - return require('continuation-local-storage'); -}; - var LoopBackContext = module.exports; /** @@ -85,7 +74,7 @@ LoopBackContext.createContext = function(scopeName) { process.context = process.context || {}; var ns = process.context[scopeName]; if (!ns) { - ns = cls().createNamespace(scopeName); + ns = cls.createNamespace(scopeName); process.context[scopeName] = ns; // Set up LoopBackContext.getCurrentContext() LoopBackContext.getCurrentContext = function() { diff --git a/test/main.test.js b/test/main.test.js index faf5534..29a4d16 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -5,6 +5,7 @@ 'use strict'; +var async = require('async'); var LoopBackContext = require('..'); var Domain = require('domain'); var EventEmitter = require('events').EventEmitter; @@ -98,4 +99,34 @@ describe('LoopBack Context', function() { }); }); }); + + // Credits for the original idea for this test case to @marlonkjoseph + // Original source of the POC gist of the idea: + // https://gist.github.com/marlonkjoseph/f42f3c71f746896a0d4b7279a34ea753 + // Heavily edited by others + it('keeps context when using waterfall() from async 1.5.2', + function(done) { + expect(require('async/package.json').version).to.equal('1.5.2'); + LoopBackContext.runInContext(function() { + // Trigger async waterfall callbacks + async.waterfall([ + function pushToContext(next) { + var ctx = LoopBackContext.getCurrentContext(); + expect(ctx).is.an('object'); + ctx.set('test-key', 'test-value'); + next(); + }, + function pullFromContext(next) { + var ctx = LoopBackContext.getCurrentContext(); + expect(ctx).is.an('object'); + var testValue = ctx && ctx.get('test-key', 'test-value'); + next(null, testValue); + }, + function verify(testValue, next) { + expect(testValue).to.equal('test-value'); + next(); + }, + ], done); + }); + }); }); diff --git a/test/mocha.opts b/test/mocha.opts new file mode 100644 index 0000000..eb83dbe --- /dev/null +++ b/test/mocha.opts @@ -0,0 +1 @@ +-r cls-hooked