Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🤦 Fixing issues around Oauth and SlackApiErrors #527

Merged
merged 1 commit into from
Oct 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion slack/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,16 @@ class SlackRequestError(SlackClientError):


class SlackApiError(SlackClientError):
"""Error raised when Slack does not send the expected response."""
"""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.

Note:
The message (str) passed into the exception is used when
a user converts the exception to a str.
i.e. str(SlackApiError("This text will be sent as a string."))
"""

def __init__(self, message, response):
msg = f"{message}\nThe server responded with: {response}"
Expand Down
11 changes: 9 additions & 2 deletions slack/web/base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

# ThirdParty Imports
import aiohttp
from aiohttp import FormData
from aiohttp import FormData, BasicAuth

# Internal Imports
from slack.web.slack_response import SlackResponse
Expand Down Expand Up @@ -104,6 +104,7 @@ def api_call(
params: dict = None,
json: dict = None,
headers: dict = {},
auth: dict = None,
) -> Union[asyncio.Future, SlackResponse]:
"""Create a request and execute the API call to Slack.

Expand Down Expand Up @@ -143,6 +144,9 @@ def api_call(

api_url = self._get_url(api_method)

if auth:
auth = BasicAuth(auth["client_id"], auth["client_secret"])

req_args = {
"headers": self._get_headers(has_json, has_files, headers),
"data": data,
Expand All @@ -151,6 +155,7 @@ def api_call(
"json": json,
"ssl": self.ssl,
"proxy": self.proxy,
"auth": auth,
}

if self._event_loop is None:
Expand Down Expand Up @@ -248,7 +253,9 @@ async def _request(self, *, http_verb, api_url, req_args):
"status_code": res.status,
}
async with aiohttp.ClientSession(
loop=self._event_loop, timeout=aiohttp.ClientTimeout(total=self.timeout)
loop=self._event_loop,
timeout=aiohttp.ClientTimeout(total=self.timeout),
auth=req_args.pop("auth"),
) as session:
async with session.request(http_verb, api_url, **req_args) as res:
return {
Expand Down
8 changes: 6 additions & 2 deletions slack/web/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,8 +1124,12 @@ def oauth_access(
client_secret (str): Issued when you created your application. e.g. '33fea0113f5b1'
code (str): The code param returned via the OAuth callback. e.g. 'ccdaa72ad'
"""
headers = {"client_id": client_id, "client_secret": client_secret, "code": code}
return self.api_call("oauth.access", data=kwargs, headers=headers)
kwargs.update({"code": code})
return self.api_call(
"oauth.access",
data=kwargs,
auth={"client_id": client_id, "client_secret": client_secret},
)

def pins_add(self, *, channel: str, **kwargs) -> Union[Future, SlackResponse]:
"""Pins an item to a channel.
Expand Down
2 changes: 1 addition & 1 deletion slack/web/slack_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def validate(self):
self._logger.debug("Received the following response: %s", self.data)
return self
msg = "The request to the Slack API failed."
raise e.SlackApiError(message=msg, response=self.data)
raise e.SlackApiError(message=msg, response=self)

@staticmethod
def _next_cursor_is_present(data):
Expand Down
2 changes: 2 additions & 0 deletions tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def fake_req_args(headers=ANY, data=ANY, params=ANY, json=ANY):
"json": json,
"ssl": ANY,
"proxy": ANY,
"auth": ANY,
}
return req_args

Expand All @@ -29,6 +30,7 @@ def fake_send_req_args(headers=ANY, data=ANY, params=ANY, json=ANY):
"ssl": ANY,
"proxy": ANY,
"files": ANY,
"auth": ANY,
}
return req_args

Expand Down
11 changes: 11 additions & 0 deletions tests/web/test_web_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ def test_slack_api_error_is_raised_on_unsuccessful_responses(self, mock_request)
with self.assertRaises(err.SlackApiError):
self.client.api_test()

def test_slack_api_rate_limiting_exception_returns_retry_after(self, mock_request):
mock_request.response.side_effect = [
{"data": {"ok": False}, "status_code": 429, "headers": {"Retry-After": 30}}
]
with self.assertRaises(err.SlackApiError) as context:
self.client.api_test()
slack_api_error = context.exception
self.assertFalse(slack_api_error.response["ok"])
self.assertEqual(429, slack_api_error.response.status_code)
self.assertEqual(30, slack_api_error.response.headers["Retry-After"])

def test_the_api_call_files_argument_creates_the_expected_data(self, mock_request):
self.client.token = "xoxa-123"
with mock.patch("builtins.open", mock.mock_open(read_data="fake")):
Expand Down