slackapi / python-slack-sdk

Slack Developer Kit for Python
https://tools.slack.dev/python-slack-sdk/
MIT License
3.86k stars 832 forks source link

When sending a message with metdata through the API as a bot, the metadata.event_payload is not returned #1501

Closed patrickshuff closed 4 months ago

patrickshuff commented 5 months ago

Reproducible in:

This is reproducible using the python-sdk, the slack-go sdk, and when using cURL to post a message directly to the API.

It should be noted this is not specific to python, this also happens when posting an equivalent messaging using the slack-go implementation. This is happening within the Netflix enterprise workspace.

The Slack SDK version

This happens both with the python SDK, and the golang sdk. Not specific to client.

Python runtime version

This happens both with the python SDK, and the golang sdk. Not specific to client.

OS info

This happens both with the python SDK, and the golang sdk. Not specific to OS.

Steps to reproduce:

(Share the commands to run, source code, and project settings (e.g., setup.py))

  1. First, with a bot token, send a trival message with trivial metadata like so:
from slack_sdk import WebClient
client = WebClient(token=BOT_TOKEN)

response = client.chat_postMessage(
        channel='#test',
        text="test message",
    metadata={"event_type": "testevent", "event_payload": {"key1": "value1"}}

    )
print(response)

Here is a screenshot showing the response running the above python and we can conclude that the metadata and event_payload is properly being sent to the API: 334560504-5a931656-5ef8-44c0-a553-64d106f84f0a

This can also be reproduced with slack-go golang client:

client := slack.Client(...)

s1, s2, err := client.PostMessage(
                        "#test",
                        slack.MsgOptionText(("test message"), false),
                        slack.MsgOptionMetadata(
                                slack.SlackMetadata{
                                        EventType: "testevent",
                                        EventPayload: map[string]interface{}{
                                                "key1": "value1",
                                        },
                                },
                        ),
                )

Expected result:

The message should include both the event_type and the event_payload with the message like so:

{
  "type": "message",
  "channel": "XXXX",
  "text": "text message",
  "ts": "1716927137.354419",
  "subtype": "bot_message",
  "event_ts": "1716927137.354419",
  "bot_id": "XXXX",
  "bot_profile": {
    "app_id": "XXXX",
    "id": "XXXX",
    "name": "bot",
    "team_id": "XXXX",
    "updated": 1522349148
  },
  "team": "XXXX",
  "replace_original": false,
  "delete_original": false,
  "metadata": {
    "event_type": "testevent",
    "event_payload": {
      "event_type": "testevent", 
      "event_payload": {"key1": "value1"}}           < ----------- What is expected             
  },
  "blocks": [
    {
      "type": "rich_text",
      "block_id": "2zt",
      "elements": [
        {
          "type": "rich_text_section",
          "elements": [
            {
              "type": "text",
              "text": "test message"
            }
          ]
        }
      ]
    }
  ]
}

Actual result:

The message includes the event_type but the event_payload is not returned in the response. This is observed in the python SDK, the golang SDK, and the slack inspector. Here is message:

{
  "type": "message",
  "channel": "XXXX",
  "text": "text message",
  "ts": "1716927137.354419",
  "subtype": "bot_message",
  "event_ts": "1716927137.354419",
  "bot_id": "XXXX",
  "bot_profile": {
    "app_id": "XXXX",
    "id": "XXXX",
    "name": "bot",
    "team_id": "XXXX",
    "updated": 1522349148
  },
  "team": "XXXX",
  "replace_original": false,
  "delete_original": false,
  "metadata": {
    "event_type": "testevent",
    "event_payload": null                         < -----------  What we actually receive using python, golang, and inspecting slack desktop gui payload response
  },
  "blocks": [
    {
      "type": "rich_text",
      "block_id": "2zt",
      "elements": [
        {
          "type": "rich_text_section",
          "elements": [
            {
              "type": "text",
              "text": "test message"
            }
          ]
        }
      ]
    }
  ]
}

I've also used the inspector in slack and verified that the metadata payload is also not being sent there either: image

misscoded commented 5 months ago

Hi @patrickshuff! I saw this was raised internally, and it looks like you may have received a resolution already, but I wanted to respond here and double-check – just in case anyone else runs into the same issue in the future. 🙂

As far as the thread I'm reading goes, it seems like the metadata was being returned as expected as seen in the payload screenshot provided above, but it just wasn't visible when inspecting the desktop client (as the client doesn't include metadata).

Is that correct? If I've gotten that wrong, please let me know, else we'll go ahead and close this issue if the issue is resolved.

patrickshuff commented 5 months ago

I'll try and summarize the underlying issue, and some potential options for getting around this, and then my opinions on what the slack engineering team should do:

The Root Problem

By default both on the RTM based message retrieval, and the API based message retrieval, any metadata EventPayload that is sent that does not explicitly have that event metadata structure added to the application manifest is stripped out. However on both endpoints it still passes the EventType through to the caller.

Recommended Solution (not verified)

The recommended fix from slack is to add metadata for the underlying event to the slack app manifest as defined here: https://api.slack.com/automation/metadata-events

I have not verified this fix as I'm unable to do this at this time.

Potential workaround

For the API based conversation.history method there is a flag called include_all_metadata that is available in (thanks PR-1139) that you can pass that will have the server return all of the metadata. I have confirm this with the python and golang clients.

API Docs: https://api.slack.com/methods/conversations.history#arg_include_all_metadata

For the RTM based message retrieval, which I am using for my use case, you are out of luck because there is no option to pass in a flag like this.

My opinions on what slack should do

This is a super confusing and painful developer experience to pass back the EventType but strip out the EventPayload. I spent an inordinate amount of time trying multiple SDKs on both the sending and receiving side to end up at the conclusion that the server must be stripping out the payload. The behavior that I'd rather see, in order of preference:

  1. Pass the entire EventType and EventPayload with the messages as they're being retrieved from both the API and RTM based endpoints regardless of a valid definition. The metadata was added to the API call for a reason
  2. If you decide that metadata event definition is required, it would be great if you had some way to give feedback to the developer that they're trying to send an metadata of EventType that is not registered and encourage them to do so instead of silently swallowing / discarding payload. e.g. on the server-side send back a HTTP400 with an error message that EventType is not configured.
  3. If you choose not to do no. 1 or no. 2, at least be consistent and strip both EventType and EventPayload so no metadata is returned so your developers aren't spending a ton of time trying to figure out why they're sending in the EventPayload incorrectly.

Lastly you should make sure that your HTTP API and RTM based APIs are equivalent. I was elated to know there was a workaround with include_all_metadata until I went back to the code realizing we were using the RTM interface to ingest messages to the bot.

Thanks for listening. I'll go ahead and close this out.

github-actions[bot] commented 4 months ago

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

github-actions[bot] commented 4 months ago

As this issue has been inactive for more than one month, we will be closing it. Thank you to all the participants! If you would like to raise a related issue, please create a new issue which includes your specific details and references this issue number.