cls-hooked: context is lost on 'end' event of http request stream

Here is a sample snippet:

const http = require('http');
var createNamespace = require('cls-hooked').createNamespace;
var session = createNamespace('session-ns');


const server = http.createServer(function (request, response) {
    session.run(function (ctx) {
        session.set('key', 1)
        read(request, response, function (req, res) {
            process._rawDebug(`DONE - key: ${session.get('key')}`)
            res.end(`key: ${session.get('key')}\r\n`)
        })
    })
})

server.listen(8080)

function read(req, res, done) {
    let body = [];
    req
        .on('data', function (chunk) {
            body.push(chunk)
        })
        .on('end', function () {
            body = Buffer.concat(body).toString()
            process._rawDebug(`End - key: ${session.get('key')}`)
            done(req, res)
        });
}

As you can see I have created a http server and read the body by a stream. run the snippet then use curl. curl http://localhost:8080
console output:

End - key: 1  
DONE - key: 1

for simple GET requests the context is correct, but when you send data in the body the context is lost. curl http://localhost:8080 -X POST -d aaaaaaaaaa console output:

End - key: undefined   
DONE - key: undefined

This issues is also related to this skonves/express-http-context#4. The problem is not the body-parser package, I have tracked it down and found that it is caused by the same issue as here, the callback of ‘end’ event looses context.

Node runtime: reproducible on all v8.14.0, v10.4.0, v10.3.0, and v10.14.1
OS: Ubuntu 18.04
cls-hooked: 4.2.2

About this issue

Original URL
State: open
Created 6 years ago
Reactions: 4
Comments: 25 (6 by maintainers)

Most upvoted comments

Hi @Jeff-Lewis, any progress on this?

kjavaherpour on Mar 20, 2020

@Strate @PoslavskySV I decided to use my own naive implementation.

const asyncHooks = require('async_hooks')

let contexts = {}

asyncHooks.createHook({
  init (asyncId, type, triggerAsyncId, resource) {
    if (contexts[triggerAsyncId] !== undefined) {
      contexts[asyncId] = contexts[triggerAsyncId]
    } else {
      contexts[asyncId] = {}
    }
  },

  destroy (asyncId) {
    delete contexts[asyncId]
  }
}).enable()


function run (callback) {
  let eid = asyncHooks.executionAsyncId()
  contexts[eid] = {}
  callback()
}

function set (k, v) {
  let eid = asyncHooks.executionAsyncId()
  let ctx = contexts[eid]
  if (ctx !== undefined) {
    ctx[k] = v
  }
}

function get (k) {
  let eid = asyncHooks.executionAsyncId()
  let ctx = contexts[eid]
  return ctx !== undefined ? ctx[k] : undefined
}

module.exports = {get, set, run, contexts}

It doesn’t have the above issue, and I use it with async/await on node 8.14 without any problems. Also, I use these packages on my project: express, body-parse, Sequelize, oauth2-server, cookie-parser, and request-promise-native. Though I had some issues with nested Promises at first but when I rewrote them with async/await it worked fine.

aidinrs on Feb 6, 2019

@AiDirex - I’m debugging this and it appears to be an issue isolated the the end event on a POST. The data event does propagate context correctly. I’m exploring adding a work around for the related node issue #21078.

Jeff-Lewis on Feb 17, 2019

@kibertoad We use ALS of node 12.21.0, but the problem still exists.

heby281 on Apr 8, 2021

Big thanks

On Mon, 11 Jan 2021 at 20:53, Igor Savin notifications@github.com wrote:

@alexandruluca https://github.com/alexandruluca There are no good reasons to keep using cls-hooked on node 12.18.0, async-local-storage is fully supported there and is a recommended and actually properly supported alternative. You can check this bridge lib to see the ALS equivalents to cls-hooked:

https://github.com/kibertoad/asynchronous-local-storage/blob/master/lib/cls-fallback.ts

https://github.com/kibertoad/asynchronous-local-storage/blob/master/lib/als.ts

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Jeff-Lewis/cls-hooked/issues/36#issuecomment-758152161, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSAHNWAIR5PU3HHNPKVQDTSZNCLXANCNFSM4GJOPIYQ .

alexandruluca on Jan 11, 2021

I’m hoping some changes to async_hook, specifically https://github.com/nodejs/node/pull/27581, would resolve this. Needs to be tested…

Jeff-Lewis on Dec 4, 2019

Essentially the behavior of async_hooks changed in 8.10.0 which is described in nodejs #21078. Options we have are to either change behavior back to what it used to be and what is documented by node, or stick with existing behavior.

Either way, the best option for cls-hooked is to accommodate the behavior inconsistencies across node versions and handle it appropriately.

Jeff-Lewis on Feb 16, 2019

@AiDirex did you find solution for your problem?

Strate on Feb 6, 2019