Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

body-parser and async are breaking CLS #4

Closed
amiram opened this issue Jun 6, 2017 · 12 comments
Closed

body-parser and async are breaking CLS #4

amiram opened this issue Jun 6, 2017 · 12 comments

Comments

@amiram
Copy link
Contributor

amiram commented Jun 6, 2017

To avoid weird behavior with express:

  1. Make sure you require express-http-context in the first row of your app. Some popular pkgs use async which breaks CLS.
  2. If you use body-parser, register it in express before you register express-http-context's middleware. I did after seeing that all post requests lost the context.
@skonves
Copy link
Owner

skonves commented Jun 6, 2017

Thanks for the feedback and thanks for the usage notes! Which versions of node have you been using express-http-context with?

@amiram
Copy link
Contributor Author

amiram commented Jun 7, 2017

node v6.10.2

@amiram
Copy link
Contributor Author

amiram commented Jun 7, 2017

@skonves I think you should add this to the readme. It could save many hours.

skonves added a commit that referenced this issue Jun 12, 2017
@skonves skonves closed this as completed May 22, 2018
@wolever
Copy link

wolever commented May 27, 2018

🎉 I just spent an hour on this one too. Thanks for documenting it!

Any idea why it's happening? Debugging through body-parser I don't see any reason it should happen, since this is the (very normal looking) callback that doesn't get hooked with CLS:

  // read body
  debug('read body')
  getBody(stream, opts, function (error, body) {
    // … this callback …
  })

@skonves
Copy link
Owner

skonves commented May 28, 2018

Thanks for the feedback!

What version of node are you running and what OS are you running on? FWIW, there is a known issue with how node 10 handles async/await (see issue #8).

@shaunc869
Copy link

It looks like the bug linked in the readme has been marked closed, can httpContext be updated to work with node 10 now?

@skonves
Copy link
Owner

skonves commented Dec 4, 2018

Absolutely 👍

Node 10.4.0 and later include the fixed version of V8.

@2Marks
Copy link

2Marks commented Apr 24, 2019

context request lost when doing multiple API calls, but works if it is just one API call

@vg2ls
Copy link

vg2ls commented Jul 31, 2019

any update about that? seems to strange. I am using directly cls-hooked.

@jatinb92
Copy link

jatinb92 commented Jul 3, 2020

Any update on this ... Any one?

@alexby
Copy link

alexby commented Jul 18, 2022

For those who were interested in the fix - the body-parser has a fix in 1.20:

1.20.0 / 2022-04-02
...

  • Prevent loss of async hooks context

https://github.com/expressjs/body-parser/blob/master/HISTORY.md

Maybe it will help you

@devonik
Copy link

devonik commented Aug 3, 2023

This is the way 💯 . I just updated body-parser to latest 1.20.2 and the context is there. Event if the app.use(httpContext.middleware); is defined above app.use(bodyParser).

EDIT I don't get it sometimes it's working sometimes not. Now i use 1.20.0 body-parser and app.use(httpContext.middleware); below app.use(body-parser). On Post it works sometimes on first api call sometimes only after second. But for GET it always work. I have sadly no time to debugging deeper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants