Skip to content

Commit

Permalink
Merge pull request #371 from saloonphp/fix/fail-request-properly
Browse files Browse the repository at this point in the history
Fix | Allow Return False In hasRequestFailed
  • Loading branch information
Sammyjo20 committed Feb 14, 2024
2 parents 558c48d + d531a35 commit 8dce746
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 5 deletions.
7 changes: 4 additions & 3 deletions src/Http/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,11 @@ public function failed(): bool
{
$pendingRequest = $this->getPendingRequest();

$hasRequestFailed = $pendingRequest->getRequest()->hasRequestFailed($this) || $pendingRequest->getConnector()->hasRequestFailed($this);
$requestFailedAccordingToConnector = $pendingRequest->getConnector()->hasRequestFailed($this);
$requestFailedAccordingToRequest = $pendingRequest->getRequest()->hasRequestFailed($this);

if ($hasRequestFailed === true) {
return true;
if ($requestFailedAccordingToRequest !== null || $requestFailedAccordingToConnector !== null) {
return $requestFailedAccordingToRequest || $requestFailedAccordingToConnector;
}

return $this->serverError() || $this->clientError();
Expand Down
2 changes: 1 addition & 1 deletion src/Http/Senders/GuzzleSender.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ function (TransferException $guzzleException) use ($pendingRequest, $psrRequest)

// Throw the exception our way

throw $response->toException();
return ($exception = $response->toException()) ? throw $exception : $response;
}
);
}
Expand Down
48 changes: 47 additions & 1 deletion tests/Feature/RequestExceptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use Saloon\Http\PendingRequest;
use Saloon\Http\Faking\MockClient;
use Saloon\Http\Faking\MockResponse;
use GuzzleHttp\Promise\PromiseInterface;
use GuzzleHttp\Exception\ServerException;
use Saloon\Exceptions\Request\ClientException;
use Saloon\Exceptions\Request\RequestException;
Expand All @@ -14,6 +15,7 @@
use Saloon\Tests\Fixtures\Connectors\TestConnector;
use Saloon\Exceptions\Request\FatalRequestException;
use Saloon\Tests\Fixtures\Requests\BadResponseRequest;
use Saloon\Tests\Fixtures\Requests\NotFoundFailedRequest;
use Saloon\Tests\Fixtures\Connectors\BadResponseConnector;
use Saloon\Tests\Fixtures\Exceptions\CustomRequestException;
use Saloon\Tests\Fixtures\Requests\CustomFailHandlerRequest;
Expand Down Expand Up @@ -246,13 +248,57 @@
expect($responseB->failed())->toBeTrue();
});

test('a request can mark a request as not failed', function () {
$response = TestConnector::make()->send(new NotFoundFailedRequest);

expect($response->failed())->toBeFalse();
});

test('a request can mark a request as not failed with asynchronous requests', function () {
$response = TestConnector::make()->sendAsync(new NotFoundFailedRequest)->wait();

expect($response->failed())->toBeFalse();
});

test('a request can mark a request as not failed with pools', function () {
$responseCount = 0;
$exceptionCount = 0;

$pool = TestConnector::make()->pool([
new NotFoundFailedRequest,
]);

$pool->withResponseHandler(function (Response $response) use (&$responseCount) {
expect($response)->toBeInstanceOf(Response::class);
expect($response->status())->toBe(404);

$responseCount++;
})->withExceptionHandler(function (RequestException $exception) use (&$exceptionCount) {
$response = $exception->getResponse();

expect($response)->toBeInstanceOf(Response::class);
expect($response->status())->toBe(404);

$exceptionCount++;
});

$promise = $pool->send();

expect($promise)->toBeInstanceOf(PromiseInterface::class);

$promise->wait();

expect($responseCount)->toEqual(1);
expect($exceptionCount)->toEqual(0);
});

test('the sender will throw a FatalRequestException if it cannot connect to a site using synchronous', function (string $url) {
$connector = new TestConnector($url);
$request = new UserRequest();

$this->expectException(FatalRequestException::class);

$response = $connector->send($request);
$connector->send($request);
})->with([
'https://saloon.saloon.test',
'https://saloon.doesnt-exist',
Expand Down
40 changes: 40 additions & 0 deletions tests/Fixtures/Requests/NotFoundFailedRequest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

declare(strict_types=1);

namespace Saloon\Tests\Fixtures\Requests;

use Saloon\Enums\Method;
use Saloon\Http\Request;
use Saloon\Http\Response;
use Saloon\Traits\Plugins\AlwaysThrowOnErrors;

class NotFoundFailedRequest extends Request
{
use AlwaysThrowOnErrors;

/**
* Define the HTTP method.
*/
protected Method $method = Method::GET;

/**
* Define the endpoint for the request.
*/
public function resolveEndpoint(): string
{
return '/not-found';
}

/**
* Determine if the request has failed
*/
public function hasRequestFailed(Response $response): ?bool
{
if ($response->status() === 404) {
return false;
}

return ($response->serverError() || $response->clientError());
}
}

0 comments on commit 8dce746

Please sign in to comment.