fix HTTPRequestException when body is null

[shokme]

sometimes the body response is null and even if null looks like authorized it will throw this error:

Error : Wrong parameters for MeiliSearch\Exceptions\HTTPRequestException([string $message [, long $code [, Throwable $previous = NULL]]])
 .../meilisearch-php/src/Exceptions/HTTPRequestException.php:21

by returning $response->getReasonPhrase() the error should not happen again

....
* @return string Reason phrase; must return an empty string if none present.
*/
public function getReasonPhrase();

In my test case:
using a wrong url -> http://localhost instead of http://localhost:7700 will return

MeiliSearch\Exceptions\HTTPRequestException : Not Found
.../meilisearch/meilisearch-php/src/Http/Client.php:183

I guess this will also solve meilisearch/meilisearch-laravel-scout#37

[curquiza]

Hello @shokme!
Thanks for this PR, it seems great 🙂

A question to be sure I understand your fix:

This example

<?php

require_once __DIR__ . '/vendor/autoload.php';
use MeiliSearch\Client;
$client = new MeiliSearch\Client('http://localhost:770/', 'masterKey');
$index = $client->version();

currently fails (v0.14.1) with this error:

$ php test.php
Fatal error: Uncaught Buzz\Exception\NetworkException: file_get_contents(http://localhost//version): failed to open stream: Connection refused in /Users/curquiza/Documents/tests/php2/vendor/kriswallsmith/buzz/lib/Client/FileGetContents.php:26
Stack trace:
#0 /Users/curquiza/Documents/tests/php2/vendor/meilisearch/meilisearch-php/src/Http/Client.php(165): Buzz\Client\FileGetContents->sendRequest(Object(Nyholm\Psr7\Request))
#1 /Users/curquiza/Documents/tests/php2/vendor/meilisearch/meilisearch-php/src/Http/Client.php(81): MeiliSearch\Http\Client->execute(Object(Nyholm\Psr7\Request))
#2 /Users/curquiza/Documents/tests/php2/vendor/meilisearch/meilisearch-php/src/Contracts/Endpoint.php(21): MeiliSearch\Http\Client->get('/version')
#3 /Users/curquiza/Documents/tests/php2/vendor/meilisearch/meilisearch-php/src/Delegates/HandlesSystem.php(29): MeiliSearch\Contracts\Endpoint->show()
#4 /Users/curquiza/Documents/tests/php2/test.php(11): MeiliSearch\Client->version()
#5 {main}
  thrown in /Users/curquiza/Documents/tests/php2/vendor/kriswallsmith/buzz/lib/Client/FileGetContents.php on line 26

With your fix, and the same example, it would return:

MeiliSearch\Exceptions\HTTPRequestException : Not Found
.../meilisearch/meilisearch-php/src/Http/Client.php:183

Do I understand well?
If I'm right, could you update this test to be more accurate? 🙂

meilisearch-php/tests/Endpoints/ClientTest.php

Lines 253 to 264 in 5c0562c

	public function testBadClientUrl(): void
	{
	try {
	$client = new Client('http://127.0.0.1.com:1234', 'some-key');
	$client->createIndex('index');
	} catch (\Exception $e) {
	$this->assertIsString($e->getMessage());
	return;
	}
	$this->fail('Bad client was accepted and the exception was not thrown');
	}

Thanks again for your multiple contributions 🚀

[shokme]

Hey !

No this is very different, here your exception came before getting the response. this is thrown by your HTTPClient (sendRequest() method) and the message said "uncaught" so this exception is not handled.

if you want to handle it you can do something like this:

// from the execute() method inside src/Http/Client.php
try {
    return $this->parseResponse($this->http->sendRequest($request));
} catch (NetworkExceptionInterface $e) {
    throw new HTTPRequestException($e->getCode(), $e->getMessage(), $e->getPrevious());
}

If you want to handle it, I can add it to this PR or another as you wish.

My PR fix the response exception, in my case http://localhost is a simple nginx. that's why I don't encounter the HTTPClient "Couldn't connect ..." and if I get localhost/version it will return a 404 response.

Edit:
I'm not able to find a way to test a statusCode >= 300 with a body inside, because I wonder if $response->getReasonPhrase() is not enough.

[curquiza]

Ok! It's clearer thank you 🙂

If you want to handle it, I can add it to this PR or another as you wish.

No, it's ok, I just wanted to understand, this behavior is not an emergency to handle for the moment 😉

Great! Going to merge it!

[curquiza]

🚀