Delay 'connect' events by an extra tick

moll / node-mitm

Intercept and mock outgoing Node.js network TCP connections and HTTP requests for testing. Intercepts and gives you a Net.Socket, Http.IncomingMessage and Http.ServerResponse to test and respond with. Super useful when testing code that hits remote servers.

Other

636 stars 48 forks source link

Delay 'connect' events by an extra tick #25

Closed mwoc closed 8 years ago

mwoc commented 8 years ago

I was unable to mock out http using mitm for the stripe-node repository: https://github.com/stripe/stripe-node/blob/master/lib/StripeResource.js#L238

The 'socket' event is fired after the 'connect' event on the socket, which caused that code to fail. As the 'socket' event on req happens in the next tick, the 'connect' event on the socket needs to be delayed by an extra tick, which is what this pull request achieves.

papandreou commented 8 years ago

@moll Ping?

moll commented 8 years ago

Thank you again, @mwoc, for the help! I ended up going for a slightly different design:

First refactored tests to not depend on a precise number of ticks.
Emitted connect with setTimeout and 0 to have it run after all ticks. This shouldn't have any performance effect, but should now consistently trigger connect after all intermediate events.

Also added secureConnect triggering which was missing before.

Could you, @mwoc and @papandreou, give it a try from this repo to confirm it all works as expected? I'll then cut a release. Thank you!

gustavnikolaj commented 8 years ago

@moll The unexpected-mitm module used a fork of mitm that included @mwoc's fixes. I reverted that change and linked in the master branch of mitm, and had the test suite running with no errors, so it looks good to go from my perspective at least :-) :+1:

moll commented 8 years ago

Great! Thanks again everyone! I threw v1.2.1 out there with all we've discussed. ;-)