othiym23 / node-continuation-local-storage

implementation of https://github.com/joyent/node/issues/5243
BSD 2-Clause "Simplified" License
1.13k stars 207 forks source link

context is lost in async waterfall in hapi #142

Closed elgreatly closed 4 years ago

elgreatly commented 5 years ago

i'm creating a package and use continuation-local-storage to keep the request in the context in hapi.js

1- i created the function to start context

let createNamespace = require('continuation-local-storage').createNamespace;
const ns = createNamespace('test');

bindCurrentNamespace(req, reply) {
    ns.bindEmitter(req);

    ns.run(() => {
        reply()
    });
}

2- another functions to set and get value

 setCurrentContext(req, reply) {
    ns.set('test', 'test');
    reply();
}

 getCurrentContext() {
    return ns.get('test');
 }

when i'm using these functions in async waterfall i lost the context in the second callback

async.waterfall(
    [
      function(cb) {
        getCurrentContext(); // return 'test'
        getTest(cb);
      },
      function(testData, cb1) {
       getCurrentContext(); // return undefined
       getTest2(testData, (err, data) => {
          callback(err, data);
        });
      }
    ],
    callback
  );

is there any way to fix this issue

Qard commented 5 years ago

I'd recommend trying cls-hooked as hapi uses async/await extensively which this module does not support well.

elgreatly commented 5 years ago

@Qard thanks for your reply, i tried to use cls-hooked with async.waterfall but the same issue

is there any solution for this issue?

Qard commented 5 years ago

You probably need to bind the functions given to the waterfall function to maintain the continuation.

elgreatly commented 5 years ago

@Qard you mean bind which functions?

elgreatly commented 5 years ago

i bind the functions like that but still the same issue

async.waterfall(
    [
      function(cb) {
        getCurrentContext(); // return 'test'
        getTest(cb);
      }.bind(this),
      function(testData, cb1) {
       getCurrentContext(); // return undefined
       getTest2(testData, (err, data) => {
          callback(err, data);
        });
      }.bind(this)
    ],
    callback
  );
elgreatly commented 5 years ago

even if i changed async.waterfall to callbacks function still the same issue

getCurrentContext(); // return 'test'
getTest(function(err, testData) {
    getCurrentContext(); // return undefined
       getTest2(testData, (err, data) => {
          callback(err, data);
        });
});
Qard commented 5 years ago

Not Function.bind(...), I meant namespace.bind(function). Sometimes you have to do that to ensure a function restores the continuation context when it is run. This is often used in cases with indirect asynchrony such as connection pooling.

elgreatly commented 5 years ago

@Qard thanks for your reply

i tried to do that but with the same issue

i tried to console the namespace to check if it keep the same namespace or not i checked it like that

let ns = getNamespace();
async.waterfall(
    [
      function(cb) {
        console.log(ns);
        getCurrentContext(); // return 'test'
        getTest(cb);
      },
      function(testData, cb1) {
       console.log(ns);
       getCurrentContext(); // return undefined
       getTest2(testData, (err, data) => {
          callback(err, data);
        });
      }
    ],
    callback
  );

what i found is:

  1. first log i got the ns like that
    Namespace {
    name: 'request',
    active: { _ns_name: 'request', id: 1096, 'test': 'test' },
    _set: [ null ],
    id: -1,
    _contexts:
    Map {
     1120 => { _ns_name: 'request', id: 1096, 'test': 'test' },
     1121 => { _ns_name: 'request', id: 1096, 'test': 'test' }
    }
    }
  2. second log i get the ns like that
    Namespace {
    name: 'request',
    active: null,
    _set: [],
    id: -1,
    _contexts: Map {},
    _indent: 0 
    }

why the namespace reset again in the second callback and i didn't do anything to reset it?

elgreatly commented 5 years ago

i found namespace.exit(context); in line 412 in context.js file this line exit the context before the callback Done

but when i removed it all requests overwrite each other

@Qard do you have any idea how to fix it?

Qard commented 5 years ago

I'm not really sure where binding is needed, but probably either the task functions in the array, or the callbacks used to store the result and jump to the next task in the waterfall. My guess is its more likely the callbacks.

elgreatly commented 5 years ago

the problem is the namespace already exit before i go to the next function then when i try to bind from the namespace, bind on empty one

Qard commented 5 years ago

That means something needs binding before that point. If you don't have a valid context in the callback then you need to bind the task functions. If you don't even have a valid context at the point where you are creating the task functions and calling async.waterfall(...) then the problem lies elsewhere and you probably need to bind something else. I know hapi makes extensive use of async/await, so it's just plain not going to work with this module, but with cls-hooked and possibly a few binds in the right places, it should be possible to get it working.

elgreatly commented 4 years ago

when i bind ns on the main callback functions of async.waterfall(...) it works fine

let ns = getNamespace();
async.waterfall(
    [
      ns.bind(function(cb) {
        getCurrentContext(); // return 'test'
        getTest(cb);
      }),
      ns.bind(function(testData, cb1) {
       getCurrentContext(); // return 'test'
       getTest2(testData, (err, data) => {
          callback(err, data);
        });
      })
    ],
    callback
  );

@Qard thanks for your help