Skip to content

fix(msteams): Answer a rejected card action with the API's status - #123477

Open
vaind wants to merge 1 commit into
masterfrom
ivandlugos/msteams-card-action-4xx-response
Open

fix(msteams): Answer a rejected card action with the API's status#123477
vaind wants to merge 1 commit into
masterfrom
ivandlugos/msteams-card-action-4xx-response

Conversation

@vaind

@vaind vaind commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

_issue_state_change classifies the ApiError the issue update API raises and then falls through to return response, which the failed call never bound. Every rejected card action raises UnboundLocalError and answers 500.

MS Teams card actions are forwarded asynchronously through WebhookPayload, and the drain reads a 500 as retryable. A permanently rejected action — the clicking user lacks permission, or names a release that isn't configured — is retried ten times at hour-long backoff, and because msteams is a strict provider the rest of the tenant's mailbox waits behind it. Answering with the API's own status makes it a dropped_4xx on the first attempt, so the head clears.

The inNextRelease branch also assumed a mapping body, which is a second way into the same 500: a ValidationError raised on a bare string serializes to a list. _is_next_release_error checks the shape before reaching into it.

Tests

Two cases in test_action_state_change.py, both of which reproduce the 500 without the fix: a 403 with a mapping body, and a 400 with a list body.

Context

Found while reviewing #123422, which lets the drain skip a failed msteams record instead of head-blocking on it. That option papers over this; the wedge itself is this bug, and the fix stands on its own.

`_issue_state_change` classified the `ApiError` from the issue update API and then
fell through to `return response`, which the failed call never bound. Every rejected
card action raised `UnboundLocalError` and answered 500.

MS Teams card actions are forwarded asynchronously through `WebhookPayload`, and the
drain reads a 500 as retryable: a permanently rejected action -- the clicking user
lacks permission, or names a release that isn't configured -- was retried ten times
at hour-long backoff while the rest of the tenant's mailbox waited behind it. A 4xx
is dropped instead, so the head clears on the first attempt.

The `inNextRelease` check also assumed a mapping body, which is a second way into the
same 500: a `ValidationError` raised on a bare string serializes to a list.
@vaind
vaind requested a review from a team as a code owner September 2, 2026 19:17
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant