Unable to cause Exceptions in a ResponseStack

[avehlies]

I ran into a situation at work where I wanted to write a test around retrying an API if there are connection issues (specifically a GuzzleHttp ConnectException). The application code would try to make a call to the API 3 times before giving up.

What I ended up trying was:

class TimeoutResponse extends Response
{
    public function getBody(RequestInfo $request)
    {
        throw new ConnectException(...);
    }
}

and then

$this->mockServer->setResponseOfPath('/v1/get-data',
    new ResponseStack(
        new TimeoutResponse('timeoutBody'),
        new TimeoutResponse('timeoutBody'),
        new Response('goodBody')
    )
);

I was never getting the 'goodBody' response back and I narrowed it down to how ResponseStack is handling ->next().

InternalServer.php::sendResponse:

echo $response->getBody($this->request);

if( $response instanceof MultiResponseInterface ) {
    $response->next();
    self::storeResponse($this->tmpPath, $response);
}

When the exception is thrown, the $response->next() is never called.

I believe a valid fix for this would be (in PHP5.4 since we can't use finally):

try {
    echo $response->getBody($this->request);
    if( $response instanceof MultiResponseInterface ) {
        $response->next();
        self::storeResponse($this->tmpPath, $response);
    }
} catch (Exception $exception) {
    if( $response instanceof MultiResponseInterface ) {
        $response->next();
        self::storeResponse($this->tmpPath, $response);
    }
    throw $exception;
}

I made the changes and tried to write an integration test, but was having trouble accessing my TimeoutResponse and TimeoutException test classes not being recognized because of namespace issues. Before mucking around with it more, I was going to see if this seems like a reasonable feature and/or approach?

[avehlies]

This was inspired by not being able to get a working solution for #38 on my end. As I think more about it, this probably isn't the best approach because the server shouldn't really be handling an exception, just not responding. (Though I will say, being able to throw an Exception is a lot quicker than waiting for a timeout in a test.)

[donatj]

Sorry, this completely slipped my radar until now. This is functioning as designed.

You should simulate the response instead of throwing an actual exception.

Your solution above would mask actual exceptions caused by test failures.