Skip to content

Commit

Permalink
Drop continuation-local-storage, use cls-hooked
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
josieusa authored and bajtos committed Jan 3, 2017
1 parent 7cef481 commit 996a49f
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 21 deletions.
2 changes: 0 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
sudo: false
language: node_js
node_js:
- "0.10"
- "0.12"
- "4"
- "6"
37 changes: 34 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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

Expand Down
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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",
Expand Down
15 changes: 2 additions & 13 deletions server/current-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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() {
Expand Down
31 changes: 31 additions & 0 deletions test/main.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

'use strict';

var async = require('async');
var LoopBackContext = require('..');
var Domain = require('domain');
var EventEmitter = require('events').EventEmitter;
Expand Down Expand Up @@ -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);
});
});
});
1 change: 1 addition & 0 deletions test/mocha.opts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-r cls-hooked

0 comments on commit 996a49f

Please sign in to comment.