Skip to content

Commit

Permalink
Fix #1496 Async client uses blocking call when uploading file with v2 (
Browse files Browse the repository at this point in the history
  • Loading branch information
seratch committed May 16, 2024
1 parent 694ec2f commit a1c0d3e
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 21 deletions.
5 changes: 5 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ def run(self):
" await self.files_getUploadURLExternal(",
async_source,
)
async_source = re.sub(
r" self._upload_file\(",
" await self._upload_file(",
async_source,
)
async_source = re.sub(
r" self.files_completeUploadExternal\(",
" await self.files_completeUploadExternal(",
Expand Down
26 changes: 26 additions & 0 deletions slack_sdk/web/async_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
) # type: ignore
from .async_slack_response import AsyncSlackResponse
from .deprecation import show_deprecation_warning_if_any
from .file_upload_v2_result import FileUploadV2Result
from .internal_utils import (
convert_bool_to_0_or_1,
_build_req_args,
Expand Down Expand Up @@ -214,3 +215,28 @@ async def _request(self, *, http_verb, api_url, req_args) -> Dict[str, Any]:
req_args=req_args,
retry_handlers=self.retry_handlers,
)

async def _upload_file(
self,
*,
url: str,
data: bytes,
logger: logging.Logger,
timeout: int,
proxy: Optional[str],
ssl: Optional[SSLContext],
) -> FileUploadV2Result:
"""Upload a file using the issued upload URL"""
result = await _request_with_session(
current_session=self.session,
timeout=timeout,
logger=logger,
http_verb="POST",
api_url=url,
req_args={"data": data, "proxy": proxy, "ssl": ssl},
retry_handlers=self.retry_handlers,
)
return FileUploadV2Result(
status=result.get("status_code"),
body=result.get("body"),
)
9 changes: 4 additions & 5 deletions slack_sdk/web/async_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
_warn_if_text_or_attachment_fallback_is_missing,
_remove_none_values,
_to_v2_file_upload_item,
_upload_file_via_v2_url,
_validate_for_legacy_client,
_print_files_upload_v2_suggestion,
)
Expand Down Expand Up @@ -3584,17 +3583,17 @@ async def files_upload_v2(

# step2: "https://files.slack.com/upload/v1/..." per file
for f in files:
upload_result = _upload_file_via_v2_url(
upload_result = await self._upload_file(
url=f["upload_url"],
data=f["data"],
logger=self._logger,
timeout=self.timeout,
proxy=self.proxy,
ssl=self.ssl,
)
if upload_result.get("status") != 200:
status = upload_result.get("status")
body = upload_result.get("body")
if upload_result.status != 200:
status = upload_result.status
body = upload_result.body
message = (
"Failed to upload a file "
f"(status: {status}, body: {body}, filename: {f.get('filename')}, title: {f.get('title')})"
Expand Down
14 changes: 12 additions & 2 deletions slack_sdk/web/async_internal_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def convert_params(values: dict) -> dict:

try:
async with session.request(http_verb, api_url, **req_args) as res:
data: Union[dict, bytes] = {}
data: Union[dict, bytes, str] = {}
if res.content_type == "application/gzip":
# admin.analytics.getFile
data = await res.read()
Expand All @@ -117,6 +117,14 @@ def convert_params(values: dict) -> dict:
headers=res.headers,
data=data,
)
elif res.content_type == "text/plain":
# https://files.slack.com/upload/v1/...
data = await res.text()
retry_response = RetryHttpResponse(
status_code=res.status,
headers=res.headers,
data=data,
)
else:
try:
data = await res.json()
Expand All @@ -143,7 +151,9 @@ def convert_params(values: dict) -> dict:
)

if logger.level <= logging.DEBUG:
body = data if isinstance(data, dict) else "(binary)"
body = "(binary)"
if isinstance(data, dict) or isinstance(data, str):
body = data
logger.debug(
"Received the following response - "
f"status: {res.status}, "
Expand Down
28 changes: 27 additions & 1 deletion slack_sdk/web/base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@

from slack_sdk.errors import SlackRequestError
from .deprecation import show_deprecation_warning_if_any
from .file_upload_v2_result import FileUploadV2Result
from .internal_utils import (
convert_bool_to_0_or_1,
get_user_agent,
_get_url,
_build_req_args,
_build_unexpected_body_error_message,
_upload_file_via_v2_url,
)
from .slack_response import SlackResponse
from slack_sdk.http_retry import default_retry_handlers
Expand Down Expand Up @@ -560,10 +562,34 @@ def _build_urllib_request_headers(
if has_json:
headers.update({"Content-Type": "application/json;charset=utf-8"})
if has_files:
# will be set afterwards
# will be set afterward
headers.pop("Content-Type", None)
return headers

def _upload_file(
self,
*,
url: str,
data: bytes,
logger: logging.Logger,
timeout: int,
proxy: Optional[str],
ssl: Optional[SSLContext],
) -> FileUploadV2Result:
"""Upload a file using the issued upload URL"""
result = _upload_file_via_v2_url(
url=url,
data=data,
logger=logger,
timeout=timeout,
proxy=proxy,
ssl=ssl,
)
return FileUploadV2Result(
status=result.get("status"),
body=result.get("body"),
)

# =================================================================

@staticmethod
Expand Down
9 changes: 4 additions & 5 deletions slack_sdk/web/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
_warn_if_text_or_attachment_fallback_is_missing,
_remove_none_values,
_to_v2_file_upload_item,
_upload_file_via_v2_url,
_validate_for_legacy_client,
_print_files_upload_v2_suggestion,
)
Expand Down Expand Up @@ -3575,17 +3574,17 @@ def files_upload_v2(

# step2: "https://files.slack.com/upload/v1/..." per file
for f in files:
upload_result = _upload_file_via_v2_url(
upload_result = self._upload_file(
url=f["upload_url"],
data=f["data"],
logger=self._logger,
timeout=self.timeout,
proxy=self.proxy,
ssl=self.ssl,
)
if upload_result.get("status") != 200:
status = upload_result.get("status")
body = upload_result.get("body")
if upload_result.status != 200:
status = upload_result.status
body = upload_result.body
message = (
"Failed to upload a file "
f"(status: {status}, body: {body}, filename: {f.get('filename')}, title: {f.get('title')})"
Expand Down
7 changes: 7 additions & 0 deletions slack_sdk/web/file_upload_v2_result.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
class FileUploadV2Result:
status: int
body: str

def __init__(self, status: int, body: str):
self.status = status
self.body = body
9 changes: 7 additions & 2 deletions slack_sdk/web/internal_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,9 @@ def _upload_file_via_v2_url(
else:
raise SlackRequestError(f"Invalid proxy detected: {proxy} must be a str value")

if logger.level <= logging.DEBUG:
logger.debug(f"Sending a request: POST {url}")

resp: Optional[HTTPResponse] = None
req: Request = Request(method="POST", url=url, data=data, headers={})
if opener:
Expand All @@ -388,8 +391,10 @@ def _upload_file_via_v2_url(
body: str = resp.read().decode(charset) # read the response body here
if logger.level <= logging.DEBUG:
message = (
"Received the following response - ",
f"status: {resp.status}, " f"headers: {dict(resp.headers)}, " f"body: {body}",
"Received the following response - "
f"status: {resp.status}, "
f"headers: {dict(resp.headers)}, "
f"body: {body}"
)
logger.debug(message)

Expand Down
27 changes: 26 additions & 1 deletion slack_sdk/web/legacy_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@
from slack_sdk.errors import SlackRequestError
from .async_internal_utils import _files_to_data, _get_event_loop, _request_with_session
from .deprecation import show_deprecation_warning_if_any
from .file_upload_v2_result import FileUploadV2Result
from .internal_utils import (
convert_bool_to_0_or_1,
get_user_agent,
_get_url,
_build_req_args,
_build_unexpected_body_error_message,
_upload_file_via_v2_url,
)
from .legacy_slack_response import LegacySlackResponse as SlackResponse
from ..proxy_env_variable_loader import load_http_proxy_from_env
Expand Down Expand Up @@ -521,10 +523,33 @@ def _build_urllib_request_headers(
if has_json:
headers.update({"Content-Type": "application/json;charset=utf-8"})
if has_files:
# will be set afterwards
# will be set afterward
headers.pop("Content-Type", None)
return headers

def _upload_file(
self,
*,
url: str,
data: bytes,
logger: logging.Logger,
timeout: int,
proxy: Optional[str],
ssl: Optional[SSLContext],
) -> FileUploadV2Result:
result = _upload_file_via_v2_url(
url=url,
data=data,
logger=logger,
timeout=timeout,
proxy=proxy,
ssl=ssl,
)
return FileUploadV2Result(
status=result.get("status"),
body=result.get("body"),
)

# =================================================================

@staticmethod
Expand Down
9 changes: 4 additions & 5 deletions slack_sdk/web/legacy_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
_warn_if_text_or_attachment_fallback_is_missing,
_remove_none_values,
_to_v2_file_upload_item,
_upload_file_via_v2_url,
_validate_for_legacy_client,
_print_files_upload_v2_suggestion,
)
Expand Down Expand Up @@ -3586,17 +3585,17 @@ def files_upload_v2(

# step2: "https://files.slack.com/upload/v1/..." per file
for f in files:
upload_result = _upload_file_via_v2_url(
upload_result = self._upload_file(
url=f["upload_url"],
data=f["data"],
logger=self._logger,
timeout=self.timeout,
proxy=self.proxy,
ssl=self.ssl,
)
if upload_result.get("status") != 200:
status = upload_result.get("status")
body = upload_result.get("body")
if upload_result.status != 200:
status = upload_result.status
body = upload_result.body
message = (
"Failed to upload a file "
f"(status: {status}, body: {body}, filename: {f.get('filename')}, title: {f.get('title')})"
Expand Down

0 comments on commit a1c0d3e

Please sign in to comment.