uperl / AnyEvent-WebSocket-Client

WebSocket client for AnyEvent
9 stars 10 forks source link

Add prepare callback to connect() #34

Open laurent-aml opened 6 years ago

laurent-aml commented 6 years ago

connect() is missing the prepare callback, similarly to what AnyEvent::Socket::tcp_connect provides. I suggest:

@@ -70,7 +70,7 @@ has max_payload_size => (

 sub connect
 {
-  my($self, $uri) = @_;
+  my($self, $uri, $preparecb) = @_;
   unless(ref $uri)
   {
     require URI;
@@ -167,7 +167,10 @@ sub connect
         undef $done;
       }
     });
-  }, sub { $self->timeout };
+  }, sub {
+    my $timeout = $preparecb->(@_) if defined $preparecb;
+    return $timeout || $self->timeout;
+  };
   $done;
 }
plicease commented 6 years ago

Looks reasonable. This ( + documentation + a test ) would make a good PR.

plicease commented 5 years ago

I am not sure but I think I'd like the prepare callback to be a attribute on the client object.

plicease commented 5 years ago

Although I am not sure, I did try implementing it as an optional last argument on the connect function here:

https://github.com/plicease/AnyEvent-WebSocket-Client/commit/6fd90140ca4b8f675c98743cb2ab08d052fffd07

arguably this is more like the AnyEvent::Socket interface, especially now that we are going to have optional host and port overrides.

laurent-aml commented 4 years ago

I was about to revisit this. Thanks for your work on this. Your patch looks good, except that it does not fallback to the default timeout if the prepare callback returns 0/undef (which is the behavior of AnyEvent::Socket).

Suggestion:

@@ -83,7 +83,10 @@ has env_proxy => (

 sub connect
 {
-  my($self, $uri, $host, $port) = @_;
+  my $self = shift;
+  my $uri = shift;
+  my $prepare_cb = pop if ref $_[-1] eq 'CODE';
+  my($host, $port) = @_;
   unless(ref $uri)
   {
     require URI;
@@ -184,7 +187,10 @@ sub connect
         undef $done;
       }
     });
-  }, sub { $self->timeout });
+  }, sub {
+    my $timeout = $prepare_cb->(@_) if defined $prepare_cb;
+    return $timeout || $self->timeout;
+  });
   $done;
 }