Skip to content

TEL-717: ack/bye to 200 ok after cancel - #756

Merged
hechen-eng merged 1 commit into
mainfrom
cancel-after-200ok
Jul 28, 2026
Merged

TEL-717: ack/bye to 200 ok after cancel#756
hechen-eng merged 1 commit into
mainfrom
cancel-after-200ok

Conversation

@hechen-eng

Copy link
Copy Markdown
Contributor

cancel and 200 ok is a common race. If 200 ok arrives after cancel, we should ACK and BYE.

@hechen-eng
hechen-eng requested a review from a team as a code owner July 24, 2026 17:44

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@hechen-eng

Copy link
Copy Markdown
Contributor Author

not related to this change, but it seems we have some flaky tests https://github.com/livekit/sip/actions/runs/30114193566/attempts/1?pr=756

@alexlivekit alexlivekit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If 200 ok arrives after cancel, we should ACK and BYE.

If this is an outbound call, only we can be sending CANCEL requests, and these should be rejected/dropped. If the remote party wants to reject an incoming (lk -> remote) call, they can simply respond with a >=400 status code.

What is this trying to fix exactly?

@alexlivekit alexlivekit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I stand corrected, we absolutely need to ACK & BYE the call if we want to abort, but receive a 200.

@hechen-eng
hechen-eng merged commit 66f432b into main Jul 28, 2026
7 of 11 checks passed
@hechen-eng
hechen-eng deleted the cancel-after-200ok branch July 28, 2026 02:47
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.

2 participants