-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Force the DNS module to invoke callbacks asynchronously #1164
Comments
Could you write a code example? |
be careful. wont fix. |
any reason why? |
i dont want to change API right now - also good for fast lookups. |
But the change would only be noticeable to anyone who is relying on the callback to be invoked in the same tick right? And if anyone is relying on that, I guess it is bad practice anyways. What do you think? |
I think this fix does not change API because there is no guarantee that a callback is invoked immediately. |
@koichik Like ;) |
@koichik ok |
@ry Thanks! |
Frankly, I'm -1 to this because the API doesn't state that it will be invoked asynchronously either. The safe coder would prepare for the callback being invoked in either situation. |
Yes. It is the essence of the problem.
I agree. So this fix does not impact to him/her. And, this also fixes #1202. I think that this helps many Noders. |
Yes, agreed that people should not rely on either behaviour, but it's simple enough to ensure async callbacks. |
Please review. |
@ry, @bnoordhuis, @koichik Thanks! |
Please review 1aed45e. |
The current DNS module may call the callback function directly, which means that any code that assumes that the callback will be invoked only after the complete function executes is broken. Please could it be changed to invoke all callbacks (not just in the DNS module) by passing them to nextTick() if the calling function is the public API and it wants to invoke the user supplied callback.
The text was updated successfully, but these errors were encountered: