Fix Slack file upload (#135818)

* pgrade Slack integration to use AsyncWebClient and support files_upload_v2

- Replaced deprecated WebClient with AsyncWebClient throughout the integration.
- Removed the unsupported `run_async` parameter.
- Added a helper function to resolve channel names to channel IDs.
- Updated `_async_send_local_file_message` and `_async_send_remote_file_message` to handle Slack's new API requirements, including per-channel uploads.
- Updated dependency from slackclient==2.5.0 to slack-sdk>=3.0.0.
- Improved error handling and logging for channel resolution and file uploads.

* Fix test to use AsyncWebClient for Slack authentication flow

* Fix Slack authentication URL by removing the www subdomain

* Refactor Slack file upload functionality and add utility for file uploads
This commit is contained in:
jsuar 2025-01-19 15:09:04 -05:00 committed by GitHub
parent a69786f64f
commit a2d76cac5a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 166 additions and 60 deletions

View File

@ -5,8 +5,8 @@ from __future__ import annotations
import logging import logging
from aiohttp.client_exceptions import ClientError from aiohttp.client_exceptions import ClientError
from slack import WebClient
from slack.errors import SlackApiError from slack.errors import SlackApiError
from slack_sdk.web.async_client import AsyncWebClient
from homeassistant.config_entries import ConfigEntry from homeassistant.config_entries import ConfigEntry
from homeassistant.const import CONF_API_KEY, Platform from homeassistant.const import CONF_API_KEY, Platform
@ -40,7 +40,9 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set up Slack from a config entry.""" """Set up Slack from a config entry."""
session = aiohttp_client.async_get_clientsession(hass) session = aiohttp_client.async_get_clientsession(hass)
slack = WebClient(token=entry.data[CONF_API_KEY], run_async=True, session=session) slack = AsyncWebClient(
token=entry.data[CONF_API_KEY], session=session
) # No run_async
try: try:
res = await slack.auth_test() res = await slack.auth_test()
@ -49,6 +51,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
_LOGGER.error("Invalid API key") _LOGGER.error("Invalid API key")
return False return False
raise ConfigEntryNotReady("Error while setting up integration") from ex raise ConfigEntryNotReady("Error while setting up integration") from ex
data = { data = {
DATA_CLIENT: slack, DATA_CLIENT: slack,
ATTR_URL: res[ATTR_URL], ATTR_URL: res[ATTR_URL],

View File

@ -4,8 +4,8 @@ from __future__ import annotations
import logging import logging
from slack import WebClient
from slack.errors import SlackApiError from slack.errors import SlackApiError
from slack_sdk.web.async_client import AsyncSlackResponse, AsyncWebClient
import voluptuous as vol import voluptuous as vol
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
@ -57,10 +57,10 @@ class SlackFlowHandler(ConfigFlow, domain=DOMAIN):
async def _async_try_connect( async def _async_try_connect(
self, token: str self, token: str
) -> tuple[str, None] | tuple[None, dict[str, str]]: ) -> tuple[str, None] | tuple[None, AsyncSlackResponse]:
"""Try connecting to Slack.""" """Try connecting to Slack."""
session = aiohttp_client.async_get_clientsession(self.hass) session = aiohttp_client.async_get_clientsession(self.hass)
client = WebClient(token=token, run_async=True, session=session) client = AsyncWebClient(token=token, session=session) # No run_async
try: try:
info = await client.auth_test() info = await client.auth_test()

View File

@ -2,7 +2,7 @@
from __future__ import annotations from __future__ import annotations
from slack import WebClient from slack_sdk.web.async_client import AsyncWebClient
from homeassistant.config_entries import ConfigEntry from homeassistant.config_entries import ConfigEntry
from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
@ -14,21 +14,18 @@ from .const import ATTR_URL, ATTR_USER_ID, DATA_CLIENT, DEFAULT_NAME, DOMAIN
class SlackEntity(Entity): class SlackEntity(Entity):
"""Representation of a Slack entity.""" """Representation of a Slack entity."""
_attr_attribution = "Data provided by Slack"
_attr_has_entity_name = True
def __init__( def __init__(
self, self,
data: dict[str, str | WebClient], data: dict[str, AsyncWebClient],
description: EntityDescription, description: EntityDescription,
entry: ConfigEntry, entry: ConfigEntry,
) -> None: ) -> None:
"""Initialize a Slack entity.""" """Initialize a Slack entity."""
self._client = data[DATA_CLIENT] self._client: AsyncWebClient = data[DATA_CLIENT]
self.entity_description = description self.entity_description = description
self._attr_unique_id = f"{data[ATTR_USER_ID]}_{description.key}" self._attr_unique_id = f"{data[ATTR_USER_ID]}_{description.key}"
self._attr_device_info = DeviceInfo( self._attr_device_info = DeviceInfo(
configuration_url=data[ATTR_URL], configuration_url=str(data[ATTR_URL]),
entry_type=DeviceEntryType.SERVICE, entry_type=DeviceEntryType.SERVICE,
identifiers={(DOMAIN, entry.entry_id)}, identifiers={(DOMAIN, entry.entry_id)},
manufacturer=DEFAULT_NAME, manufacturer=DEFAULT_NAME,

View File

@ -7,5 +7,5 @@
"integration_type": "service", "integration_type": "service",
"iot_class": "cloud_push", "iot_class": "cloud_push",
"loggers": ["slack"], "loggers": ["slack"],
"requirements": ["slackclient==2.5.0"] "requirements": ["slack_sdk==3.33.4"]
} }

View File

@ -5,13 +5,13 @@ from __future__ import annotations
import asyncio import asyncio
import logging import logging
import os import os
from typing import Any, TypedDict from typing import Any, TypedDict, cast
from urllib.parse import urlparse from urllib.parse import urlparse
from aiohttp import BasicAuth, FormData from aiohttp import BasicAuth
from aiohttp.client_exceptions import ClientError from aiohttp.client_exceptions import ClientError
from slack import WebClient
from slack.errors import SlackApiError from slack.errors import SlackApiError
from slack_sdk.web.async_client import AsyncWebClient
import voluptuous as vol import voluptuous as vol
from homeassistant.components.notify import ( from homeassistant.components.notify import (
@ -38,6 +38,7 @@ from .const import (
DATA_CLIENT, DATA_CLIENT,
SLACK_DATA, SLACK_DATA,
) )
from .utils import upload_file_to_slack
_LOGGER = logging.getLogger(__name__) _LOGGER = logging.getLogger(__name__)
@ -136,7 +137,7 @@ class SlackNotificationService(BaseNotificationService):
def __init__( def __init__(
self, self,
hass: HomeAssistant, hass: HomeAssistant,
client: WebClient, client: AsyncWebClient,
config: dict[str, str], config: dict[str, str],
) -> None: ) -> None:
"""Initialize.""" """Initialize."""
@ -160,17 +161,23 @@ class SlackNotificationService(BaseNotificationService):
parsed_url = urlparse(path) parsed_url = urlparse(path)
filename = os.path.basename(parsed_url.path) filename = os.path.basename(parsed_url.path)
try: channel_ids = [await self._async_get_channel_id(target) for target in targets]
await self._client.files_upload( channel_ids = [cid for cid in channel_ids if cid] # Remove None values
channels=",".join(targets),
file=path, if not channel_ids:
filename=filename, _LOGGER.error("No valid channel IDs resolved for targets: %s", targets)
initial_comment=message, return
title=title or filename,
thread_ts=thread_ts or "", await upload_file_to_slack(
) client=self._client,
except (SlackApiError, ClientError) as err: channel_ids=channel_ids,
_LOGGER.error("Error while uploading file-based message: %r", err) file_content=None,
file_path=path,
filename=filename,
title=title,
message=message,
thread_ts=thread_ts,
)
async def _async_send_remote_file_message( async def _async_send_remote_file_message(
self, self,
@ -183,12 +190,7 @@ class SlackNotificationService(BaseNotificationService):
username: str | None = None, username: str | None = None,
password: str | None = None, password: str | None = None,
) -> None: ) -> None:
"""Upload a remote file (with message) to Slack. """Upload a remote file (with message) to Slack."""
Note that we bypass the python-slackclient WebClient and use aiohttp directly,
as the former would require us to download the entire remote file into memory
first before uploading it to Slack.
"""
if not self._hass.config.is_allowed_external_url(url): if not self._hass.config.is_allowed_external_url(url):
_LOGGER.error("URL is not allowed: %s", url) _LOGGER.error("URL is not allowed: %s", url)
return return
@ -196,36 +198,35 @@ class SlackNotificationService(BaseNotificationService):
filename = _async_get_filename_from_url(url) filename = _async_get_filename_from_url(url)
session = aiohttp_client.async_get_clientsession(self._hass) session = aiohttp_client.async_get_clientsession(self._hass)
# Fetch the remote file
kwargs: AuthDictT = {} kwargs: AuthDictT = {}
if username and password is not None: if username and password:
kwargs = {"auth": BasicAuth(username, password=password)} kwargs = {"auth": BasicAuth(username, password=password)}
resp = await session.request("get", url, **kwargs)
try: try:
resp.raise_for_status() async with session.get(url, **kwargs) as resp:
resp.raise_for_status()
file_content = await resp.read()
except ClientError as err: except ClientError as err:
_LOGGER.error("Error while retrieving %s: %r", url, err) _LOGGER.error("Error while retrieving %s: %r", url, err)
return return
form_data: FormDataT = { channel_ids = [await self._async_get_channel_id(target) for target in targets]
"channels": ",".join(targets), channel_ids = [cid for cid in channel_ids if cid] # Remove None values
"filename": filename,
"initial_comment": message,
"title": title or filename,
"token": self._client.token,
}
if thread_ts: if not channel_ids:
form_data["thread_ts"] = thread_ts _LOGGER.error("No valid channel IDs resolved for targets: %s", targets)
return
data = FormData(form_data, charset="utf-8") await upload_file_to_slack(
data.add_field("file", resp.content, filename=filename) client=self._client,
channel_ids=channel_ids,
try: file_content=file_content,
await session.post("https://slack.com/api/files.upload", data=data) filename=filename,
except ClientError as err: title=title,
_LOGGER.error("Error while uploading file message: %r", err) message=message,
thread_ts=thread_ts,
)
async def _async_send_text_only_message( async def _async_send_text_only_message(
self, self,
@ -327,3 +328,46 @@ class SlackNotificationService(BaseNotificationService):
title, title,
thread_ts=data.get(ATTR_THREAD_TS), thread_ts=data.get(ATTR_THREAD_TS),
) )
async def _async_get_channel_id(self, channel_name: str) -> str | None:
"""Get the Slack channel ID from the channel name.
This method retrieves the channel ID for a given Slack channel name by
querying the Slack API. It handles both public and private channels.
Including this so users can provide channel names instead of IDs.
Args:
channel_name (str): The name of the Slack channel.
Returns:
str | None: The ID of the Slack channel if found, otherwise None.
Raises:
SlackApiError: If there is an error while communicating with the Slack API.
"""
try:
# Remove # if present
channel_name = channel_name.lstrip("#")
# Get channel list
# Multiple types is not working. Tested here: https://api.slack.com/methods/conversations.list/test
# response = await self._client.conversations_list(types="public_channel,private_channel")
#
# Workaround for the types parameter not working
channels = []
for channel_type in ("public_channel", "private_channel"):
response = await self._client.conversations_list(types=channel_type)
channels.extend(response["channels"])
# Find channel ID
for channel in channels:
if channel["name"] == channel_name:
return cast(str, channel["id"])
_LOGGER.error("Channel %s not found", channel_name)
except SlackApiError as err:
_LOGGER.error("Error getting channel ID: %r", err)
return None

View File

@ -2,7 +2,7 @@
from __future__ import annotations from __future__ import annotations
from slack import WebClient from slack_sdk.web.async_client import AsyncWebClient
from homeassistant.components.sensor import ( from homeassistant.components.sensor import (
SensorDeviceClass, SensorDeviceClass,
@ -43,7 +43,7 @@ async def async_setup_entry(
class SlackSensorEntity(SlackEntity, SensorEntity): class SlackSensorEntity(SlackEntity, SensorEntity):
"""Representation of a Slack sensor.""" """Representation of a Slack sensor."""
_client: WebClient _client: AsyncWebClient
async def async_update(self) -> None: async def async_update(self) -> None:
"""Get the latest status.""" """Get the latest status."""

View File

@ -0,0 +1,62 @@
"""Utils for the Slack integration."""
import logging
import aiofiles
from slack_sdk.errors import SlackApiError
from slack_sdk.web.async_client import AsyncWebClient
_LOGGER = logging.getLogger(__name__)
async def upload_file_to_slack(
client: AsyncWebClient,
channel_ids: list[str | None],
file_content: bytes | str | None,
filename: str,
title: str | None,
message: str,
thread_ts: str | None,
file_path: str | None = None, # Allow passing a file path
) -> None:
"""Upload a file to Slack for the specified channel IDs.
Args:
client (AsyncWebClient): The Slack WebClient instance.
channel_ids (list[str | None]): List of channel IDs to upload the file to.
file_content (Union[bytes, str, None]): Content of the file (local or remote). If None, file_path is used.
filename (str): The file's name.
title (str | None): Title of the file in Slack.
message (str): Initial comment to accompany the file.
thread_ts (str | None): Thread timestamp for threading messages.
file_path (str | None): Path to the local file to be read if file_content is None.
Raises:
SlackApiError: If the Slack API call fails.
OSError: If there is an error reading the file.
"""
if file_content is None and file_path:
# Read file asynchronously if file_content is not provided
try:
async with aiofiles.open(file_path, "rb") as file:
file_content = await file.read()
except OSError as os_err:
_LOGGER.error("Error reading file %s: %r", file_path, os_err)
return
for channel_id in channel_ids:
try:
await client.files_upload_v2(
channel=channel_id,
file=file_content,
filename=filename,
title=title or filename,
initial_comment=message,
thread_ts=thread_ts or "",
)
_LOGGER.info("Successfully uploaded file to channel %s", channel_id)
except SlackApiError as err:
_LOGGER.error(
"Error while uploading file to channel %s: %r", channel_id, err
)

2
requirements_all.txt generated
View File

@ -2720,7 +2720,7 @@ sisyphus-control==3.1.4
skyboxremote==0.0.6 skyboxremote==0.0.6
# homeassistant.components.slack # homeassistant.components.slack
slackclient==2.5.0 slack_sdk==3.33.4
# homeassistant.components.xmpp # homeassistant.components.xmpp
slixmpp==1.8.5 slixmpp==1.8.5

View File

@ -2190,7 +2190,7 @@ simplisafe-python==2024.01.0
skyboxremote==0.0.6 skyboxremote==0.0.6
# homeassistant.components.slack # homeassistant.components.slack
slackclient==2.5.0 slack_sdk==3.33.4
# homeassistant.components.smart_meter_texas # homeassistant.components.smart_meter_texas
smart-meter-texas==0.5.5 smart-meter-texas==0.5.5

View File

@ -12,7 +12,7 @@ from homeassistant.core import HomeAssistant
from tests.common import MockConfigEntry, load_fixture from tests.common import MockConfigEntry, load_fixture
from tests.test_util.aiohttp import AiohttpClientMocker from tests.test_util.aiohttp import AiohttpClientMocker
AUTH_URL = "https://www.slack.com/api/auth.test" AUTH_URL = "https://slack.com/api/auth.test"
TOKEN = "abc123" TOKEN = "abc123"
TEAM_NAME = "Test Team" TEAM_NAME = "Test Team"

View File

@ -81,7 +81,7 @@ async def test_flow_user_cannot_connect(
async def test_flow_user_unknown_error(hass: HomeAssistant) -> None: async def test_flow_user_unknown_error(hass: HomeAssistant) -> None:
"""Test user initialized flow with unreachable server.""" """Test user initialized flow with unreachable server."""
with patch( with patch(
"homeassistant.components.slack.config_flow.WebClient.auth_test" "homeassistant.components.slack.config_flow.AsyncWebClient.auth_test"
) as mock: ) as mock:
mock.side_effect = Exception mock.side_effect = Exception
result = await hass.config_entries.flow.async_init( result = await hass.config_entries.flow.async_init(