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

Remove invalid method call #675

Conversation

greg0ire
Copy link
Contributor

@greg0ire greg0ire commented Apr 20, 2021

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related Issue n/a
Need Doc update no

Describe your change

I'm removing a call to existent method ConnectException::hasResponse()
When a connection fails, there cannot be a response: the client would
have to be connected to get one. Therefore, it would make no sense to
have a hasResponse() method on ConnectException.

What problem is this fixing?

This fixes a crash that can happen in case of connection issues.

@greg0ire
Copy link
Contributor Author

The build failures seem unrelated (429 too many attempts?)

@relaypilot
Copy link

I am also randomly receiving the following error:

Call to undefined method GuzzleHttp\\Exception\\ConnectException::hasResponse() at /home/my-website/vendor/algolia/algoliasearch-client-php/src/Http/GuzzleHttpClient.php:`

Using the following versions on PHP 7.4

"algolia/algoliasearch-client-php": "^3.0",
"algolia/scout-extended": "^1.17",
"laravel/framework": "^8.12",
"guzzlehttp/guzzle": "^7.0.1",

@greg0ire
Copy link
Contributor Author

@chloelbn please review

When a connection fails, there cannot be a response: the client would
have to be connected to get one. Therefore, it would make no sense to
have a hasResponse() method on ConnectException.
@chloelbn chloelbn force-pushed the stop-attempting-to-get-response-from-connect-exception branch from 12def50 to 8c510f0 Compare April 23, 2021 12:58
@chloelbn chloelbn mentioned this pull request Apr 23, 2021
@chloelbn
Copy link
Contributor

I've opened another PR to workaround the CI failing; we'll fix this another way and we'll be able to move forward!
#678

@chloelbn chloelbn closed this Apr 23, 2021
@greg0ire greg0ire deleted the stop-attempting-to-get-response-from-connect-exception branch April 24, 2021 07:54
@greg0ire
Copy link
Contributor Author

@chloelbn I think closing and reopening this PR would have been enough: In general, CI usually runs against a merge of the target and the source branch (not sure if that's the case with Circle CI though). So in theory, when reopening, a new merge would have been performed, with the new commit on master that contains your fix 🙂

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.

None yet

3 participants