slackapi / slackapi/python-slack-sdk
chat_postMessage silently forwards thread_id to the API, so a threaded reply posts to the channel
Nessuno ha ancora preso questa issue.
- Lingua principale
- Python
- Stelle
- 4k
- Fork
- 857
- Merge medio
- 22h 21m
- PR unite (30g)
- 16
Descrizione
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
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia in slack_sdk/web/client.py, in particolare con chat_postMessage e files_upload_v2, e traccia il percorso con cui i parametri dichiarati e **kwargs arrivano a _remove_none_values e api_call. Esamina le opzioni di avviso dell’issue e i test esistenti prima di scegliere un comportamento. Il lavoro è completo quando la gestione selezionata è coperta per le chiavi sconosciute e i casi segnalati thread_id e file_content non falliscono più silenziosamente o in modo fuorviante.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- python
- Ambito
- api, backend-api-design
- Tipo di issue
- Bug
- Difficoltà
- 4/5
- Tempo stimato
- 3-5 giorni
- Stato di attività
- Attiva
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 48/100