From 3bae15f5d890ac7a90a4a9765adb43819ddd7796 Mon Sep 17 00:00:00 2001 From: josieusa Date: Mon, 5 Sep 2016 15:27:57 +0200 Subject: [PATCH] Try cls-hooked-interceptor, add test, edit README --- README.md | 24 +++++++++++++++ package.json | 4 ++- test/main.test.js | 76 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 57b4c65..14314ed 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,30 @@ Current context for LoopBack applications, based on cls-hooked. +## Main changes in `cls-hooked` branch + +The work-in-progress `cls-hooked` branch uses `cls-hooked` NPM package, which in turn uses `async-hook`. The latter requires Node > 4.5 as specified in its own `package.json`. **This change alone doesn't solve, for now, any problem with `loopback-context`, especially issues related to lost context (for example see [loopback #1495](https://github.com/strongloop/loopback/issues/1495)), as far as the writer knows**. But it uses Node `AsyncWrap`, which may have some pros in the long term. + +However, as a work-in-progress, this branch also includes the experimental package `cls-hooked-interceptor` (**not ready for production**), which should give useful warnings when there is the risk that the context would get lost at runtime. **Neither this solves any problem with `loopback-context`**. It's just *a work-in-progress that will eventually help with debugging and mantaining apps experiencing such issues, common especially in legacy apps*. + +Therefore, **what is the solution to issues with lost context?** For now, you should implement your own solution. For example, the writer uses the `npm shrinkwrap` solution mentioned in [loopback #1495](https://github.com/strongloop/loopback/issues/1495), but even this could be useless for you, in case you get a broken install (this is unpredictable and hard to notice, and this package wants to change that). + +To test this branch, just run the tests with `npm test`. (You will also see a warning from `cls-hooked-interceptor`, in addition to the test output; this is the desired behavior). + +If you want to see all the infos and warnings that `cls-hooked-interceptor` sends to the `debug` package, declare the environment variable `DEBUG=cls-hooked-interceptor` when running the tests. + +OS X example: + +`DEBUG=cls-hooked-interceptor npm test` + +By default, the test makes the context get lost. If you want to test the opposite case when the package does *not* warn, because the context is *not* lost, just declare the environment variable `KEEP_CONTEXT=true` when running the tests (*this variable has no effect outside of tests!*). + +OS X example: + +`KEEP_CONTEXT=true npm test` + +**TODO**: try to refactor the tests using the package `lose-cls-context`. + ## Usage 1) Add `per-request-context` middleware to your diff --git a/package.json b/package.json index 6b3c018..dda51df 100644 --- a/package.json +++ b/package.json @@ -20,11 +20,13 @@ }, "license": "MIT", "dependencies": { - "continuation-local-storage": "^3.1.7", + "cls-hooked-interceptor": "github:josieusa/cls-hooked-interceptor", + "continuation-local-storage": "3.1.7", "depd": "^1.1.0", "optional": "^0.1.3" }, "devDependencies": { + "async": "1.5.2", "chai": "^3.5.0", "dirty-chai": "^1.2.2", "eslint": "^2.13.1", diff --git a/test/main.test.js b/test/main.test.js index 11e8f30..4508279 100644 --- a/test/main.test.js +++ b/test/main.test.js @@ -5,11 +5,18 @@ 'use strict'; +// If you wish to test when NOT losing context, +// set the environment variable KEEP_CONTEXT to the string 'true'. +// See README for more info. +var keepContext = process.env.KEEP_CONTEXT === 'true'; +if (keepContext) { + require('cls-hooked'); +} var ClsContext = require('..'); +var loopback = require('loopback'); var Domain = require('domain'); var EventEmitter = require('events').EventEmitter; var expect = require('./helpers/expect'); -var loopback = require('loopback'); var request = require('supertest'); describe('LoopBack Context', function() { @@ -99,3 +106,70 @@ describe('LoopBack Context', function() { }); }); }); + +describe('cls-hooked-interceptor', function() { + it('does ' + + (keepContext ? 'not ' : '') + + 'warn when async waterfall does ' + + (keepContext ? 'not ' : '') + + 'lose context', + function(done) { + // If you wish to test if NOT losing context, + // set loseContext to false at the top of the file. + + // Begin intercepting async calls + var warnedDangerousAsyncImport = false; + require('cls-hooked-interceptor')(function(warning) { + if (warning.code !== 'EASYNCCODE') { + console.warn(warning.message); + } + if (warning.code === 'ECLSAFTERINCOMPATIBLEMODULE') { + warnedDangerousAsyncImport = true; + } + }); + // trick cls-hooked-interceptor + if (keepContext) { + require('cls-hooked'); + } + // Make cls-hooked-interceptor emit warnings + // ASYNC VERSION MATTERS! 1.5.2 is required in order for this test to pass. + var async = require('async'); + ClsContext.runInContext(function() { + // function 1 which pulls context + var fn1 = function(cb) { + var ctx = ClsContext.getCurrentContext(); + expect(ctx).is.an('object'); + ctx.set('test-key', 'test-value'); + cb(); + }; + // function 2 which pulls context + var fn2 = function(cb) { + var ctx = ClsContext.getCurrentContext(); + if (keepContext) { + expect(ctx).is.an('object'); + } else { + expect(ctx).is.not.an('object'); + } + var testValue = ctx && ctx.get('test-key', 'test-value'); + cb(null, testValue); + }; + // Trigger async waterfall callbacks + var asyncFn = function() { + async.waterfall([ + fn1, + fn2, + ], function(err, testValue) { + if (keepContext) { + expect(testValue).to.equal('test-value'); + expect(warnedDangerousAsyncImport).to.be.false(); + } else { + expect(testValue).to.not.equal('test-value'); + expect(warnedDangerousAsyncImport).to.be.true(); + } + done(); + }); + }; + asyncFn(); + }); + }); +});