fix: gracefully handle non json responses #112
Conversation
| # delete calls do not return a body | ||
| if resp.text == "" and resp.status_code == 200: | ||
| return None | ||
|
|
||
| # this is a safety net, if we get here it means the Resend API is having issues | ||
| # and most likely the gateway is returning htmls | ||
| if "application/json" not in resp.headers["content-type"]: |
There was a problem hiding this comment.
I would suggest to use req.headers.get("content-type", "") to avoid any KeyError
There was a problem hiding this comment.
we can also take a different approach
try:
dict_response = resp.json()
except requests.exceptions.JSONDecodeError:
raise_for_code_and_type(
code=500,
message="Failed to parse Resend API response. Please try again.",
error_type="InternalServerError",
)| # handle error in case there is a statusCode attr present | ||
| # and status != 200 | ||
| # and status != 200 and response is a json. | ||
| if resp.status_code != 200 and resp.json().get("statusCode"): |
There was a problem hiding this comment.
we can re-use the resp.json() definition
dict_response = resp.json()if resp.status_code != 200 and dict_response.get("statusCode"):raise_for_code_and_type(
code=dict_response.get("statusCode"),
message=dict_response.get("message"),
error_type=dict_response.get("name"),
)return cast(T, dict_response)
|
those comments are only improvements that we can make later. overall it looks like it is working |
|
thanks for the suggestions @pedroimpulcetto, for expediency im merging this as is and will push a pr with the improvements separately |
tests/emails_test.py
Outdated
| "subject": "subject", | ||
| "html": "html", | ||
| } | ||
| with self.assertRaises(ResendError): |
There was a problem hiding this comment.
let's assert for the error message too Failed to parse Resend API response. Please try again
There was a problem hiding this comment.
☝️ this is important to avoid a scenario where we are asserting for 500, but the error "inside" the API is not the one we are trying to simulate in the test
fixes: #111