rveachkc / pymsteams

Format messages and post to Microsoft Teams.
https://pypi.org/project/pymsteams/
Apache License 2.0
418 stars 77 forks source link

TeamsWebhookException despite successful POST #135

Closed alextadams88 closed 1 year ago

alextadams88 commented 1 year ago

Describe the bug During send(), Teams returns a successful response payload, and the message successfully appears in the teams channel, however a TeamsWebhookException is raised regardless.

pymsteams is expecting the response text to simply be "1"

if r.status_code == requests.codes.ok and r.text == '1': # pylint: disable=no-member
            return True
        else:
            raise TeamsWebhookException(r.text)

However r.text is actually:

{"version":"1.1","content":{"headers":[{"key":"Content-Type","value":["text/plain; charset=utf-8"]}]},"statusCode":200,"reasonPhrase":"OK","headers":[],"trailingHeaders":[],"requestMessage":null,"isSuccessStatusCode":true}

To Reproduce Steps to reproduce the behavior:

Send any message. It may be dependent on the webhook you are hitting.

Expected behavior TeamsWebhookException is not thrown.

Screenshots thrownException successfulTeamsMessage

Additional context This was working for us up until Friday, 9/30, around 19:00, when it suddenly started failing. Seems that perhaps Teams may have changed their API.

alextadams88 commented 1 year ago

after removing the check for r.text == '1', it was successful.

demonstratingTheChange2

anotherSuccessfulTeamsMessage

alextadams88 commented 1 year ago

Created https://github.com/rveachkc/pymsteams/pull/136

kingsleyadam commented 1 year ago

We're getting the same and noticed the same behavior. It's true, if the response code is successful, then it can be assumed the message was successfully sent (although maybe there was a reason at the time that the text response also needed to be 1).

Unfortunately I can't find any documentation on the api itself and the possible responses. https://learn.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using?tabs=cURL. Maybe I'm just missing it.

Until this is patched I'm going to implement another catch downstream to verify the response.

For anyone else that may need this in the meantime.

# Patch in a successful response, see: https://github.com/rveachkc/pymsteams/issues/135
try:
    card.send()
except pymsteams.TeamsWebhookException as teams_exception:
    try:
        # If this is a successful response it should return a JSON value.
        # If it's not successful it could return a number of things, catch this.
        _response = json.loads(teams_exception.args[0])
    except ValueError:
        raise teams_exception

    if _response.get('statusCode') != 200:
        raise teams_exception
rveachkc commented 1 year ago

Thanks, I noticed the "1" was passed, even though it was never documented. This was done out of an abundance of caution.

Thanks to @alextadams88, it was trivial to merge the PR and release in v0.2.2

atc0005 commented 1 year ago

For anyone else coming across this regarding the response code needing to be 1, see also:

FWIW, I'm still seeing a 1 as the response code from submitting via webhook URL. Perhaps they're rolling out the change to specific tenants first before deploying the change for everyone?

EDIT:

Regarding documentation, the 1 response code is noted here:

mockodin commented 1 year ago

We're getting the same and noticed the same behavior. It's true, if the response code is successful, then it can be assumed the message was successfully sent (although maybe there was a reason at the time that the text response also needed to be 1).

Unfortunately I can't find any documentation on the api itself and the possible responses. https://learn.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using?tabs=cURL. Maybe I'm just missing it.

Until this is patched I'm going to implement another catch downstream to verify the response.

For anyone else that may need this in the meantime.

# Patch in a successful response, see: https://github.com/rveachkc/pymsteams/issues/135
try:
    card.send()
except pymsteams.TeamsWebhookException as teams_exception:
    try:
        # If this is a successful response it should return a JSON value.
        # If it's not successful it could return a number of things, catch this.
        _response = json.loads(teams_exception.args[0])
    except ValueError:
        raise teams_exception

    if _response.get('statusCode') != 200:
        raise teams_exception

Worth noting that originally MS always returned 200 even when there was an internal failure, you had to check the body for "1", the appears to have changed. Oddly I see documentation of the change as early as 2019 however it only just now broke.

This was seem most often when a back end node was not fully initialized but traffic has been routed to the node before it was ready and was not retried, I think indicating a 40x.

I would still suggest we look at the json response along the lines of:

            try:
                status = resp.json()
                if resp.status_code == httpx.codes.OK:
                    return status['isSuccessStatusCode'] is True
            except Exception:
                raise TeamsWebhookException(resp.text)