Skip to content

Asynchronous interface #16

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

Closed
6uliver opened this issue Nov 18, 2019 · 2 comments · Fixed by #27
Closed

Asynchronous interface #16

6uliver opened this issue Nov 18, 2019 · 2 comments · Fixed by #27

Comments

@6uliver
Copy link

6uliver commented Nov 18, 2019

First of all, your package is great, thank you!

My issue is a question/suggestion: The official Node.JS documentation says that the crypto.randomBytes method can block the execution:

The crypto.randomBytes() method will not complete until there is sufficient entropy available. This should normally never take longer than a few milliseconds. The only time when generating the random bytes may conceivably block for a longer period of time is right after boot, when the whole system is still low on entropy.

You can find a question whether should we use sync or async:
nodejs/help#457

I don't have any performance issue yet, but I would like to use this package in production. In my use case I expect to have a lot of call to this function so it would be better if it did not block the execution. I know that a few milliseconds as the documentation says is not a lot for a single call but in a server environment a lot of few milliseconds call can block other tasks.

I suggest that we should have an interface (eg. cryptoRandomStringAsync) which does similar things but returns a Promise. What do you think?

If you like the idea but don't have time for this I can implement it and send a PR.

Thanks

@sindresorhus
Copy link
Owner

sindresorhus commented Nov 18, 2019

👍

PR welcome. Should be a cryptoRandomString.async method. Don't forget to update index.d.ts and the readme.

@sindresorhus
Copy link
Owner

If anyone wants to work on this, see the previous attempt and feedback in #18.

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