Conversation
lishanl
left a comment
lishanl
left a comment
There was a problem hiding this comment.
Thanks Ant. It's a lot more readable.
Btw, python 3.9 doesn't support union with | that's what the python 3.9 codevalidation complained about.
add the following to enable future behavior?
from __future__ import annotations| async def _check_errors_field(self, errors: list | str, jsondata: dict): | ||
| has_data_field = 'data' in jsondata | ||
| if has_data_field and not self.suppress_errors: | ||
| raise ShopifyGQLError(jsondata) |
There was a problem hiding this comment.
thought/rhetoric question: would this scenario ever happen?
There was a problem hiding this comment.
That's a nice safety net, just in case. I don't know if it's possible but better to have code to catch it.
| errorlist = '\n'.join([err['message'] for err in jsondata['errors'] if 'message' in err]) | ||
| raise ValueError(f'GraphQL query is incorrect:\n{errorlist}') |
There was a problem hiding this comment.
question: check suppress_errors before raising just to keep the original logic? though I don't remember the purpose of suppress_errors. I have never set it to True.
There was a problem hiding this comment.
I guess sometimes one can get data and errors ... not sure. The point here is not to change functionality so I didn't touch it.
| if 'extensions' in err and 'code' in err['extensions']: | ||
| error_code = err['extensions']['code'] | ||
| self._handle_max_cost_exceeded_error_code(error_code=error_code) | ||
| await self._handle_throttled_error_code(error_code=error_code, jsondata=jsondata) |
There was a problem hiding this comment.
thought: it should be safe to assume the throttle error is the only error when it happens
There was a problem hiding this comment.
Again, I reproduced the same functionalities and avoided making any changes.
|
yea refactoring should preserve the original functionality / behavior. 👍🏼 Thanks ant |
2 participants
This part of the code was rather hard to follow and I think error prone although it was thoroughly tested.
This is however a nice cleanup that allows one to easily add more error handling without too much headache, I think. 🙏
It's more lines of code but I believe that they are much more readable.