chat_postMessage silently forwards thread_id to the API, so a threaded reply posts to the channel
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 48/100
- issue の種類
- バグ
- 明瞭さ
- おおむね明確
- 活発さ
- 活発
- 技術スタック
- python
調査の方向性
slack_sdk/web/client.py から始め、特に chat_postMessage と files_upload_v2 について、宣言されたパラメーターと **kwargs が _remove_none_values および api_call に到達するまでを追跡します。動作を選択する前に、issue の警告オプションと既存のテストを確認します。完了条件は、選択した処理が未知のキーについてカバーされ、報告された thread_id と file_content のケースが、もはや黙って、または誤解を招く形で失敗しないことです。
索引モデルが issue の本文から書いたものです。
説明
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
- 主要言語
- Python
- スター
- 4k
- フォーク
- 857
- 平均マージ
- 22時間 21分
- マージ済み PR(30日)
- 16
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
slackapi/python-slack-sdk のほかの issue
-
needs info server-side-issue
難易度 4/5 3〜5日 初心者へのやさしさ 35/100
slackapi/python-slack-sdk#1961 · コメント 3 件 ·
-
Use logger.isEnabledFor(logging.DEBUG) instead of logger.level <= logging.DEBUG for debug guards オープンauto-triage-skip bug
難易度 4/5 3〜5日 初心者へのやさしさ 55/100
slackapi/python-slack-sdk#1957 ·
-
auto-triage-skip discussion
難易度 5/5 1週間以上 初心者へのやさしさ 35/100
slackapi/python-slack-sdk#1940 · コメント 2 件 ·
-
auto-triage-skip bug socket-mode
難易度 3/5 1〜2日 初心者へのやさしさ 72/100
slackapi/python-slack-sdk#1922 · コメント 2 件 ·
-
auto-triage-skip bug python web-client
難易度 3/5 1〜2日 初心者へのやさしさ 52/100
slackapi/python-slack-sdk#1853 · コメント 2 件 ·
slackapi/python-slack-sdk の issue をすべて見る
似ている issue
-
area: harness bug status: needs-triage
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
Human-Agent-Society/reef#625 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
-
難易度 1/5 1時間未満 初心者へのやさしさ 80/100
learningequality/kolibri#15351 · コメント 2 件 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
Name consistency オープン
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
eellak/triplestore#65 · コメント 1 件 ·