fix(discord): preflight attachment size before upload
Reject oversized local attachments before channel.send(file=...) so users get an explicit notice instead of a doomed 413 round-trip. Fixes #50846
This commit is contained in:
@@ -170,6 +170,10 @@ _DISCORD_SELECT_MAX_ROWS = 5
|
||||
# Model-select capacity: keep 2 rows for Back/Cancel, fill the rest with selects.
|
||||
_DISCORD_MODEL_SELECT_CAPACITY = (_DISCORD_SELECT_MAX_ROWS - 2) * _DISCORD_SELECT_MAX_OPTIONS
|
||||
_DISCORD_BUTTON_LABEL_LIMIT = 80
|
||||
# Default Discord attachment cap for DMs / channels without a guild boost
|
||||
# context. Guild channels expose the effective limit via
|
||||
# ``guild.filesize_limit`` (boost tier may raise it). See issue #50846.
|
||||
_DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES = 25 * 1024 * 1024
|
||||
_DISCORD_ELLIPSIS = "\u2026"
|
||||
_DISCORD_NONCONVERSATIONAL_METADATA_KEYS = frozenset({
|
||||
"non_conversational", "non_conversational_history",
|
||||
@@ -3157,7 +3161,19 @@ class DiscordAdapter(DiscordMediaMixin, BasePlatformAdapter):
|
||||
success=True, message_id=last_id, continuation_message_ids=tuple(continuation_ids),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _discord_upload_limit_bytes(channel: Any) -> int:
|
||||
"""Return the effective Discord attachment size limit for *channel*.
|
||||
|
||||
Prefer the guild's boost-aware ``filesize_limit`` when present; fall
|
||||
back to the platform default for DMs / group DMs without a guild.
|
||||
"""
|
||||
guild = getattr(channel, "guild", None)
|
||||
if guild is not None:
|
||||
limit = getattr(guild, "filesize_limit", None)
|
||||
if isinstance(limit, int) and limit > 0:
|
||||
return limit
|
||||
return _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES
|
||||
|
||||
async def play_tts(self, chat_id: str, audio_path: str, **kwargs) -> SendResult:
|
||||
"""Play auto-TTS audio: in the guild's VC if joined, else as a file attachment."""
|
||||
|
||||
@@ -33,6 +33,35 @@ class DiscordMediaMixin:
|
||||
if not channel:
|
||||
return SendResult(success=False, error=f"Channel {chat_id} not found")
|
||||
filename = file_name or os.path.basename(file_path)
|
||||
try:
|
||||
file_size = os.path.getsize(file_path)
|
||||
except OSError as exc:
|
||||
return SendResult(success=False, error=f"Cannot stat file {filename}: {exc}")
|
||||
# Reject oversized files before upload (#50846): no doomed 413 round-trip,
|
||||
# and the user gets an explicit notice instead of a silent failure.
|
||||
limit = self._discord_upload_limit_bytes(channel)
|
||||
if file_size > limit:
|
||||
size_mb = file_size / (1024 * 1024)
|
||||
limit_mb = limit / (1024 * 1024)
|
||||
error = (
|
||||
f"File too large for Discord upload: {filename} is "
|
||||
f"{size_mb:.1f} MB (limit {limit_mb:.0f} MB)"
|
||||
)
|
||||
logger.warning("[%s] %s", self.name, error)
|
||||
notice = (
|
||||
f"⚠️ Could not attach `{filename}` — {size_mb:.1f} MB exceeds "
|
||||
f"Discord's {limit_mb:.0f} MB upload limit for this channel. "
|
||||
f"Compress the file or share a link instead."
|
||||
)
|
||||
try:
|
||||
if not self._is_forum_parent(channel):
|
||||
await channel.send(content=notice)
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"[%s] Failed to send oversized-file notice for %s",
|
||||
self.name, filename, exc_info=True,
|
||||
)
|
||||
return SendResult(success=False, error=error)
|
||||
logger.info(
|
||||
"[%s] Sending file attachment %s (%s) to %s", self.name, filename,
|
||||
os.path.splitext(filename)[1].lower() or "no-ext", chat_id,
|
||||
@@ -96,6 +125,7 @@ class DiscordMediaMixin:
|
||||
await asyncio.sleep(human_delay)
|
||||
files: List[Any] = []
|
||||
captions: List[str] = []
|
||||
skip_notices: List[str] = []
|
||||
aiohttp_session = None
|
||||
try:
|
||||
for image_url, alt_text in chunk:
|
||||
@@ -106,6 +136,30 @@ class DiscordMediaMixin:
|
||||
if not os.path.exists(local_path):
|
||||
logger.warning("[%s] Skipping missing image: %s", self.name, local_path)
|
||||
continue
|
||||
# Same preflight as _send_file_attachment (#50846): an oversized
|
||||
# local image would 413 the whole chunk and drop its siblings
|
||||
# into the fallback path.
|
||||
try:
|
||||
_img_size = os.path.getsize(local_path)
|
||||
except OSError as stat_err:
|
||||
logger.warning(
|
||||
"[%s] Skipping unreadable image %s: %s",
|
||||
self.name, local_path, stat_err,
|
||||
)
|
||||
continue
|
||||
_img_limit = self._discord_upload_limit_bytes(channel)
|
||||
if _img_size > _img_limit:
|
||||
logger.warning(
|
||||
"[%s] Skipping oversized image in batch: %s is %.1f MB (limit %.0f MB)",
|
||||
self.name, os.path.basename(local_path),
|
||||
_img_size / (1024 * 1024), _img_limit / (1024 * 1024),
|
||||
)
|
||||
skip_notices.append(
|
||||
f"⚠️ Skipped `{os.path.basename(local_path)}` — "
|
||||
f"{_img_size / (1024 * 1024):.1f} MB exceeds Discord's "
|
||||
f"{_img_limit / (1024 * 1024):.0f} MB upload limit."
|
||||
)
|
||||
continue
|
||||
files.append(_discord_mod.File(local_path, filename=os.path.basename(local_path)))
|
||||
else:
|
||||
if not is_safe_url(image_url):
|
||||
@@ -135,9 +189,21 @@ class DiscordMediaMixin:
|
||||
logger.warning("[%s] Download failed for %s: %s", self.name, image_url[:80], dl_err)
|
||||
continue
|
||||
if not files:
|
||||
# Everything in this chunk was skipped. Still surface any
|
||||
# oversized-file notices so the drop is not silent.
|
||||
if skip_notices and not self._is_forum_parent(channel):
|
||||
try:
|
||||
await channel.send(content="\n".join(skip_notices))
|
||||
except Exception:
|
||||
logger.debug(
|
||||
"[%s] Failed to send oversized-image notices",
|
||||
self.name, exc_info=True,
|
||||
)
|
||||
continue
|
||||
# Use the first caption if any (Discord only has one message body for the group)
|
||||
content = captions[0] if captions else None
|
||||
if skip_notices:
|
||||
content = "\n".join(([content] if content else []) + skip_notices)
|
||||
logger.info(
|
||||
"[%s] Sending %d image(s) as single Discord message (chunk %d/%d)",
|
||||
self.name, len(files), chunk_idx + 1, len(chunks),
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
@@ -418,3 +419,143 @@ async def test_send_file_attachment_forum_uses_files_kwarg(tmp_path, monkeypatch
|
||||
assert isinstance(thread_kwargs.get("files"), list) and len(thread_kwargs["files"]) == 1
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Upload-size preflight (#50846 / #52698)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_discord_upload_limit_uses_guild_filesize_limit():
|
||||
from plugins.platforms.discord.adapter import (
|
||||
DiscordAdapter,
|
||||
_DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES,
|
||||
)
|
||||
|
||||
guild_channel = SimpleNamespace(guild=SimpleNamespace(filesize_limit=50 * 1024 * 1024))
|
||||
dm_channel = SimpleNamespace(guild=None)
|
||||
no_limit_guild = SimpleNamespace(guild=SimpleNamespace(filesize_limit=0))
|
||||
|
||||
assert DiscordAdapter._discord_upload_limit_bytes(guild_channel) == 50 * 1024 * 1024
|
||||
assert DiscordAdapter._discord_upload_limit_bytes(dm_channel) == _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES
|
||||
assert DiscordAdapter._discord_upload_limit_bytes(no_limit_guild) == _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_file_attachment_rejects_oversized_before_upload(tmp_path):
|
||||
"""Oversized local files must not call channel.send(file=...) — issue #50846."""
|
||||
adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***"))
|
||||
adapter._is_forum_parent = lambda _ch: False # type: ignore[method-assign]
|
||||
|
||||
from plugins.platforms.discord.adapter import _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES
|
||||
|
||||
big = tmp_path / "clip.mp4"
|
||||
big.write_bytes(b"x")
|
||||
|
||||
send = AsyncMock(return_value=SimpleNamespace(id=999))
|
||||
channel = SimpleNamespace(id=555, guild=None, send=send)
|
||||
adapter._client = SimpleNamespace(
|
||||
get_channel=lambda _cid: channel,
|
||||
fetch_channel=AsyncMock(),
|
||||
)
|
||||
|
||||
original = os.path.getsize
|
||||
|
||||
def fake_getsize(path):
|
||||
if str(path) == str(big):
|
||||
return _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES + 1
|
||||
return original(path)
|
||||
|
||||
os.path.getsize = fake_getsize
|
||||
try:
|
||||
result = await adapter._send_file_attachment("555", str(big))
|
||||
finally:
|
||||
os.path.getsize = original
|
||||
|
||||
assert result.success is False
|
||||
assert "too large" in (result.error or "").lower()
|
||||
assert "clip.mp4" in (result.error or "")
|
||||
assert send.await_count == 1
|
||||
assert send.await_args is not None
|
||||
kwargs = send.await_args.kwargs
|
||||
assert "file" not in kwargs and "files" not in kwargs
|
||||
assert "Could not attach" in (kwargs.get("content") or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_video_respects_guild_filesize_limit(tmp_path):
|
||||
"""Guild boost limit is honored; files under the higher cap still upload."""
|
||||
adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***"))
|
||||
adapter._is_forum_parent = lambda _ch: False # type: ignore[method-assign]
|
||||
|
||||
video = tmp_path / "ok.mp4"
|
||||
video.write_bytes(b"fake-video-bytes")
|
||||
|
||||
sent_msg = SimpleNamespace(
|
||||
id=42,
|
||||
attachments=[SimpleNamespace(filename="ok.mp4", url="https://cdn.example/ok.mp4")],
|
||||
)
|
||||
send = AsyncMock(return_value=sent_msg)
|
||||
channel = SimpleNamespace(
|
||||
id=777,
|
||||
guild=SimpleNamespace(filesize_limit=50 * 1024 * 1024),
|
||||
send=send,
|
||||
)
|
||||
adapter._client = SimpleNamespace(
|
||||
get_channel=lambda _cid: channel,
|
||||
fetch_channel=AsyncMock(),
|
||||
)
|
||||
|
||||
result = await adapter.send_video("777", str(video))
|
||||
assert result.success is True
|
||||
assert result.message_id == "42"
|
||||
assert send.await_count == 1
|
||||
assert send.await_args is not None
|
||||
kwargs = send.await_args.kwargs
|
||||
assert kwargs.get("file") is not None or kwargs.get("files")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_send_video_oversized_skips_base_fallback(tmp_path, monkeypatch):
|
||||
"""Oversized send_video returns failure without falling back to base adapter."""
|
||||
adapter = DiscordAdapter(PlatformConfig(enabled=True, token="***"))
|
||||
adapter._is_forum_parent = lambda _ch: False # type: ignore[method-assign]
|
||||
|
||||
from plugins.platforms.discord.adapter import _DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES
|
||||
|
||||
video = tmp_path / "huge.mp4"
|
||||
video.write_bytes(b"x")
|
||||
|
||||
send = AsyncMock(return_value=SimpleNamespace(id=1))
|
||||
channel = SimpleNamespace(id=1, guild=None, send=send)
|
||||
adapter._client = SimpleNamespace(
|
||||
get_channel=lambda _cid: channel,
|
||||
fetch_channel=AsyncMock(),
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
os.path,
|
||||
"getsize",
|
||||
lambda path: (
|
||||
_DISCORD_DEFAULT_UPLOAD_LIMIT_BYTES + 10
|
||||
if str(path) == str(video)
|
||||
else 0
|
||||
),
|
||||
)
|
||||
|
||||
base_called = {"yes": False}
|
||||
|
||||
async def boom(*_a, **_k):
|
||||
base_called["yes"] = True
|
||||
raise AssertionError("base send_video must not run for preflight reject")
|
||||
|
||||
monkeypatch.setattr(
|
||||
"gateway.platforms.base.BasePlatformAdapter.send_video",
|
||||
boom,
|
||||
)
|
||||
|
||||
result = await adapter.send_video("1", str(video))
|
||||
assert result.success is False
|
||||
assert "too large" in (result.error or "").lower()
|
||||
assert base_called["yes"] is False
|
||||
|
||||
Reference in New Issue
Block a user