Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Mattermost alert webhook call #5700

Merged
merged 1 commit into from
May 5, 2020
Merged

Conversation

Scotchester
Copy link
Contributor

@Scotchester Scotchester commented May 4, 2020

Mattermost seems to have gotten more particular about the Content-Type header of incoming webhooks in the upgrade from 5.18 to 5.22.

We were seeing errors of this kind:

{
  "level": "error",
  "ts": 1588614108.121396,
  "caller": "mlog/log.go:175",
  "msg": "Could not decode the multipart payload of incoming webhook.",
  "path": "/hooks/<hook_id>",
  "request_id": "wsb5g1xiqine7knmgj9yo9887w",
  "ip_addr": "<ip>",
  "user_id":"",
  "method": "POST",
  "err_where": "incomingWebhook",
  "http_code": 400,
  "err_details": "mime: no media type"
}

This change switches our alert webhook's requests.post call from using Requests' data param to using its json param, which sets the Content-Type to application/json, as Mattermost now requires.

See: https://requests.readthedocs.io/en/master/user/quickstart/#more-complicated-post-requests

Changes

  • Switches Mattermost alert webhook requests.post call from data to json

Testing

In your cfgov-refresh virtualenv, open the Python console and walk through the following:

>>> import json
>>> import requests
# Be sure to replace the `GHE` in the string below.
>>> payload = {
...     'text': 'Manual MM webhook testing',
...     'username': 'alertbot',
...     'icon_url': 'https://GHE/raw/CFGOV/platform/master/alerts/scream.png'
... }

# Confirm existing error behavior:
# (Contact me for the full URL you should use.)
>>> resp = requests.post(
...     'https://mattermost/hooks/<hook_id>',
...     data=json.dumps(payload)
... )
>>> resp
<Response [400]>
>>> resp.content
b'{
    "id": "api.webhook.incoming.error",
    "message": "Could not decode the multipart payload of incoming webhook.",
    "detailed_error": "",
    "request_id": "b38mndqaz7y8zche3dtwjfj1zc",
    "status_code": 400
}'
# If you have MM system console access, you can also see an error in the logs there.

# Try the new behavior:
# (Contact me for the full URL you should use.)
>>> resp = requests.post(
...     'https://mattermost/hooks/<hook_id>',
...     json=payload
... )
>>> resp
<Response [200]>
# You will also see the successful post in the cf.gov Alerts channel.

Notes

  • I think this may have the interesting side effect of actually using the scream emoji for the bot's avatar, instead of the stock webhook logo.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the CFPB development guidelines
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 馃憞)
  • Project documentation has been updated
  • Reviewers requested with the Reviewers tool 鉃★笍

Mattermost seems to have gotten more particular about the `Content-Type` 
header of incoming webhooks in the upgrade from 5.18 to 5.22.

We were seeing errors of this kind:

```json
{
  "level": "error",
  "ts": 1588614108.121396,
  "caller": "mlog/log.go:175",
  "msg": "Could not decode the multipart payload of incoming webhook.",
  "path": "/hooks/<hook_id>",
  "request_id": "wsb5g1xiqine7knmgj9yo9887w",
  "ip_addr": "<ip>",
  "user_id":"",
  "method": "POST",
  "err_where": "incomingWebhook",
  "http_code": 400,
  "err_details": "mime: no media type"
}
```

This change switches our alert webhook's `requests.post` call from using 
Requests' `data` param to using its `json` param, which sets the 
`Content-Type` to `application/json`, as Mattermost now requires.

See: 
https://requests.readthedocs.io/en/master/user/quickstart/#more-complicated-post-requests
Copy link
Member

@higs4281 higs4281 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go team 馃槺

@Scotchester Scotchester merged commit deb6632 into master May 5, 2020
@Scotchester Scotchester deleted the fix-mattermost-alerts branch May 5, 2020 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants