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

End method is missing #3

Open
Ayms opened this issue Oct 9, 2014 · 4 comments
Open

End method is missing #3

Ayms opened this issue Oct 9, 2014 · 4 comments

Comments

@Ayms
Copy link

Ayms commented Oct 9, 2014

The module is calling a .end method that does not exist.

Suggested modification:

Connection.prototype.end = function() {
this.emit('end' or 'close');
};

I don't know the rationale of the check of 'tick' for the 'end' event but if you call destroy for a connection that never happened (ie calling party not responding), this does not work, probably there should be a timeout for this case, please look at https://github.com/Ayms/torrent-live/blob/master/freerider.js (seek for 'utp') and maybe the 'close' event should be let to the user and the internal 'close' event should be a '_close' event or something like this --> this.once('_close', function() {... this.emit('close',xx)...}

@mafintosh I put it in my PR todo list if you want.

@mafintosh
Copy link
Owner

The end method should be provided by require('stream').Duplex which we inherit from. Do you have failing test case?

@Ayms
Copy link
Author

Ayms commented Oct 9, 2014

Right, and yes, in the following example the 'flush' event is never fired in sendFin:

socket.on('connect',function() {
  socket.destroy();
});

@mafintosh
Copy link
Owner

socket.destroy() should emit 'close` and force close the connection. It probably doesn't do that right now though so we should fix that

@Ayms
Copy link
Author

Ayms commented Oct 13, 2014

Do you mean that this.end should be replaced by this.emit('close') in destroy method (what I am doing currently) or that something should be combined in sendFin to emit close when flush is not to be emitted?

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