SlackApiError: 'dict' object has no attribute 'status_code'
vinceta opened this issue · 3 comments
slack_sdk.web reports the 'dict' object has no attribute 'status_code' error when attempting to retrieve the SlackApiError's status_code.
Reproducible in:
$ pip list | grep slack
slack-sdk 3.23.0
$ pip freeze | grep slack
-e git+ssh://git@github.com/python-slack-sdk.git@9967dc0a206d974f7ef2c09dd6c49f70b98ecf1e#egg=slack_sdk
$ python --version
Python 3.9.6
$ sw_vers
ProductName: macOS
ProductVersion: 13.6.1
BuildVersion: 22G313Steps to reproduce:
- When catching SlackApiError, decide the different process logics by retrieving and checking the returned response's
status_code, similar to the example code of checking ratelimitstatus_code 429in slack.dev website. The code looks like below:
try:
response = send_slack_message(channel, message)
except SlackApiError as e:
if err_status_code := e.response.status_code >= 500:
print(f"SlackApiError returned status code: {err_status_code}")
else:
<omitted>- Wait for Slack Service Unavailable e.g. 503 to trigger the except processing in the above code
Expected result:
When SlackApiError happens, the code (by calling slack_sdk.web) can retrieve status_code from SlackApiError's response attribute successfully, w/o throwing out any further error.
Actual result:
When a real Slack service outage happened, the following AttributeError 'dict' object has no attribute 'status_code' was throw out unexpectedly:
slack_sdk.errors.SlackApiError: Received a response in a non-JSON format: <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"><html><head><title>503 Service Unavailable</title>...
The server responded with: {'status': 503, 'headers': {'date': '<day>, <date> <time>', 'server': 'Apache', 'content-length': '299', 'content-type': 'text/html; charset=iso-8859-1', 'x-envoy-upstream-service-time': '1', 'x-backend': 'main_normal main_canary_with_overflow main_control_with_overflow', 'x-server': 'slack-www-hhvm-main-iad-vdno', 'via': 'envoy-www-iad-rplytdoy, envoy-edge-iad-zbjxhisv', 'x-slack-shared-secret-outcome': 'no-match', 'x-edge-backend': 'envoy-www', 'x-slack-edge-shared-secret-outcome': 'no-match', 'connection': 'close'}, 'body': '<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">\n<html><head>\n<title>503 Service Unavailable</title>\n</head><body>\n<h1>Service Unavailable</h1>\n<p>The server is temporarily unable to service your\nrequest due to maintenance downtime or capacity\nproblems. Please try again later.</p>\n</body></html>\n'}
During handling of the above exception, another exception occurred:
AttributeError: 'dict' object has no attribute 'status_code'Probable cause:
When encountering API return's json decoding error (that's the case of 500 error), the response is a dict (returned by _perform_urllib_http_request()) - this implementation in slack_sdk.web.base_client module must have been there for a long time, even before v3.0 :
response = self._perform_urllib_http_request(url=url, args=request_args)
response_body = response.get("body", None) # skipcq: PTC-W0039
response_body_data: Optional[Union[dict, bytes]] = response_body
if response_body is not None and not isinstance(response_body, bytes):
try:
response_body_data = json.loads(response["body"])
except json.decoder.JSONDecodeError:
message = _build_unexpected_body_error_message(response.get("body", ""))
raise err.SlackApiError(message, response)But SlackApiError's response attribute should be the type of SlackResponse instead of dict that doesn't have status_code:
class SlackApiError(SlackClientError):
"""Error raised when Slack does not send the expected response.
Attributes:
response (SlackResponse): The SlackResponse object containing all of the data sent back from the API.Probable solution:
There are probably several solutions. To make the code easy to reference (always provide status_code return for whatever SlackApiError) and consistent (use only one type of response for SlackApiError and one entry to SlackApiError), we can re-assemble the response body data by leveraging the message output from _build_unexpected_body_error_message() (that's a str), with one line change only:
except json.decoder.JSONDecodeError:
message = _build_unexpected_body_error_message(response.get("body", ""))
# raise err.SlackApiError(message, response)
response_body_data = {'ok': False, 'error': message}Then this payload will be sent to SlackResponse's validate(), that will trigger SlackApiError eventually:
return SlackResponse(
client=self,
http_verb="POST", # you can use POST method for all the Web APIs
api_url=url,
req_args=request_args,
data=response_body_data,
headers=dict(response["headers"]),
status_code=response["status"],
).validate()Seeking the feedback and suggestions - I can raise a PR if the above proposed fix is acceptable.
Mock test:
Using the above proposed solution, mock test passed like below - no longer trigger another unexpected AttributeError, also provide status_code access and consistent error message:
>>> with patch('slack_sdk.WebClient._perform_urllib_http_request') as mock_urllib:
... http_error_response = HTTPError("https://example.com/api/data", 503, "503 Service Unavailable", None, BytesIO(b'<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"><html><head><title>503 Service Unavailable</title>'))
... mock_urllib.side_effect = http_error_response
... try:
... ret = client.chat_postMessage(channel="chan", text="hello")
... except HTTPError as e:
... response_headers = {'date': '<day>, <date> <time>', 'server': 'Apache', 'content-length': '299', 'content-type': 'text/html; charset=iso-8859-1'}
... response = {"status": e.code, "headers": response_headers, "body": e.read().decode("utf-8")}
... print(f"The type of returned response: {type(response)}")
... response_body = response.get("body", None)
... response_body_data: Optional[Union[dict, bytes]] = response_body
... if response_body is not None and not isinstance(response_body, bytes):
... try:
... response_body_data = json.loads(response["body"])
... except json.decoder.JSONDecodeError:
... message = _build_unexpected_body_error_message(response.get("body", ""))
... # raise SlackApiError(message, response)
... response_body_data = {'ok': False, 'error': message}
... try:
... SlackResponse(
... client=client,
... http_verb="POST", # you can use POST method for all the Web APIs
... api_url=url,
... req_args=request_args,
... data=response_body_data,
... headers=dict(response["headers"]),
... status_code=response["status"],
... ).validate()
... except SlackApiError as e:
... print(f"SlackApiError attribute response's status_code: {e.response.status_code}")
... print(f"SlackApiError attribute response's data: {e.response.data}")
...
The type of returned response: <class 'dict'>
SlackApiError attribute response's status_code: 503
SlackApiError attribute response's data: {'ok': False, 'error': 'Received a response in a non-JSON format: <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN"><html><head><title>503 Service Unavailable</title>'}
>>>Hi @vinceta thanks for writing and providing great context 💯
This does feel like odd behavior and it seems like our documentation may not take into account all edge cases
Thank you for the exploratory work surrounding this issue 🥇 my main concern surrounding returning a SlackResponse instead of the existing behavior is that it may cause a braking change to the library
I'd like to get @seratch incite on this behavior has this been brought up before and has there been some thoughts surrounding this?
Improving the documentation and examples around this specific edge case could be a first step as well
Hi @vinceta, thank you very much for taking the time to write this insightful bug report. Indeed, this seems to be a bug on the SDK. If you're willing to make a pull request to resolve this issue, we are happy to merge your contribution. Otherwise, I will resolve this issue when I have time for it soon!
@seratch @WilliamBergamin thanks for checking this and confirming this's indeed a bug. I will make a pull request and ask for your further review, thanks again for your swift response!