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

Usage in prod - service logic should not rely on success of ns.run #45

Open
orishofman opened this issue Jul 10, 2019 · 4 comments
Open

Comments

@orishofman
Copy link

Hi,
I want to use cls-hooked in my services that run in a prod env.
From what I saw in the Namespace.prototype.run source code, the callback I set to run when ns.run is finished will run only if the code in the try statement will not throw any errors.
This means that if by any chance there will be an error before my callback will be called, then my code won't run.

I am using this package in a way I believe many others are using it(I am running my service with express) - calling "ns.run(() =>{ next() })" in a middleware in the start of the chain and then use the namespace in later parts of the chain. I am using it to add a transaction id to all my logs.
If the call to ns.run would fail for any reason, I would of course prefer my code to run anyway and to just not have the transaction id in the logs of this specific session.
From what I understand from the code I read, if ns.run will fail then my code won't run for the specific api request.

I guess that if there will be a "ns.safeRun" or something of the kind that will act the same as ns.run but in the end will run my callback either way then this would solve my issue.
I believe that this is something that other people who will use this package will want.

Or, maybe there is a way to do what I wrote here and I just didn't figure it out.
What do you say?

Thank you!

@ecdeveloper
Copy link

Good point. Probably, an interim solution would be to wrap the ns.run() into a try/catch block, and invoke your next() inside the catch() block?

@orishofman
Copy link
Author

@ecdeveloper Does ns.run() has any async code running when it is invoked?
If not, I guess wrapping it with a try/catch block should work :)

@LucasHang
Copy link

Did it work @orishofman ?

@orishofman
Copy link
Author

@LucasHang we did not wrap it in try/catch. We have prod services running this way for 3 years with no issues. If you can use newer versions of nodejs I would probably recommend using https://nodejs.org/api/async_context.html#class-asynclocalstorage as it is already in a stable state for the newer versions and is a built in feature

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

3 participants