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

Call to undefined method GuzzleHttp\Exception\ConnectException::getResponse() #42

Closed
TechPrivacyLab opened this issue Nov 5, 2021 · 3 comments
Assignees

Comments

@TechPrivacyLab
Copy link

In

$response = $e->getResponse();
the getResponse() method is undefined.

To reproduce it I put in the ZammadAPIClient\Client constructor a wrong URL:

use ZammadAPIClient\Client;
$client = new Client([
    'url' => 'https://wrong.zammad.url', // Wrong URL to my Zammad installation
    ...
]);

In this case $e is an instance of GuzzleHttp\Exception\ConnectException with no getResponse() method.

mgruner added a commit that referenced this issue Aug 1, 2022
@mgruner
Copy link
Collaborator

mgruner commented Aug 1, 2022

@TechPrivacyLab could you please check the changes in #51. This is the best I could come up with now, to only wrap the response if one exists, and re-throw the error otherwise.

@jepf do you perhaps remember why the try/catch was done at all? Is it really required?

@jepf
Copy link
Contributor

jepf commented Aug 1, 2022

@jepf do you perhaps remember why the try/catch was done at all? Is it really required?

@mgruner

The try/catch is common practice for error handling at this point.

Possible solutions:
a) try to find the right exception class which contains the response object (maybe there changed something in Guzzle)
b) or just output the message ($e->getMessage())of the exception and return some kind of empty Guzzle request object.

Be sure to not break the behavior by returning something unexpected to the caller.

@mgruner
Copy link
Collaborator

mgruner commented Aug 1, 2022

@jepf can you check my proposed changes in #51 please? It should not break anything, because it a) returns the error response if one is available and b) does not cause an unrelated RuntimeException as it does right now (that's what this bug is about), but instead throws a proper meaningful exception.

@mgruner mgruner self-assigned this Aug 1, 2022
mgruner added a commit that referenced this issue Sep 2, 2022
@mgruner mgruner closed this as completed in ecc170e Sep 2, 2022
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

No branches or pull requests

3 participants