Skip to content

TEL-575: do not transition state for 407#680

Merged
hechen-eng merged 3 commits into
mainfrom
407-state-update
May 9, 2026
Merged

TEL-575: do not transition state for 407#680
hechen-eng merged 3 commits into
mainfrom
407-state-update

Conversation

@hechen-eng
Copy link
Copy Markdown
Contributor

No description provided.

@hechen-eng hechen-eng requested a review from a team as a code owner May 8, 2026 18:05
Comment thread pkg/sip/inbound.go
// retry with digest credentials. This is part of the normal SIP auth
// handshake, not a finalized error, so suppress the deferred state
// transition to avoid recording a 0-duration call.
state = nil
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 think this is fine for now. But if we don't get the second invite, as its designed currently, we won't be logging the auth failure or showing it in the dashboard.. right ? Any way around that ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm, I thought that's part of the long-term plan? We don't need to log since sipfe will log the 407 response.

With current short-term fix, if there is a second invite, we are all good. If not, we'll have a hanging INCOMING record. As far as sip, 407 is a final response, the SIP transaction is complete from the server's perspective. Are we okay with this for now?

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.

The long term was more about logging an event for the first auth challenge. We still need to handle the case of a second invite not coming. If 407 is the final response in that case, how will billing know to end the call ?

Copy link
Copy Markdown
Contributor Author

@hechen-eng hechen-eng May 8, 2026

Choose a reason for hiding this comment

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

from billing's perspective, the call never started since the call is never active, so we should be fine in terms of billing.

It's bad user experience though (a hanging INCOMING call status). Maybe we can use a timeout to guard this 🤔

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.

So we send them the UpdateSIPCallStateRequest with callStatus set to SCS_INCOMING before all this happens don't we ? If that's not the case, then we are good.

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 we are sending SCS_INCOMING, we should also send either SCS_DISCONNECT or SCS_ERROR. As you suggested a timeout for the second invite might be a good approach then

Comment thread pkg/sip/inbound.go Outdated
@hechen-eng hechen-eng requested a review from nishadmusthafa May 8, 2026 21:14
Copy link
Copy Markdown
Contributor

@nishadmusthafa nishadmusthafa left a comment

Choose a reason for hiding this comment

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

Thank you @hechen-eng !

@hechen-eng hechen-eng merged commit 73c72b8 into main May 9, 2026
6 checks passed
@hechen-eng hechen-eng deleted the 407-state-update branch May 9, 2026 01:22
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