Skip to content

Commit b5823f8

Browse files
seratchseratch
authored
Fix #1143 Keep Retry-After header compatibility wit 2.x series (#1148)
1 parent bcaee74 commit b5823f8

File tree

15 files changed

+83
-24
lines changed

15 files changed

+83
-24
lines changed

‎integration_tests/web/test_issue_1143.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import os
2+
import unittest
3+
4+
from integration_tests.env_variable_names import SLACK_SDK_TEST_BOT_TOKEN
5+
from integration_tests.helpers import async_test
6+
from slack_sdk.errors import SlackApiError
7+
from slack_sdk.web import WebClient
8+
from slack_sdk.web.async_client import AsyncWebClient
9+
10+
11+
class TestWebClient(unittest.TestCase):
12+
"""Runs integration tests with real Slack API
13+
14+
export SLACK_SDK_TEST_BOT_TOKEN=xoxb-xxx
15+
python setup.py run_integration_tests --test-target integration_tests/web/test_issue_1143.py
16+
17+
https://github.com/slackapi/python-slack-sdk/issues/1143
18+
"""
19+
20+
def setUp(self):
21+
self.bot_token = os.environ[SLACK_SDK_TEST_BOT_TOKEN]
22+
23+
def tearDown(self):
24+
pass
25+
26+
def test_backward_compatible_header(self):
27+
client: WebClient = WebClient(token=self.bot_token)
28+
try:
29+
while True:
30+
client.users_list()
31+
except SlackApiError as e:
32+
self.assertIsNotNone(e.response.headers["Retry-After"])
33+
34+
@async_test
35+
async def test_backward_compatible_header_async(self):
36+
client: AsyncWebClient = AsyncWebClient(token=self.bot_token)
37+
try:
38+
while True:
39+
await client.users_list()
40+
except SlackApiError as e:
41+
self.assertIsNotNone(e.response.headers["Retry-After"])

‎slack_sdk/audit_logs/v1/client.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,11 +260,13 @@ def _perform_http_request(
260260
# read the response body here
261261
charset = e.headers.get_content_charset() or "utf-8"
262262
response_body: str = e.read().decode(charset)
263+
# As adding new values to HTTPError#headers can be ignored, building a new dict object here
264+
response_headers = dict(e.headers.items())
263265
resp = AuditLogsResponse(
264266
url=url,
265267
status_code=e.code,
266268
raw_body=response_body,
267-
headers=dict(e.headers.items()),
269+
headers=response_headers,
268270
)
269271
if e.code == 429:
270272
# for backward-compatibility with WebClient (v.2.5.0 or older)

‎slack_sdk/scim/v1/client.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,11 +332,13 @@ def _perform_http_request(
332332
# read the response body here
333333
charset = e.headers.get_content_charset() or "utf-8"
334334
response_body: str = e.read().decode(charset)
335+
# As adding new values to HTTPError#headers can be ignored, building a new dict object here
336+
response_headers = dict(e.headers.items())
335337
resp = SCIMResponse(
336338
url=url,
337339
status_code=e.code,
338340
raw_body=response_body,
339-
headers=dict(e.headers.items()),
341+
headers=response_headers,
340342
)
341343
if e.code == 429:
342344
# for backward-compatibility with WebClient (v.2.5.0 or older)

‎slack_sdk/web/base_client.py‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -414,19 +414,25 @@ def _perform_urllib_http_request(
414414
return resp
415415

416416
except HTTPError as e:
417-
resp = {"status": e.code, "headers": e.headers}
417+
# As adding new values to HTTPError#headers can be ignored, building a new dict object here
418+
response_headers = dict(e.headers.items())
419+
resp = {"status": e.code, "headers": response_headers}
418420
if e.code == 429:
419421
# for compatibility with aiohttp
420422
if (
421-
"retry-after" not in resp["headers"]
422-
and "Retry-After" in resp["headers"]
423+
"retry-after" not in response_headers
424+
and "Retry-After" in response_headers
423425
):
424-
resp["headers"]["retry-after"] = resp["headers"]["Retry-After"]
426+
response_headers["retry-after"] = response_headers[
427+
"Retry-After"
428+
]
425429
if (
426-
"Retry-After" not in resp["headers"]
427-
and "retry-after" in resp["headers"]
430+
"Retry-After" not in response_headers
431+
and "retry-after" in response_headers
428432
):
429-
resp["headers"]["Retry-After"] = resp["headers"]["retry-after"]
433+
response_headers["Retry-After"] = response_headers[
434+
"retry-after"
435+
]
430436

431437
# read the response body here
432438
charset = e.headers.get_content_charset() or "utf-8"
@@ -437,7 +443,7 @@ def _perform_urllib_http_request(
437443
retry_request = RetryHttpRequest.from_urllib_http_request(req)
438444
retry_response = RetryHttpResponse(
439445
status_code=e.code,
440-
headers={k: [v] for k, v in e.headers.items()},
446+
headers={k: [v] for k, v in response_headers.items()},
441447
data=response_body.encode("utf-8")
442448
if response_body is not None
443449
else None,

‎slack_sdk/web/legacy_base_client.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -508,19 +508,21 @@ def _perform_urllib_http_request(
508508
return {"status": resp.code, "headers": resp.headers, "body": body}
509509
raise SlackRequestError(f"Invalid URL detected: {url}")
510510
except HTTPError as e:
511-
resp = {"status": e.code, "headers": e.headers}
511+
# As adding new values to HTTPError#headers can be ignored, building a new dict object here
512+
response_headers = dict(e.headers.items())
513+
resp = {"status": e.code, "headers": response_headers}
512514
if e.code == 429:
513515
# for compatibility with aiohttp
514516
if (
515-
"retry-after" not in resp["headers"]
516-
and "Retry-After" in resp["headers"]
517+
"retry-after" not in response_headers
518+
and "Retry-After" in response_headers
517519
):
518-
resp["headers"]["retry-after"] = resp["headers"]["Retry-After"]
520+
response_headers["retry-after"] = response_headers["Retry-After"]
519521
if (
520-
"Retry-After" not in resp["headers"]
521-
and "retry-after" in resp["headers"]
522+
"Retry-After" not in response_headers
523+
and "retry-after" in response_headers
522524
):
523-
resp["headers"]["Retry-After"] = resp["headers"]["retry-after"]
525+
response_headers["Retry-After"] = response_headers["retry-after"]
524526

525527
# read the response body here
526528
charset = e.headers.get_content_charset() or "utf-8"

‎slack_sdk/webhook/client.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,13 @@ def _perform_http_request(
178178
# read the response body here
179179
charset = e.headers.get_content_charset() or "utf-8"
180180
response_body: str = e.read().decode(charset)
181+
# As adding new values to HTTPError#headers can be ignored, building a new dict object here
182+
response_headers = dict(e.headers.items())
181183
resp = WebhookResponse(
182184
url=url,
183185
status_code=e.code,
184186
body=response_body,
185-
headers=dict(e.headers.items()),
187+
headers=response_headers,
186188
)
187189
if e.code == 429:
188190
# for backward-compatibility with WebClient (v.2.5.0 or older)

‎tests/slack_sdk/scim/mock_web_api_server.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def _handle(self):
5757
return
5858
if "ratelimited" in pattern:
5959
self.send_response(429)
60-
self.send_header("Retry-After", 1)
60+
self.send_header("retry-after", 1)
6161
self.set_common_headers()
6262
self.wfile.write(
6363
"""{"ok": false, "error": "ratelimited"}""".encode("utf-8")

‎tests/slack_sdk/web/mock_web_api_server.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def _handle(self):
115115
return
116116
if "ratelimited" in pattern:
117117
self.send_response(429)
118-
self.send_header("Retry-After", 1)
118+
self.send_header("retry-after", 1)
119119
self.set_common_headers()
120120
self.wfile.write(
121121
"""{"ok": false, "error": "ratelimited"}""".encode("utf-8")
@@ -160,7 +160,7 @@ def _handle(self):
160160
):
161161
self.state["rate_limited_count"] += 1
162162
self.send_response(429)
163-
self.send_header("Retry-After", 1)
163+
self.send_header("retry-after", 1)
164164
self.send_header("content-type", "application/json;charset=utf-8")
165165
self.send_header("connection", "close")
166166
self.end_headers()

‎tests/slack_sdk/web/mock_web_api_server_http_retry.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ def _handle(self):
4343
return
4444
if pattern == "rate_limited":
4545
self.send_response(429)
46-
self.send_header("Retry-After", 1)
46+
self.send_header("retry-after", 1)
4747
self.set_common_headers()
4848
self.wfile.write(
4949
"""{"ok":false,"error":"rate_limited"}""".encode("utf-8")

‎tests/slack_sdk/web/test_web_client.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ def test_slack_api_rate_limiting_exception_returns_retry_after(self):
9191
except err.SlackApiError as slack_api_error:
9292
self.assertFalse(slack_api_error.response["ok"])
9393
self.assertEqual(429, slack_api_error.response.status_code)
94+
self.assertEqual(1, int(slack_api_error.response.headers["retry-after"]))
9495
self.assertEqual(1, int(slack_api_error.response.headers["Retry-After"]))
9596

9697
def test_the_api_call_files_argument_creates_the_expected_data(self):

0 commit comments

Comments
 (0)