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

Server: Detect when response has been sent #396

Closed
CupOfTea696 opened this issue Nov 1, 2020 · 17 comments
Closed

Server: Detect when response has been sent #396

CupOfTea696 opened this issue Nov 1, 2020 · 17 comments
Labels

Comments

@CupOfTea696
Copy link

Currently, I can't seem to find a way to check when a response has been sent. Once a response is returned via the Handler Callback, there is no way of knowing what happened to it, or check when it has been sent completely.

Use case: Checking that all requests have been handled and sent to the client before shutting down the server.

Personally, I am trying to close the server once a request has been handled, much like in this StackOverflow question, however, the suggested solution here (closing the socket connection) does not seem to work properly, as it only closes the socket connection after another request has been received.

@clue clue added the question label Nov 3, 2020
@clue
Copy link
Member

clue commented Nov 3, 2020

@CupOfTea696 Thanks for your question, this is an interesting one :-)

The EventLoop keeps running as long as is has something to do. This means that if you stop listening for any incoming connections and stop reading from the last connection, it will automatically stop. You can also explicitly stop() the EventLoop, but that's usually not recommended.

Here's quick gist to show you the concept:

$loop = React\EventLoop\Factory::create();

$socket = new React\Socket\Server(8080, $loop);

$server = new React\Http\Server($loop, function (Psr\Http\Message\ServerRequestInterface $request) use ($socket) {
    $socket->close();

    return new React\Http\Message\Response(
        200,
        array(
            'Content-Type' => 'text/plain'
        ),
        "Hello World!\n"
    );
});


$server->listen($socket);

$loop->run();

I hope this helps 👍

@clue clue closed this as completed Nov 3, 2020
@CupOfTea696
Copy link
Author

CupOfTea696 commented Nov 3, 2020

@clue The problem is, if I do that, the loop keeps going until another connection is received. A simplified version of my code goes like this:

$loop = \React\EventLoop\Factory::create();
$socket = new \React\Socket\Server('8080', $loop);
$server = new \React\Http\Server($loop, function (\Psr\Http\Message\ServerRequestInterface $serverRequest) use ($socket) {
    if ($serverRequest->getUri()->getPath() === '/test') {
        echo "Closing...\n";
        $socket->close();
    } else {
        echo "Continue...\n";
    }

    return new \React\Http\Message\Response(
        200,
        array(
            'Content-Type' => 'text/plain'
        ),
        'Hello ' . $serverRequest->getUri()->getPath()
    );
});

$server->listen($socket);
$loop->run();
echo "\nAfter loop run\n";

And then when I hit the Server in my browser:

GET http://127.0.0.1:8080/foo     →   OK "Hello /foo"
GET http://127.0.0.1:8080/test    →   OK "Hello /test"
GET http://127.0.0.1:8080/bar     →   OK "Hello /bar"
GET http://127.0.0.1:8080/test    →   Error: connect ECONNREFUSED

And the command line output:

> php test.php
Continue...
Closing...
Continue...

After loop run

The /bar request shouldn't be handled anymore, but it is.

@clue
Copy link
Member

clue commented Nov 3, 2020

I can not reproduce the problem you're seeing with the above example I've posted. Can you reproduce this?

me@me-in:~/workspace/reactphp-http$ php examples/foo-stop.php &
[1] 75429
me@me-in:~/workspace/reactphp-http$ curl -v http://localhost:8080
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.68.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: text/plain
< Server: ReactPHP/1
< Date: Tue, 03 Nov 2020 13:17:51 GMT
< Content-Length: 13
< Connection: close
< 
Hello World!
* Closing connection 0
me@me-in:~/workspace/reactphp-http$ fg
-bash: fg: job has terminated
[1]+  Done                    php examples/foo-stop.php

@CupOfTea696
Copy link
Author

@clue this is weird… when I use curl, I can't replicate this, but when using my browser, it does accept and handle two requests.

@clue
Copy link
Member

clue commented Nov 7, 2020

The React\Socket\Server accepts any number of connections. Once "enough" data is received on any of the connections, it will invoke the HTTP request handler which in turn closes the listening socket, so it no longer accepts any new connections. This does not affect any pending connections that may or may not still sending an HTTP request.

I'm not sure what use-case you have for closing the listening socket, but in this case you may want to also keep track of all open connections and explicitly close any pending connections?

As an alternative, you can also limit the number of open connections (what happens if the first connection doesn't actually send an HTTP request?) or you may explicitly stop() the loop (connections will be left dangling until they're explicitly closed / the program terminates).

@CupOfTea696
Copy link
Author

CupOfTea696 commented Nov 8, 2020

@clue My use case is that I am opening an external web-based login from a command-line application that redirects to the react server in order for the cli app to receive the auth token. Once I successfully receive the auth token, I want to close the server so that when the user returns to the command line app, it's not still blocking the process.

As far as I can see I have no way to access the open connections?

@clue
Copy link
Member

clue commented Nov 8, 2020

Interesting use-case, thanks for sharing!

Here's the gist to get the list of all currently connected clients:

$clients = array();
$server->on('connection', function (React\Socket\ConnectionInterface $client) use (&$clients) {
    $clients []= $client;
    $client->on('close', function() use ($client, &$clients) {
        unset($clients[array_search($client, $clients)]);
    });
});

You should be able to combine this with the above example, i.e. close the listening socket and any pending connections. Note that calling close() on the connection that triggered the request would immediately discard any buffers, so that you would no longer receive the response body. You may want to use end() method instead in this case.

@CupOfTea696
Copy link
Author

@clue Hmm, seems like closing (or ending) the connections results in no response being sent unless they are wrapped inside a $loop->futureTick(). If the response is either a Deferred or streaming response, it doesn't work at all.

$loop = React\EventLoop\Factory::create();

$socket = new React\Socket\Server(8080, $loop);

$clients = [];
$socket->on('connection', function (React\Socket\ConnectionInterface $client) use (&$clients) {
    $hash = md5(spl_object_hash($client));
    $clients[$hash] = $client;
    $client->on('close', function() use ($hash, &$clients) {
        unset($clients[$hash]);
    });
});

$server = new React\Http\Server($loop, function (Psr\Http\Message\ServerRequestInterface $request) use ($loop, $socket, &$clients) {
    echo "request\n";

    $loop->futureTick(function () use ($socket, $clients) {
        foreach ($clients as $hash => $client) {
            echo "end connection $hash\n";
            $client->end();
        }

        echo "close socket\n";
        $socket->close();
    });

    echo "respond\n";

    return new React\Http\Message\Response(
        200,
        array(
            'Content-Type' => 'text/plain'
        ),
        "Hello World!\n"
    );
});


$server->listen($socket);

$loop->run();

In the command line that will output something like this:

request
respond
end connection ac91a3c5fd5aed36f9475f886b813511
end connection 59f01e12fef0ef15616f0ea5eaf77ade
close socket

@CupOfTea696
Copy link
Author

Upon further inspection, I now understand what's actually happening behind the scenes and why the loop doesn't stop after that first request. When browsing to the URL in chrome, it instantly opens 2 connections. The 1st connection is the actual HTTP request, the second one is just opened to speed up subsequent requests. When hitting the URL again in chrome, it opens a 3rd connection and keeps it open, but sends the HTTP request to that second connection is opened initially. Any subsequent request will open another connection, and send the HTTP request over on the previous connection.

So the reason the $loop doesn't stop after it's handled that 1st HTTP request, is because of that second connection that is being kept open by chrome. I've got a working solution now that doesn't close any connections that are being handled, I'm not sure if this is the best way to go about it, though.

$loop = React\EventLoop\Factory::create();

$socket = new React\Socket\Server(8080, $loop);

$connections = [];
$socket->on('connection', function (React\Socket\ConnectionInterface $client) use (&$connections) {
    $hash = md5(spl_object_hash($client));
    $connections[$hash] = [
        'client' => $client,
        'used' => false,
    ];

    echo "new connection $hash\n";

    $client->once('data', function() use ($hash, &$connections) {
        echo "request on $hash\n";
        $connections[$hash]['used'] = true;
    });

    $client->on('close', function() use ($hash, &$connections) {
        echo "closed $hash\n";
        unset($connections[$hash]);
    });
});

$server = new React\Http\Server($loop, function (Psr\Http\Message\ServerRequestInterface $request) use ($loop, $socket, &$connections) {
    $loop->futureTick(function () use ($socket, &$connections) {
        foreach ($connections as $hash => $connection) {
            if (! $connection['used']) {
                echo "close $hash\n";
                $connection['client']->close();
            }
        }

        echo "close socket\n";
        $socket->close();
    });

    return new React\Http\Message\Response(
        200,
        array(
            'Content-Type' => 'text/plain'
        ),
        "Hello World!\n"
    );
});


$server->listen($socket);

$loop->run();

And then the cli output is something like this:

new connection 05d526f67532f81c88c7e45cf6fc9a9e
new connection 1c0f1386bac0ec98d1737bc5cd93b61a
request on 05d526f67532f81c88c7e45cf6fc9a9e
close 1c0f1386bac0ec98d1737bc5cd93b61a
closed 1c0f1386bac0ec98d1737bc5cd93b61a
close socket
closed 05d526f67532f81c88c7e45cf6fc9a9e

@clue
Copy link
Member

clue commented Jan 20, 2021

@CupOfTea696 Thanks for reporting back, this is indeed an interesting problem you're facing!

Additionally, note that each incoming connection can potentially send some data before the first connections emits "enough" data to be considered a valid HTTP request. In particular, this means that a second client connection would not be closed in your last snippet if it already sent some data. This is trivial to reproduce with telnet/nc, but it may or may not be a problem depending on your particular use case. Invoking end() or close() on each connection after a (small) timeout is probably a good idea.

@CupOfTea696
Copy link
Author

@clue Is there no way to know which connection a request originated from? I feel like that would make this a whole lot less complicated. It also seems that using a timeout has the potential of closing the connection from the initial request before the response has been sent, which would be undesirable.

Actually, I'm not sure what you're saying is true. In my snippet, it will first close any connections that haven't received data. It then closes the socket server, meaning no new requests will come in. Then the connection that created the initial request will close once it's done. Any other connections that are open and had data written to them will continue receiving data (I believe) and either trigger the HTTP server's request handler or send an error response. Both scenarios will close the connection, unless I'm missing something here, which is entirely possible because I'm not sure I fully understand the internal workings of ReactPHP.

@gitneko
Copy link

gitneko commented Jan 21, 2021

@clue Is there no way to know which connection a request originated from? I feel like that would make this a whole lot less complicated. It also seems that using a timeout has the potential of closing the connection from the initial request before the response has been sent, which would be undesirable.

Actually there is! Just not the easy way.

The HTTP request instance contains the source ip and port (remote ip and port) and you need to listen on the socket server for new connections, where each connection instance also has a source ip and port. So you simply need to match these two together.

Basically

$this->socket->on('connection', function (ConnectionInterface $conn) {
    $source = $conn->getRemoteAddress();
    $pos = \strpos($source, '://');
    
    $source = \substr($source, ($pos === false ? 0 : ($pos + 3)));
    $this->connections[$source] = $conn;
    
    $conn->once('close', function () use ($source) {
        unset($this->connections[$source]);
    });
});

and for each request

$params = $request->getServerParams();
$source = $params['REMOTE_ADDR'].':'.$params['REMOTE_PORT'];
$conn = $this->connections[$source];

@CupOfTea696
Copy link
Author

@gitneko surely multiple connections cant come from the same ip and port?

@gitneko
Copy link

gitneko commented Jan 22, 2021

@CupOfTea696 I've tested this before (some months ago when I implemented it) and each connection used a different source port (from the same ip).

@clue
Copy link
Member

clue commented Jan 25, 2021

@gitneko Excellent suggestion, thanks for chiming in! 👍

TCP/IP uses a tuple of source IP, source port, destination IP and destination port. In other words, this is always guaranteed to be unique for the lifetime of a connection, but may be reused at a later point in time 👍

@CupOfTea696
Copy link
Author

@gitneko @clue thanks, I'll try that out, that's a great suggestion!

@quazardous
Copy link

quazardous commented Jul 26, 2021

@gitneko hi,

I've linked the connection and request doing some tweaks with the on('headers') event

#414 (comment)

I needed to cancel the connection timeout at the request level. I've used spl_hash on the conn object to register it and keep track of it and create a context.

I was wondering if the general design could be more vertical. Now it's kind of horizontal:

There is a socket server dealing with connections, there is a http server dealing with requests. The two worlds are linked at some points behind the scene with a central loop and events on different layers.

Well maybe we could have some kind of "upgrade" mechanism. Connection "evolves" into a Request and Request "evolves" into a Respons". Each evolution could add attributes that follows the meta token (connection/request/response wrapper).

Each transition/evolution could be followed by some event/listener mechanism on the meta token wrapper.

Well it's out loud thinking 😄

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

No branches or pull requests

4 participants