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

Exceptions on failed listen are not propagated #13

Open
Altai-man opened this issue Apr 10, 2020 · 2 comments
Open

Exceptions on failed listen are not propagated #13

Altai-man opened this issue Apr 10, 2020 · 2 comments

Comments

@Altai-man
Copy link
Member

<perlmaros> i do have another question regarding Cro::HTTP::Server ...
<perlmaros> when the specified port is already in use, $server.start just prints 'address already in use', but there is no exception and also the return value does not indicate a failure
<perlmaros> how does one detect such a failure?
<sena_kun> perlmaros, it seems rakudo throws an AdHoc exception, but I suspect Cro::TCP::Listener does not propagate it. I'll file a ticket now...

Possibly #10 is related.

@Altai-man
Copy link
Member Author

Altai-man commented Apr 10, 2020

A sample to reproduce, perlmaros++:

use Cro::HTTP::Log::File;
use Cro::HTTP::Server;
use Cro::HTTP::Router;
 
sub routes() {
    route {
        get -> '/' {
            content 'text/plain', 'hello world';
        }
    }
}
 
my $server = Cro::HTTP::Server.new(
    http => <1.1>,
    host => '0.0.0.0',
    port => 22, # already in use and also privileged
    application => routes(),
    after => [
        Cro::HTTP::Log::File.new(logs => $*OUT, errors => $*ERR)
    ]
);
 
$server.start;
say "Should not be called";

@robertlemmen
Copy link

I am suffering from this problem as well, but also have two somewhat related issues:

  • if one sets the port to 0, IO::Socket::Async correctly picks a free one, but it is hard to figure out what that chosen port is through the layers. This would be very helpful for testing where it's hard to predict which ports are free, especially when running multiple tests in parallel.
  • There is a bit of a race, where after a Cro server is started it isn't actually fully up yet, due to the asyn nature of the IO::Socket::Async.listen call. again, this is annoying in testing if you start firing requests against it right away.

so I butchered the Listener a bit, now it does all three but is really ugly :) So not saying it is a solution, but perhaps interesting:

diff --git a/lib/Cro/TCP.pm6 b/lib/Cro/TCP.pm6
index fd7eeaf..aea2c99 100644
--- a/lib/Cro/TCP.pm6
+++ b/lib/Cro/TCP.pm6
@@ -63,11 +63,27 @@ class Cro::TCP::Listener does Cro::Source {
     method produces() { Cro::TCP::ServerConnection }
 
     method incoming() {
-        supply {
-            whenever IO::Socket::Async.listen($!host, $!port) -> $socket {
-                emit Cro::TCP::ServerConnection.new(:$socket);
-            }
+        my $done-promise = Promise.new;
+        my $exception = Nil;
+        my $supplier = Supplier.new;
+        start {
+            my $listener = IO::Socket::Async.listen($!host, $!port).tap(-> $socket {
+                    $supplier.emit(Cro::TCP::ServerConnection.new(:$socket));
+                },
+                quit => -> $e {
+                    $exception = $e;
+                    $done-promise.keep;
+                });
+            $listener.socket-port.then({
+                $!port = $listener.socket-port.result;
+                $done-promise.keep;
+            });
+        }
+        await $done-promise;
+        if defined $exception {
+            die $exception;
         }
+        return $supplier.Supply;
     }
 }
 

Interestingly this causes a failure in the cro test suite which I believe to be actually bogus and more a result of the same problems: port clashes combined with the failure to propagate/report the "address already in use"...

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