Skip to content

Conversation

@TheAthlete
Copy link

Implemented adding parameters for AnyEvent::Handle to the establish method

This functionality is required to add various AnyEvent::Handler parameters and handlers, for example:

use AnyEvent::Socket qw(tcp_server);
use AnyEvent::WebSocket::Server;
 
my $server = AnyEvent::WebSocket::Server->new();
 
my $tcp_server;
$tcp_server = tcp_server undef, 8080, sub {
    my ($fh) = @_;
    $server->establish($fh, {
      on_eof => sub {
        my $handle = shift;
        # some code
      },
    })->cb(sub {
        my $connection = eval { shift->recv };
        if($@) {
            warn "Invalid connection request: $@\n";
            close($fh);
            return;
        }
    });
};

@debug-ito
Copy link
Owner

Thanks for making the pull-request.

I don't mind adding capability to customize the AnyEvent::Handle object, but that feature would be experimental forever. Changing configuration of the AnyEvent::Handle object changes its behavior drastically, and I cannot guarantee AnyEvent::WebSocket::Server still works correctly. In addition, some attributes (e.g. on_read and on_eof) of AnyEvent::Handle are overwritten by AnyEvent::WebSocket::Server and AnyEvent::WebSocket::Connection modules. Is it ok for you?

If you describe a little more detail on what you want to do, maybe we can come up with a more reliable way to achieve that.

@s0me0ne-unkn0wn
Copy link

@debug-ito What we're trying to achieve here is to take the control over the situation when connection closes unexpectedly during websocket handshake. Without the patch, we had sporadic crashes inside the handshake code which we couldn't handle (I assume it happened when clients were closing the connection before handshake procedure was complete, although we didn't investigate it in deep). After the patch provided by @TheAthlete those crashes are gone, that's why I believe it's useful. Any advice on how to make it in a right way is appreciated!

@debug-ito
Copy link
Owner

Thanks for explaining your problem.

If something bad happens during the handshake process, it should be reported as error from the AnyEvent::CondVar returned by establish method. If the current implementation fails to do that, it's a bug and should be fixed so that the CondVar reports the error.

Did the CondVar throw any exception when $condvar->recv is called? What error message did you get?

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

Successfully merging this pull request may close these issues.

3 participants