gnikyt / Basic-Shopify-API

A simple API wrapper for Shopify using Guzzle for REST and GraphQL
MIT License
220 stars 66 forks source link

Inconsistent Error Handling in use of Body in Response #124

Open ooglek opened 3 years ago

ooglek commented 3 years ago

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 commented 2 years ago

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