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

[9.x] Allow faking connection errors in HTTP client #44852

Closed

Conversation

sebdesign
Copy link
Contributor

@sebdesign sebdesign commented Nov 6, 2022

Purpose

This PR brings the ability to fake connection/networking errors when HTTP requests are made.

Why this is useful

Until now, we could fake HTTP responses, both successful or failed (4xx/5xx). But there is no way to test how the HTTP client or our application behaves when there is a networking error, e.g. a host that could not be resolved, a connection timeout, an SSL error, etc.

In fact, there are currently no tests that assert the ConnectionFailed event is dispatched, or that the retry method catches the ConnectionException.

Example use-case

I have some jobs that make API calls to a server that might be down or timeouts. When that happens, I need to notify someone.
Of course, I could do that before, but there was no easy way to test that case.

class ExampleJob implements ShouldQueue
{
    public function handle()
    {
        rescue(
            fn () => Http::get('https://aws.example.com')),
            fn ($e) => Notification::send($developers, new AwsIsDownAgain($e->getMessage())),
        );
    }
}

public function test_aws_is_down()
{
    Http::fake(['aws.example.com' => Http::error('down')]);
    Notification::fake([AwsIsDownAgain::class]);

    Bus::dispatch(new ExampleJob());

    Http::assertSent(fn ($request, $response) => $response instanceof ConnectionException);
    Notification::assertSentTo($developers, fn (AwsIsDownAgain $notification) => $notification->message === 'down');
}

Usage

The new Http::error() method accepts two optional parameters: an error message, and an array with the (cURL) handler context.

Http::fake([
    'error.example.com' => Http::error(),
    'expired.example.com' => Http::error('Could not resolve host: expired.example.com'),
    'timeout.example.com' => Http::error('Connection timed out after 42 milliseconds', ['errno' => 28]),
]);

This method works with all the common features of the HTTP client:

  • Concurent requests using HTTP::pool()
  • Dispatching RequestSending and ConnectionFailed events
  • Faking response sequences using Http::sequence() and Http::fakeSequence()
  • Inspecting requests using assertSent(), assertNotSent(), etc.
  • Recording requests / exceptions using Http::recorded()

Simple requests

Inside your tests, making a request will still throw a ConnectionException, but it will not actually send the request.

public function test_the_domain_expired()
{
    Http::fake([
        'expired.example.com' => Http::error('Could not resolve host: expired.example.com'),
    ]);

    $this->expectException(ConnectionException::class);
    $this->expectExceptionMessage('Could not resolve host: expired.example.com');

    Http::get('https://expired.example.com');
}

Concurrent requests

When using Http::pool(), the response instance will be a Http\Client\ConnectionException instead of a Http\Client\Response.

public function test_aws_is_down()
{
    Http::fake([
        'us-east-1.amazon.com' => Http::response('us-east-1 is up'),
        'us-east-2.amazon.com' => Http::error('us-east-2 is down'),
    ]);

    [$usEast1, $usEast2] = Http::pool(fn (Pool $pool) => [
        $pool->get('us-east-1.amazon.com'),
        $pool->get('us-east-2.amazon.com'),
    ]);

   $this->assertInstanceOf(Response::class, $usEast1);
   $this->assertEquals('us-east-1 is up', $usEast1->body());

   $this->assertInstanceOf(ConnectionException::class, $usEast2);
   $this->assertEquals('us-east-2 is down', $usEast2->getMessage());
}

Events

public function test_events()
{
    Event::fake();

    Http::fake(['error.example.com' => Http::error()]);

    rescue(fn () => Http::get('https://error.example.com'));

    Event::assertDispatched(RequestSending::class);
    Event::assertDispatched(ConnectionFailed::class);
    Event::assertNotDispatched(ResponseReceived::class);
}

Faking response sequences

public function test_sequences()
{
    Http::fake(['example.com' => Http::sequence()
        ->push('Hello world')
        ->pushError('Connection failed'),
    ]);

    $response = Http::get('example.com');

    $this->assertEquals('Hello world', $response->body());

    $this->expectException(ConnectionException::class);
    $this->expectExceptionMessage('Connection failed');

    Http::get('example.com');
}

Inspecting requests

When asserting requests that throws connection errors, the Http::assert*() methods receive a Http\Client\ConnectionException instead of a Http\Client\Response.

public function test_inspections()
{
    Http::fake(['error.example.com' => Http::error('Connection failed')]);

    rescue(fn () => Http::get('https://error.example.com'));

    Http::assertSentCount(1);

    Http::assertSent(function (Request $request, ConnectionException $response) {
        return $request->url() === 'https://error.example.com'
            && $response->getMessage() === 'Connection failed';
    });
}

@GrahamCampbell GrahamCampbell changed the title Allow faking connection errors in HTTP client [9.x] Allow faking connection errors in HTTP client Nov 6, 2022
@timacdonald
Copy link
Member

I needed this just the other day!

@commentatorboy
Copy link

commentatorboy commented Nov 8, 2022

Haha. Best timing ever. I am actually in the same situation.
I wanted to manually throw a ConnectionException but it would only throw RequestException.

But one question.
@sebdesign is there a way to choose what to throw?

For example:

Http::fakeSequence()
   ->pushError(new RequestException(), ....)
   ->pushError(new ConnectionException(), ....)
   ->pushStatus(200)

@sebdesign
Copy link
Contributor Author

@commentatorboy in what case would you need to throw a RequestException? Laravel automatically converts 4xx/5xx responses to RequestException, if you call throw() on the response:

Http::fakeSequence()
   ->pushStatus(400) // this will throw a RequestException
   ->pushStatus(200)

@commentatorboy
Copy link

@sebdesign Thank you for writing.

Aha I see. I thought that there were other types of exceptions related to Http. So yeah thank you for clearing that up. :)

@taylorotwell
Copy link
Member

taylorotwell commented Nov 9, 2022

Feels like a lot of code got shifted around here to add this feature. Can you elaborate on some of those changes and why they were needed?

@sebdesign
Copy link
Contributor Author

sebdesign commented Nov 10, 2022

@taylorotwell of course!

Factory::error()

First, the Http::error() method returns a closure, because the ConnectException needs a Request object to be constructed.
Closures are already supported when faking responses:

Http::fake([
    '*' => fn ($request, $options) => Http::response(),
]);

Http::fake(fn ($request, $options) => Http::response());

These closures were evaluated in the Factory::stubUrl() method or in the Factory::fake() method, respectively.

The problem in the Factory::fake() method is that the TransferStats are evaluated there, which is not quite right because it only works for closures that return response promises, and Http::response(), but not for arrays. This was introduced in #37738, and as you can see in this line, the $effectiveUri is still null, which means the transfer stats were not evaluated, which make this test a false positive.

In order to address this, the transfer stats need to be evaluated much later, after the stubs have been converted to responses.

PendingRequest::makePromise()

When sending async/concurrent requests, the promises must be fulfilled with the response, or rejected with the response of the RequestException. In case a ConnectException was thrown, the ConnectionFailed event was not dispatched, and the exception was not wrapped in Laravel's ConnectionException.

PendingRequest::buildRecorderHandler()

Previously, the recorder was only handling fulfilled promises, but not rejected ones. In order to record requests that throw ConnectExceptions, we need to record a pair with the request and the ConnectionException.

PendingRequest::buildStubHandler()

Instead of evaluating all the stub callbacks that match the request, and then filtering them to select the first one that is not null, we can use the reduce method to only evaluate the first stub that satisfies the request.

Then we decide how to handle the result: if it's null, we send it to the real handler. If it's a ConnectException, we reject the promise. If it's an array, we convert it to a response promise that will be fulfilled.

ResponseSequence::pushError()

In order to push a ConnectException to the sequence, we need to use a closure again. Previously the __invoke method would return response promises only, but since the pushError() yields a closure and not a promise, this makes the testSequenceBuilder test to error around the recorder handler.

The solution is to evaluate the closure inside the __invoke method, which is invoked here.

I hope my explanation is not very confusing, and I'd be happy to address anything you want.

@taylorotwell
Copy link
Member

I dunno - there are just too many changes here on an already fragile component we have had problems with PRs breaking before. I would just suggest just putting some HTTP calls where you need this testability behind another class that you can inject and mock and then throw ConnectionExceptions that way to test your systems response to those types of exceptions.

@taylorotwell
Copy link
Member

Or, if you can figure out a way to add this to Http::fake without so many changes feel free to PR that.

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

Successfully merging this pull request may close these issues.

4 participants