5.x: startRequest event called multiple times

[whataboutpereira]

I'm noticing a weird behaviour with startRequest event being called multiple times for a single request.

I'm using an event handler based on RecordHarAttributes and I was going to count the number of requests in addition to timings. When I incremented requests count in startRequest() I noticed the number is very high and it turned out startRequest() is called 9 times with the same URI/Request object. Is that by design?

Also, is completeReceivingResponse() called for failed requests as well? If yes, it might be the best place to increment requests count. For now I'm incrementing it in startSendingRequest().

function fetch(Request $request, HttpClient $client, LocalSemaphore $semaphore, HttpRequestTimings $timings): string
{
    $lock = $semaphore->acquire();
    $request->addEventListener($timings);
    $response = $client->request($request);

    if ($response->getStatus() !== 200) {
        throw new \Exception($response->getStatus() . " " . $response->getReason());
    }

    $body = $response->getBody()->buffer();

    $lock->release();

    return $body;
}

[kelunik]

Hmmm, every Amp\Http\Client\InterceptedHttpClient calls startRequest currently, this doesn't seem right. Do you have other feedback regarding the event listener API? I have built an initial set of events for 4.x without knowing what's really needed.

[whataboutpereira]

Hmmm, every Amp\Http\Client\InterceptedHttpClient calls startRequest currently, this doesn't seem right. Do you have other feedback regarding the event listener API? I have built an initial set of events for 4.x without knowing what's really needed.

Unfortunately not, the script has been working nicely now for several months.

[kelunik]

Idea: Save list of interceptors (either in properties or a request attribute) and then detect requests passing through the same interceptor twice and error in that case. Also, if the list of interceptors is empty, call startRequest, so it's not called repeatedly.

[kelunik]

See also https://square.github.io/okhttp/features/events/.

[kelunik]

Initial draft of phases:

<?php

namespace Amp\Http\Client;

enum RequestState
{
    case Initial;
    case Queued;
    case DnsResolution;
    case Connecting;
    case TlsHandshake;
    case SendingHeaders;
    case SendingBody;
    case Waiting;
    case ReceivingHeaders;
    case ReceivingBody;
    case Complete;
    case Failed;
}

[kelunik]

This will be fixed in #334.

[kelunik]

This now landed in 5.0.0 Beta 14.

[whataboutpereira]

Checking out the new events using an event listener based on LogHttpArchive.php, but I see requestEnd is not firing. Will keep digging tomorrow.

[kelunik]

@whataboutpereira Please open a new issue then.