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

Possible TypeError when type hinting the Pool rejected callable with a RequestException #3011

Open
gdejong opened this issue May 6, 2022 · 9 comments
Labels
lifecycle/stale No activity for a long time

Comments

@gdejong
Copy link

gdejong commented May 6, 2022

Guzzle version(s) affected: 7.4.2

Description
The documentation for using a GuzzleHttp\Pool suggests type hinting the rejected callable with a \GuzzleHttp\Exception\RequestException
https://docs.guzzlephp.org/en/stable/quickstart.html#concurrent-requests

This can lead to a TypeError since it is possible to receive a \GuzzleHttp\Exception\ConnectException at this point as well.

How to reproduce

<?php
declare(strict_types=1);

use GuzzleHttp\Client;
use GuzzleHttp\Exception\RequestException;
use GuzzleHttp\Pool;
use GuzzleHttp\Psr7\Request;
use GuzzleHttp\Psr7\Response;

require __DIR__ . "/vendor/autoload.php";

$client = new Client();

$requests = [
    new Request("GET", 'http://localhost:1337'),
];

$pool = new Pool($client, $requests, [
    'concurrency' => 5,
    'fulfilled' => static function (Response $response, $index) {
        echo "fulfilled!" . PHP_EOL;
    },
    'rejected' => static function (RequestException $reason, $index) {
        echo "rejected: " . $reason->getMessage() . PHP_EOL;
    },
]);

$pool->promise()->wait();

This results in the following error:

PHP Fatal error:  Uncaught TypeError: Argument 1 passed to {closure}() must be an instance of GuzzleHttp\Exception\RequestException, instance of GuzzleHttp\Exception\ConnectException given

Possible Solution

Changing RequestException to TransferException would prevent the TypeError:

    'rejected' => static function (TransferException $reason, $index) {
        echo "rejected: " . $reason->getMessage() . PHP_EOL;
    },

(since both RequestException and ConnectException extend the TransferException)

Can the example in the documentation be updated to prevent a TypeError from happening?

Additional context

image

@guigui0326
Copy link

Using TransferException will throw [GuzzleHttp\Exception\RequestException], how to solve it?

@stale
Copy link

stale bot commented Sep 25, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Sep 25, 2022
@gdejong
Copy link
Author

gdejong commented Sep 28, 2022

Not stale.

@stale stale bot removed the lifecycle/stale No activity for a long time label Sep 28, 2022
@wassilis83
Copy link

I have this issue too using latest laravel 9, I tried the TransferException fix but for me is not working, I always got GuzzleHttp\Exception\ConnectException given. Any fix for this?

@magnetik
Copy link

7.5 is affected too.

@stempora
Copy link

Any solution for this ?

@stale
Copy link

stale bot commented Jun 10, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Jun 10, 2023
@GrahamCampbell
Copy link
Member

This is just a documentation issue. If you are unsure of the correct type to use, you could just put Throwable there.

@stale stale bot removed the lifecycle/stale No activity for a long time label Jun 10, 2023
Copy link

stale bot commented Mar 17, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 2 weeks if no further activity occurs. Thank you for your contributions.

@stale stale bot added the lifecycle/stale No activity for a long time label Mar 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale No activity for a long time
Projects
None yet
Development

No branches or pull requests

6 participants