feat: add support for Guzzle 8 #9377
Conversation
GrahamCampbell
changed the title
e9b14c3 to
406ea4c
Compare
| function ($line) { | ||
| return strpos($line, 'Content-Length:') !== 0; | ||
| } |
There was a problem hiding this comment.
nit
| function ($line) { | |
| return strpos($line, 'Content-Length:') !== 0; | |
| } | |
| fn ($line) => strpos($line, 'Content-Length:') !== 0; |
| // Guzzle 7 carries the response on RequestException, Guzzle 8 only on | ||
| // its ResponseException subclass, hence the method_exists() check. | ||
| $res = method_exists($ex, 'getResponse') ? $ex->getResponse() : null; | ||
| $body = (string) $res->getBody(); |
There was a problem hiding this comment.
In this instance, we would get a PHP fatal error ("Call to a member function getBody() on null") if the above exception does not have method getResponse. However, the current behavior is the same - if getResponse() returns null, the same PHP fatal error is thrown.
If I am understanding this correctly, EVERY instance in Guzzle 7 which returned a response for RequestException::getResponse() now returns a ResponseException in Guzzle 8, is that accurate? If so this should be safe, and any changes to this behavior would be a separate bug fix.
Even so it might be a good idea to protect from fatal errors like this, and to add a comment in the method description that the method expects either a RequestException with a response in Guzzle 7, or a ResponseException in Guzzle 8.
| $body = (string) $res->getBody(); | |
| $body = (string) $res?->getBody(); |
Hectorhammett
left a comment
Hectorhammett
left a comment
There was a problem hiding this comment.
This looks good to me. Thank you so much @GrahamCampbell For this contribution!
3 participants
Guzzle 8.0.0 was released on July 20th. Lots of people want to use it, and thus popular packages in the ecosystem like this one need to be updated to support it, so here I am, adding support. :)