Skip to content

Conversation

@kamilmysliwiec
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

AsyncHooksModule which uses async_hooks module capabilities (share async state, for example, between HTTP requests). Blocked due to memory leaks - async_hooks is still in experimental stage.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@kamilmysliwiec kamilmysliwiec changed the title feature() add async_hooks module (async storage) feature() add async_hooks module (async storage) [BLOCKED] Dec 29, 2018
@coveralls
Copy link

Pull Request Test Coverage Report for Build 1344

  • 32 of 56 (57.14%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.6%) to 92.516%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/common/hooks/async-hooks-middleware.ts 3 5 60.0%
packages/common/hooks/async-hooks-helper.ts 3 6 50.0%
packages/common/hooks/async-hooks-storage.ts 6 11 54.55%
packages/common/hooks/async-context.ts 13 27 48.15%
Totals Coverage Status
Change from base Build 1336: -0.6%
Covered Lines: 3118
Relevant Lines: 3288

💛 - Coveralls

@coveralls
Copy link

coveralls commented Dec 29, 2018

Pull Request Test Coverage Report for Build 1346

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 93.068%

Totals Coverage Status
Change from base Build 1336: 0.002%
Covered Lines: 3087
Relevant Lines: 3233

💛 - Coveralls

@elderapo
Copy link

It's been 2 years, async_hooks are still experimental but maybe we could get this feature behind the experimental flag?

@apricote
Copy link

apricote commented Jul 4, 2020

We adopted the code from this PR in @narando/nest-xray to implement async storage. So far we have not encountered any bugs.

https://github.com/narando/nest-xray/tree/master/lib/async-hooks


Instead of using async_hooks directly, we should explore using AsyncLocalStorage to provide this functionality. It was released in Node.js 14 and is part of the experimental async_hooks module.

@gelito
Copy link
Contributor

gelito commented Sep 12, 2020

Why not try to reactivate it? Two years later the technology has been improved.

@gms1
Copy link

gms1 commented Aug 24, 2021

Instead of using async_hooks directly, we should explore using AsyncLocalStorage to provide this functionality. It was released in Node.js 14 and is part of the experimental async_hooks module.

it has even been back ported to Node.js 12 ( the latest EOL release :-) )

@Papooch
Copy link
Contributor

Papooch commented Oct 13, 2021

I was also looking for this functionality, so I spent some time researching and testing and the result is this package: https://github.com/Papooch/nestjs-cls
It would be great if Nest provided some 'request hooks' so we could wrap the execution context of the whole request somehow instead of relying on a middleware or an 'enhancer' (though I don't know if that's even possible)

@mwebb33
Copy link

mwebb33 commented Oct 14, 2021

Any updates? This issue is basically 3 years old, and it seems like the node AsyncLocalStorage API has matured quite a bit in that time. I think it would make a huge impact on NestJS- specifically as a mechanism to pass req/ctx while avoiding the performance penalty of request scoped providers.

@kamilmysliwiec
Copy link
Member Author

specifically as a mechanism to pass req/ctx while avoiding the performance penalty of request scoped providers.

I wouldn't be so sure that using AsyncLocalStorage atm would be better (performance-wise) than properly designed request-scoped providers.

Any updates? This issue is basically 3 years old

According to the official Node.js docs, async_hooks are still experimental https://nodejs.org/api/async_hooks.html and this PR won't get merged till that changes.

@nestjs nestjs locked and limited conversation to collaborators Oct 15, 2021
@kamilmysliwiec
Copy link
Member Author

async_hooks are still experimental but if anyone wants to use this feature now, check out this library https://github.com/Papooch/nestjs-cls

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants