From 207cdb289482fa2b4277ab824974cfc44672c3d2 Mon Sep 17 00:00:00 2001 From: codomposer Date: Thu, 20 Nov 2025 17:27:43 -0500 Subject: [PATCH 1/2] fix handling markdown blocks in files_upload_v2 function --- slack_sdk/web/async_client.py | 14 +++++ slack_sdk/web/client.py | 14 +++++ slack_sdk/web/legacy_client.py | 15 +++++- tests/slack_sdk/web/test_internal_utils.py | 60 +++++++++++++++++++++- 4 files changed, 101 insertions(+), 2 deletions(-) diff --git a/slack_sdk/web/async_client.py b/slack_sdk/web/async_client.py index ca163da98..aff33bf7a 100644 --- a/slack_sdk/web/async_client.py +++ b/slack_sdk/web/async_client.py @@ -4121,6 +4121,20 @@ async def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) + + # Parse Block objects and serialize blocks/attachments to JSON + _parse_web_class_objects(kwargs) + + # Serialize blocks to JSON if present and not already a string + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + # Serialize attachments to JSON if present and not already a string + attachments = kwargs.get("attachments") + if attachments is not None and not isinstance(attachments, str): + kwargs["attachments"] = json.dumps(attachments) + return await self.api_call("files.completeUploadExternal", params=kwargs) async def functions_completeSuccess( diff --git a/slack_sdk/web/client.py b/slack_sdk/web/client.py index dfa771832..5ad90240c 100644 --- a/slack_sdk/web/client.py +++ b/slack_sdk/web/client.py @@ -4111,6 +4111,20 @@ def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) + + # Parse Block objects and serialize blocks/attachments to JSON + _parse_web_class_objects(kwargs) + + # Serialize blocks to JSON if present and not already a string + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + # Serialize attachments to JSON if present and not already a string + attachments = kwargs.get("attachments") + if attachments is not None and not isinstance(attachments, str): + kwargs["attachments"] = json.dumps(attachments) + return self.api_call("files.completeUploadExternal", params=kwargs) def functions_completeSuccess( diff --git a/slack_sdk/web/legacy_client.py b/slack_sdk/web/legacy_client.py index df2bcc370..1d1b4475d 100644 --- a/slack_sdk/web/legacy_client.py +++ b/slack_sdk/web/legacy_client.py @@ -9,7 +9,6 @@ # !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! from asyncio import Future - """A Python module for interacting with Slack's Web API.""" import json @@ -4059,6 +4058,20 @@ def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) + + # Parse Block objects and serialize blocks/attachments to JSON + _parse_web_class_objects(kwargs) + + # Serialize blocks to JSON if present and not already a string + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + # Serialize attachments to JSON if present and not already a string + attachments = kwargs.get("attachments") + if attachments is not None and not isinstance(attachments, str): + kwargs["attachments"] = json.dumps(attachments) + return self.api_call("files.completeUploadExternal", params=kwargs) def functions_completeSuccess( diff --git a/tests/slack_sdk/web/test_internal_utils.py b/tests/slack_sdk/web/test_internal_utils.py index ac7704b30..ee18849f9 100644 --- a/tests/slack_sdk/web/test_internal_utils.py +++ b/tests/slack_sdk/web/test_internal_utils.py @@ -7,7 +7,7 @@ import pytest from slack_sdk.models.attachments import Attachment -from slack_sdk.models.blocks import Block, DividerBlock +from slack_sdk.models.blocks import Block, DividerBlock, MarkdownBlock from slack_sdk.web.internal_utils import ( _build_unexpected_body_error_message, _parse_web_class_objects, @@ -134,3 +134,61 @@ def test_get_url_prevent_double_slash(self): "https://slack.com/api/chat.postMessage", "Should correctly handle api_method without a leading slash", ) + + def test_files_complete_upload_external_blocks_serialization(self): + """Test that blocks are properly serialized for files.completeUploadExternal + + This test verifies the fix for the bug where passing markdown blocks to + files_upload_v2 caused internal_error. The issue was that blocks weren't + being properly serialized to JSON strings before being sent to the API. + """ + # Test case 1: blocks as list of dicts (the bug scenario) + blocks_dict = [{"type": "markdown", "text": "_**User** posted a message_\\n> test"}] + kwargs = {"blocks": blocks_dict} + + # Simulate what files_completeUploadExternal does + _parse_web_class_objects(kwargs) + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + assert isinstance(kwargs["blocks"], str), "Blocks should be serialized to string" + assert json.loads(kwargs["blocks"]) == blocks_dict, "Serialized blocks should match original" + + # Test case 2: blocks as MarkdownBlock objects + markdown_block = MarkdownBlock(text="_**User** posted a message_\\n> test") + blocks_objects = [markdown_block] + kwargs = {"blocks": blocks_objects} + + _parse_web_class_objects(kwargs) + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + assert isinstance(kwargs["blocks"], str), "Blocks should be serialized to string" + deserialized = json.loads(kwargs["blocks"]) + assert deserialized[0]["type"] == "markdown", "Block type should be markdown" + assert deserialized[0]["text"] == "_**User** posted a message_\\n> test", "Block text should match" + + # Test case 3: blocks already as JSON string (should not double-serialize) + blocks_json = json.dumps([{"type": "markdown", "text": "test"}]) + kwargs = {"blocks": blocks_json} + + _parse_web_class_objects(kwargs) + blocks = kwargs.get("blocks") + if blocks is not None and not isinstance(blocks, str): + kwargs["blocks"] = json.dumps(blocks) + + assert kwargs["blocks"] == blocks_json, "Already serialized blocks should not be double-serialized" + + # Test case 4: attachments should also be serialized + attachments = [{"text": "attachment text", "color": "#36a64f"}] + kwargs = {"attachments": attachments} + + _parse_web_class_objects(kwargs) + attachments_val = kwargs.get("attachments") + if attachments_val is not None and not isinstance(attachments_val, str): + kwargs["attachments"] = json.dumps(attachments_val) + + assert isinstance(kwargs["attachments"], str), "Attachments should be serialized to string" + assert json.loads(kwargs["attachments"]) == attachments, "Serialized attachments should match original" From 8e199c210edd7daed7dc3c567a8e7b72da49436c Mon Sep 17 00:00:00 2001 From: codomposer Date: Thu, 20 Nov 2025 20:00:26 -0500 Subject: [PATCH 2/2] use kwargs --- slack_sdk/web/async_client.py | 16 ++-------------- slack_sdk/web/client.py | 16 ++-------------- slack_sdk/web/legacy_client.py | 16 ++-------------- 3 files changed, 6 insertions(+), 42 deletions(-) diff --git a/slack_sdk/web/async_client.py b/slack_sdk/web/async_client.py index aff33bf7a..7b9d0f382 100644 --- a/slack_sdk/web/async_client.py +++ b/slack_sdk/web/async_client.py @@ -4121,21 +4121,9 @@ async def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) - - # Parse Block objects and serialize blocks/attachments to JSON _parse_web_class_objects(kwargs) - - # Serialize blocks to JSON if present and not already a string - blocks = kwargs.get("blocks") - if blocks is not None and not isinstance(blocks, str): - kwargs["blocks"] = json.dumps(blocks) - - # Serialize attachments to JSON if present and not already a string - attachments = kwargs.get("attachments") - if attachments is not None and not isinstance(attachments, str): - kwargs["attachments"] = json.dumps(attachments) - - return await self.api_call("files.completeUploadExternal", params=kwargs) + kwargs = _remove_none_values(kwargs) + return await self.api_call("files.completeUploadExternal", json=kwargs) async def functions_completeSuccess( self, diff --git a/slack_sdk/web/client.py b/slack_sdk/web/client.py index 5ad90240c..0f429b240 100644 --- a/slack_sdk/web/client.py +++ b/slack_sdk/web/client.py @@ -4111,21 +4111,9 @@ def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) - - # Parse Block objects and serialize blocks/attachments to JSON _parse_web_class_objects(kwargs) - - # Serialize blocks to JSON if present and not already a string - blocks = kwargs.get("blocks") - if blocks is not None and not isinstance(blocks, str): - kwargs["blocks"] = json.dumps(blocks) - - # Serialize attachments to JSON if present and not already a string - attachments = kwargs.get("attachments") - if attachments is not None and not isinstance(attachments, str): - kwargs["attachments"] = json.dumps(attachments) - - return self.api_call("files.completeUploadExternal", params=kwargs) + kwargs = _remove_none_values(kwargs) + return self.api_call("files.completeUploadExternal", json=kwargs) def functions_completeSuccess( self, diff --git a/slack_sdk/web/legacy_client.py b/slack_sdk/web/legacy_client.py index 1d1b4475d..aa19c78a7 100644 --- a/slack_sdk/web/legacy_client.py +++ b/slack_sdk/web/legacy_client.py @@ -4058,21 +4058,9 @@ def files_completeUploadExternal( ) if channels: kwargs["channels"] = ",".join(channels) - - # Parse Block objects and serialize blocks/attachments to JSON _parse_web_class_objects(kwargs) - - # Serialize blocks to JSON if present and not already a string - blocks = kwargs.get("blocks") - if blocks is not None and not isinstance(blocks, str): - kwargs["blocks"] = json.dumps(blocks) - - # Serialize attachments to JSON if present and not already a string - attachments = kwargs.get("attachments") - if attachments is not None and not isinstance(attachments, str): - kwargs["attachments"] = json.dumps(attachments) - - return self.api_call("files.completeUploadExternal", params=kwargs) + kwargs = _remove_none_values(kwargs) + return self.api_call("files.completeUploadExternal", json=kwargs) def functions_completeSuccess( self,