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

Only fail webhook if http status is not 202 #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isihu
Copy link

@isihu isihu commented Nov 6, 2024

Microsoft is retiring webhooks, Workflows should be used in the future.
That also means that a webhook isn't failed when the response text is empty, but when the return code is not 202.
Due to that, the ms-teams-webhook library this action uses has released a new version, that supports the new workflows and also returns more information about the response of MS Teams.
This PR uses this response to correctly determine the success of the request.

I have not tested this thoroughly, but it works in my case.

This may be a breaking change.

@t-readyroc
Copy link

O365 webhooks will be non-functional by the end of next January. Updating the library to the new version that supports workflows is going to be the only way forward for this action. Please merge this PR.

@waigel
Copy link

waigel commented Nov 22, 2024

Please note that users must migrate their MessageCards here. We have not yet found a way to avoid or circumvent this.

Although we have provided a small AI tool that can help with this, we have not yet received enough feedback on the quality. Since you are one of the biggest projects on the lib, please let us know if we can still support you in any way.

@isihu
Copy link
Author

isihu commented Nov 22, 2024

I'm not the maintainer of this action, but it seems to me like the action does not use MessageCards anymore, so it should be fine right?

Relevant commit

@waigel
Copy link

waigel commented Nov 22, 2024

That is correct, but the lib also offers to send raw payloads. With the switch to sendRawAdaptiveCard this could be broken in future versions.

@isihu
Copy link
Author

isihu commented Nov 22, 2024

Not sure if I understood the problem correctly. Are you planning on deprecating sendRawAdaptiveCard?

@waigel
Copy link

waigel commented Nov 22, 2024

No, it was not a blocker from me, I just wanted to point out that we will not support the sending of MessageCards via the sendRawAdaptiveCard in the future. In other words, if you switch from send to sendRawAdaptiveCard, there is a risk that it will no longer be possible to send message cards in a future version, which could be a breaking change for users with raw payload.

@isihu
Copy link
Author

isihu commented Nov 22, 2024

Ah, but this project should be fine as we already moved to AdaptiveCards, got it. Thanks 👍

@dsibilio
Copy link

dsibilio commented Dec 5, 2024

@Skitionek any chance of having this reviewed and merged soon?

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.

4 participants