-
-
Notifications
You must be signed in to change notification settings - Fork 752
simulate socket timeout #282
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
Conversation
By passing in a socket timeout to the nock request, it will emit a timeout event on the request if setTimeout has been called with a lower value. It does not actually wait for the specified period of time.
|
Note: the emit error that is failing the tests is one thing that needs to be rethought - the core libraries don't actually emit an error, but they destroy the socket which will result in an error being emitted. Would appreciate your thoughts on how to handle that appropriately. |
lib/socket.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is an internal function, perhaps it should be prefixed with _?
|
Other than this, it's looking good! |
|
ha, I missed your comment on the failing test. Let me check this first. |
|
Regarding the abortion, I think it shouldn't emit an error. |
Aborting a request will not emit an error in node, however it will destroy the socket being used, which then leads to an error being emitted when it is used. Model this behaviour more accurately here.
|
Thanks for looking over it!
I agree. I've added some changes which instead of emitting an error on abort, it will do it if you attempt to write, end or respond to the request instead - this should match how a normal request behaves if a socket is destroyed (as is the case in the code above). Does that make sense? |
|
Yes it does! |
|
Hi @pgte - I've added tests and documentation. Also, writing the tests made it clear that calling it Is there anything else you need? |
|
@brettporter Thanks! landed on v1.2.0 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you! |
By passing in a socket timeout to the nock request, it will emit a timeout event on the request if setTimeout has been called with a lower value. It does not actually wait for the specified period of time.
I've checked that this works with the base http library's request.setTimeout and socket.setTimeout, both with a "once" function and an "on timeout" handler.
I wanted to first check if this approach makes sense, and if so I can flesh it out with tests and documentation.
This is related to #164, though for one specific case.