Skip to content
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

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

Open
viglucci opened this issue Aug 2, 2021 · 6 comments

Comments

@viglucci
Copy link

viglucci commented Aug 2, 2021

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
Copy link
Owner

moll commented Aug 2, 2021

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
Copy link
Author

viglucci commented Aug 2, 2021

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
Copy link
Owner

moll commented Aug 2, 2021

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
Copy link
Owner

moll commented Aug 2, 2021

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
Copy link
Author

viglucci commented Aug 2, 2021

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
Copy link
Author

viglucci commented Aug 2, 2021

@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.

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

No branches or pull requests

2 participants