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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft Teams Webhook Connector "200 OK" status insufficient indication of success #59

Closed
atc0005 opened this issue Sep 2, 2020 · 0 comments · Fixed by #81
Closed
Assignees
Labels
api bug Something isn't working
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented Sep 2, 2020

From rveachkc/pymsteams#75:

I did use a script to verify the size constraints, and I'll detail it below if it's interesting, but I talked with the Teams team and believe I found a better way.

It seems the reason for the 200 status in cases where the message doesn't send is due to 3rd party connectors, incorrectly, handling errors by deleting the configuration.

tl;dr

The team let me know that the content of the response will be '1' when the message is sent successfully. So I propose updating the check before returning in send from if r.status_code == requests.codes.ok: to if r.status_code == requests.codes.ok and r.text == '1'

Needs further research/testing. Further notes from that GH issue:

It's seemingly the combined data size sent that effects it
Without a summary, I get an exception so I'm using a single char for that (more characters would then mean removing others from the cardsection text or title, but it's not 1-for-1 i.e. a summary of '00' won't work if a "bare text message" is sent w\ 20826 characters
With a title that was the count of characters sent, the limit is 20,811 chars
W\o a title the limit bumps up to 20,827

So 20k+ characters is enough to hit a limit, but probably worth implementing the check as noted previously instead of trying to check for a specific hard-coded message size limit.

@atc0005 atc0005 added bug Something isn't working api labels Sep 2, 2020
@atc0005 atc0005 added this to the Future milestone Sep 2, 2020
@atc0005 atc0005 modified the milestones: Future, v2.3.2 Jan 28, 2021
@atc0005 atc0005 self-assigned this Jan 28, 2021
atc0005 added a commit that referenced this issue Jan 28, 2021
Per @stmoody's report in the `rveachkc/pymsteams` project,
Microsoft Teams webhook endpoint(s) will return a 200 status
response code in some scenarios when a message was actually
(seemingly silently) rejected due to size limits. Microsoft
Teams developers shared that a response string of `1` must
be present alongside a 200 status response code to indicate
successful message submission by the client.

This commit extends response validation to check for the
presence of this specific response string, otherwise returns
a custom error along with a description of what was expected
and what was actually received.

refs rveachkc/pymsteams#75
refs GH-59
atc0005 added a commit that referenced this issue Jan 28, 2021
Per @stmoody's report in the `rveachkc/pymsteams` project,
Microsoft Teams webhook endpoint(s) will return a 200 status
response code in some scenarios when a message was actually
(seemingly silently) rejected due to size limits. Microsoft
Teams developers shared that a response string of `1` must
be present alongside a 200 status response code to indicate
successful message submission by the client.

This commit extends response validation to check for the
presence of this specific response string, otherwise returns
a custom error along with a description of what was expected
and what was actually received.

refs rveachkc/pymsteams#75
refs GH-59
atc0005 added a commit that referenced this issue Jan 28, 2021
Current tests fail with prior change, pass now with updated
response text to match updated validation requirement.

refs GH-59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant