Standardize error messages #84
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmosorast
requested changes
Feb 24, 2021
| body = json.loads(body_json) | ||
| msg = body.get('errors') | ||
| finally: | ||
| raise ShopifyError(exc, msg) from exc |
There was a problem hiding this comment.
Interesting, just to get this down, I wasn't sure this would work, but it seems like any exception thrown from JSON parsing will just get swallowed implicitly, and the true exception will bubble up. Nice.
>>> try:
... raise Exception("1")
... except Exception as ex:
... try:
... raise Exception("2")
... finally:
... raise Exception("3") from ex
...
Traceback (most recent call last):
File "<stdin>", line 2, in <module>
Exception: 1
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "<stdin>", line 7, in <module>
Exception: 3
There was a problem hiding this comment.
Yup, I was skeptical of it too but it works.
| @@ -0,0 +1,3 @@ | |||
| class ShopifyError(Exception): | |||
| def __init__(self, error, msg=''): | |||
| super().__init__('{}\n{}'.format(error.__class__.__name__, msg)) | |||
There was a problem hiding this comment.
So will this, e.g., come out like
CRITICAL pyactiveresource.connection.ResourceNotFound
CRITICAL Ensure shop is entered correctly
?
There was a problem hiding this comment.
The full namespace of the exception is not printed.
CRITICAL ResourceNotFound
CRITICAL Ensure shop is entered correctly
dmosorast
approved these changes
Feb 26, 2021
tmck-code
pushed a commit
to lexerdev/tap-shopify
that referenced
this pull request
Feb 3, 2022
* Normalize the error messages * Broaden the exception * Put error message on a single line * Add helpful text in case of `ResourceNotFound` * Change top level exception logger to log lines individually * Add helpful message in case of UnauthorizedAccess error * Raise exceptions from causal exception * Bump singer-python to 5.11.0 and use `handle_top_exception` again
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
Standardize the error messages and add some information to allow end users to troubleshoot the problem.
QA steps
Risks
Rollback steps