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
637 stars 48 forks source link

Feature request: ability to reject a connection to simulate `ECONNREFUSED` #72

Open viglucci opened 2 years ago

viglucci commented 2 years ago

First off -- thank you for the great package! I've already found it very useful after just a short usage period.

For my use case, I am looking to be able to simulate a connection failure, such as ECONNREFUSED. I don't believe this functionality currently exists.

What I would recommend is a reject() method be added to socket, which is passed to the connect even callback.

mitm.on("connect", function (socket) {
  socket.reject();
});

I'll submit a PR soon adding this functionality for your review.

moll commented 2 years ago

Hey,

I'm glad you find Mitm.js useful. Hmm, is there anything in the current architecture that's making throwing any connection-time error hard? I suppose there are more connecting-related errors besides ECONNREFUSED that would be useful — DNS resolving failure for example. I'd rather make throwing them all possible than hard-code specific ones in.

I suppose connecting errors have to be emitted as error in the next tick on the client socket, rather than emitting connect. Currently Mitm.js always emits connect... Having tested this, do you know the exact order of what gets thrown when in the case of ECONNREFUSED and possibly DNS failures?

Thanks!

viglucci commented 2 years ago

is there anything in the current architecture that's making throwing any connection-time error hard?

For my specific use case, I need to refuse/error the connection before the connect event is fired on the socket.

...do you know the exact order of what gets thrown when in the case of ECONNREFUSED and possibly DNS failures?

Unfortunately, I'm not overly familiar with the lookup event and the errors that it could throw, however, ECONNREFUSED seems pretty straightforward.

My original approach could potentially be made more robust if the reject method accepted the error that you wished to be thrown, rather than hard coding it. Additionally, maybe it should be named close(error: Error): void rather than reject?

moll commented 2 years ago

I'm actually thinking whether it'd be possible to permit you to just throw whatever error you want from the mitm.on("connect") handler, and pass that on via the error event on the socket. This way Mitm.js would be agnostic to all errors and no API additions would be necessary.

moll commented 2 years ago

Just a reminder, the first argument given to mitm.on("connect") is the client Socket (from Node's net module). I'm very hesitant to add methods to it. Socket.prototype.close for example is already a thing.

viglucci commented 2 years ago

I'm actually thinking whether it'd be possible to permit you to just throw whatever error you want from the mitm.on("connect") handler, and pass that on via the error event on the socket.

Since the connect event on mitm seems to be handled sync, that should be possible with a simple try/catch. I'll explore that in an additional commit, especially given your second point about Socket.prototype.close.

viglucci commented 2 years ago

@moll https://github.com/moll/node-mitm/pull/73/files now reflects your latest thoughts/feedback on throwing inside of the connect handler. LMK if you have any thoughts. Thanks.