slackapi / slackapi/python-slack-sdk
chat_postMessage silently forwards thread_id to the API, so a threaded reply posts to the channel
まだ誰も着手していません。
- 主要言語
- Python
- スター
- 4k
- フォーク
- 857
- 平均マージ
- 22時間 21分
- マージ済み PR(30日)
- 16
説明
Summary
Every WebClient method takes **kwargs and forwards unrecognised keys into the request body
untouched. A one-character mistake in a parameter name therefore produces a request that is
missing the parameter you meant and carrying one nobody reads, with no exception and no warning.
For thread_ts nothing errors at all. A threaded reply becomes a top-level channel message and
every downstream check reports success.
Reproduction
Against slack_sdk 3.43.0, with the transport stubbed so the outgoing body is visible:
client.chat_postMessage(
channel="C1",
text="hi",
thread_id="1750000000.0001", # the parameter is thread_ts
unfurl_link=False, # the parameter is unfurl_links
)
Body actually sent:
{"thread_id": "1750000000.0001", "unfurl_link": false, "channel": "C1", "text": "hi"}
warnings raised: 0. thread_ts is absent from the body entirely, because it was None and
_remove_none_values stripped it. So the request is well formed, it just is not the request the
caller wrote.
Mechanism
In slack_sdk/web/client.py, chat_postMessage declares its known parameters and then does:
kwargs.update({ "channel": channel, "text": text, ..., "thread_ts": thread_ts, ... })
_parse_web_class_objects(kwargs)
kwargs = _remove_none_values(kwargs)
return self.api_call("chat.postMessage", json=kwargs)
thread_id arrived through **kwargs, survives _remove_none_values because it is not None,
and goes out with everything else. Nothing in the path compares the caller's keys against the
declared parameter list.
Why I think this deserves a fix
The declared parameters are already there, in the signature, which is what makes this cheap. The
method knows the full set of names it accepts. A caller who misses by one character is currently
told nothing at all, and the failure surfaces somewhere else, later, as a message in the wrong
place.
Three options, cheapest first:
- Warn on unknown keys.
warnings.warnnaming the key and the method. Non-breaking, and it
makes the mistake visible in a test run. - Warn harder on near misses. If an unknown key is within an edit distance of one or two of a
declared parameter, say which one you probably meant.thread_idtothread_ts,
unfurl_linktounfurl_links,file_contenttocontent. - Do nothing, but say so in the docstring, since the current behaviour is reasonable as an
escape hatch for API parameters the SDK has not caught up with. That is a real design reason
for**kwargsand I do not think it should be removed.
I would expect (1) or (2). Removing **kwargs would break the escape hatch and I am not
suggesting it.
A related one, same mechanism, different symptom
files_upload_v2(file_content=...) sends file_content through **kwargs and then raises
SlackRequestError: Any of file, content, and file_uploads must be specified.
which names three parameters the caller does not believe they omitted. Same root cause, and the
error message is actively misleading rather than merely absent.
How I found this, and what I am not claiming
I maintain a test harness that measures whether a coding model can drive a given SDK by running
the code it writes and asserting on the HTTP the SDK emits. slack_sdk 3.43.0 came out at the
top of everything I have measured: 16 tasks, 89 checks, three models at three attempts each, and
both frontier models passed all 89 checks on all 48 rollouts. So this is not a report that
models struggle with slack_sdk. They do not.
No model hit this bug. I found it by construction while checking whether my own scoring
could be gamed. That makes it latent, not measured, and I would rather say so than let it read
as a field report.
One limit on the repro above: my transport is stubbed, so what I can show is the body your SDK
sends. Whether Slack ignores thread_id and answers ok: true is your knowledge, not mine.
Happy to open a separate issue about files_upload's deprecation warning, which describes a
timeout risk ("may cause some issues like timeouts for relatively large files",
internal_utils.py:443) for an endpoint that has been sunset. Say the word rather than me
filing two at once.
toolshed is a small studio run by its owner, who directs the work, and AI does a lot of the
engineering.
Cal / toolshed / toolshedlabs@gmail.com
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
slack_sdk/web/client.py から始め、特に chat_postMessage と files_upload_v2 について、宣言されたパラメーターと **kwargs が _remove_none_values および api_call に到達するまでを追跡します。動作を選択する前に、issue の警告オプションと既存のテストを確認します。完了条件は、選択した処理が未知のキーについてカバーされ、報告された thread_id と file_content のケースが、もはや黙って、または誤解を招く形で失敗しないことです。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- python
- 領域
- api, backend-api-design
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 活発
- 明瞭さ
- おおむね明確
- 初心者へのやさしさ
- 48/100