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

Inconsistent Error Handling in use of Body in Response #124

Open
ooglek opened this issue Sep 24, 2021 · 1 comment
Open

Inconsistent Error Handling in use of Body in Response #124

ooglek opened this issue Sep 24, 2021 · 1 comment

Comments

@ooglek
Copy link

ooglek commented Sep 24, 2021

Hey, first off, this is great code. Thank you!!!

I've been toying around with failure conditions while using the graph() functionality.

When I pass an invalid value to a variable in my GraphQL Query, I get an instance of Osiset\BasicShopifyAPI\ResponseAccess in the $result['body'] field. I then can just dump it using the ->toArray() method, easy peasy.

But when the error is something like Authentication, the $result['body'] field contains a string, such as [API] Invalid API key or access token (unrecognized login or wrong password) and then $result['exception'] exists magically and I can also access the exception.

This creates complexity in the implementation, as you have to handle different types of failure cases in different ways, overloading the meaning of each key/value at the top level of the result of the API call leading to a lot of conditionals.

For consistency, and I'm just spitballing here:

  1. body is always an instance of Osiset\BasicShopifyAPI\ResponseAccess and you check if it ->hasErrors()
  2. exception always exists but is null unless there is an exception

I haven't dug deep enough to suggest more, just a note on how frustrating it is to have to write code to handle the different ways the variables are populated or exist or not.

@gnikyt
Copy link
Owner

gnikyt commented Nov 17, 2021

@ooglek Sounds good to me, makes sense. Would you want to do a PR for this?

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

2 participants