-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: improve voip error handling #35776
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
ed6992d
fix: voip calls not ringing after temporary disconnection
pierre-lehnen-rc 0719cd9
only auto register if a registration was lost before
pierre-lehnen-rc 27d1307
show reconnecting status on usermenu
pierre-lehnen-rc 3080596
feat: improve voip error handling
pierre-lehnen-rc b73d7a2
fixed i18n key
pierre-lehnen-rc 652bfec
Merge branch 'develop' into fix/voip-hangup-cause
pierre-lehnen-rc f2b0b8f
moved error parsing to separate files and added unit tests for it
pierre-lehnen-rc a4eb161
Merge branch 'develop' into fix/voip-hangup-cause
pierre-lehnen-rc 2c89fcd
Merge branch 'develop' into fix/voip-hangup-cause
kodiakhq[bot] 7e08948
Merge branch 'develop' into fix/voip-hangup-cause
kodiakhq[bot] 0e9ac3a
Merge branch 'develop' into fix/voip-hangup-cause
kodiakhq[bot] eff3045
Merge branch 'develop' into fix/voip-hangup-cause
kodiakhq[bot] 7f363a2
Merge branch 'develop' into fix/voip-hangup-cause
scuciatto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@rocket.chat/ui-voip': minor | ||
| '@rocket.chat/i18n': minor | ||
| '@rocket.chat/meteor': minor | ||
| --- | ||
|
|
||
| Improves handling of errors during voice calls |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
59 changes: 59 additions & 0 deletions
59
packages/ui-voip/src/lib/getMainInviteRejectionReason.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { type Cancel as SipCancel, type Invitation, type SessionState } from 'sip.js'; | ||
|
|
||
| import { getMainInviteRejectionReason } from './getMainInviteRejectionReason'; | ||
|
|
||
| const mockInvitation = (state: SessionState[keyof SessionState]): Invitation => | ||
| ({ | ||
| state, | ||
| }) as any; | ||
|
|
||
| const mockSipCancel = (reasons: string[]): SipCancel => | ||
| ({ | ||
| request: { | ||
| headers: { | ||
| Reason: reasons.map((raw) => ({ raw })), | ||
| }, | ||
| }, | ||
| }) as any; | ||
|
|
||
| describe('getMainInviteRejectionReason', () => { | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('should return undefined for natural endings', () => { | ||
| const result = getMainInviteRejectionReason(mockInvitation('Terminated'), mockSipCancel(['SIP ;cause=487 ;text="ORIGINATOR_CANCEL"'])); | ||
| expect(result).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should return priorityErrorEndings if present', () => { | ||
| const result = getMainInviteRejectionReason( | ||
| mockInvitation('Terminated'), | ||
| mockSipCancel(['SIP ;cause=488 ;text="USER_NOT_REGISTERED"']), | ||
| ); | ||
| expect(result).toBe('USER_NOT_REGISTERED'); | ||
| }); | ||
|
|
||
| it('should return the first parsed reason if call was canceled at the initial state', () => { | ||
| const result = getMainInviteRejectionReason( | ||
| mockInvitation('Initial'), | ||
| mockSipCancel(['text="UNEXPECTED_REASON"', 'text="ANOTHER_REASON"']), | ||
| ); | ||
| expect(result).toBe('UNEXPECTED_REASON'); | ||
| }); | ||
|
|
||
| it('should log a warning if call was canceled for unexpected reason', () => { | ||
| console.warn = jest.fn(); | ||
| const result = getMainInviteRejectionReason( | ||
| mockInvitation('Terminated'), | ||
| mockSipCancel(['text="UNEXPECTED_REASON"', 'text="ANOTHER_REASON"']), | ||
| ); | ||
| expect(console.warn).toHaveBeenCalledWith('The call was canceled for an unexpected reason', ['UNEXPECTED_REASON', 'ANOTHER_REASON']); | ||
| expect(result).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('should handle empty parsed reasons array gracefully', () => { | ||
| const result = getMainInviteRejectionReason(mockInvitation('Terminated'), mockSipCancel([])); | ||
| expect(result).toBeUndefined(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import type { Cancel as SipCancel, Invitation } from 'sip.js'; | ||
|
|
||
| import { parseInviteRejectionReasons } from './parseInviteRejectionReasons'; | ||
|
|
||
| const naturalEndings = [ | ||
| 'ORIGINATOR_CANCEL', | ||
| 'NO_ANSWER', | ||
| 'NORMAL_CLEARING', | ||
| 'USER_BUSY', | ||
| 'NO_USER_RESPONSE', | ||
| 'NORMAL_UNSPECIFIED', | ||
| ] as const; | ||
|
|
||
| const priorityErrorEndings = ['USER_NOT_REGISTERED'] as const; | ||
|
|
||
| export function getMainInviteRejectionReason(invitation: Invitation, message: SipCancel): string | undefined { | ||
| const parsedReasons = parseInviteRejectionReasons(message); | ||
|
|
||
| for (const ending of naturalEndings) { | ||
| if (parsedReasons.includes(ending)) { | ||
| // Do not emit any errors for normal endings | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| for (const ending of priorityErrorEndings) { | ||
| if (parsedReasons.includes(ending)) { | ||
| // An error definitely happened | ||
| return ending; | ||
| } | ||
| } | ||
|
|
||
| if (invitation?.state === 'Initial') { | ||
| // Call was canceled at the initial state and it was not due to one of the natural reasons, treat it as unexpected | ||
| return parsedReasons.shift(); | ||
kody-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| console.warn('The call was canceled for an unexpected reason', parsedReasons); | ||
| } | ||
149 changes: 149 additions & 0 deletions
149
packages/ui-voip/src/lib/parseInviteRejectionReasons.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| import type { Cancel as SipCancel } from 'sip.js'; | ||
|
|
||
| import { parseInviteRejectionReasons } from './parseInviteRejectionReasons'; | ||
|
|
||
| describe('parseInviteRejectionReasons', () => { | ||
| it('should return an empty array when message is undefined', () => { | ||
| expect(parseInviteRejectionReasons(undefined as any)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should return an empty array when headers are not defined', () => { | ||
| const message: SipCancel = { request: {} } as any; | ||
| expect(parseInviteRejectionReasons(message)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should return an empty array when Reason header is not defined', () => { | ||
| const message: SipCancel = { request: { headers: {} } } as any; | ||
| expect(parseInviteRejectionReasons(message)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should parse a single text reason correctly', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'text="Busy Here"' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['Busy Here']); | ||
| }); | ||
|
|
||
| it('should extract cause from Reason header if text is not present', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'SIP ;cause=404' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['404']); | ||
| }); | ||
|
|
||
| it('should extract text from Reason header when both text and cause are present ', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { Reason: [{ raw: 'SIP ;cause=200 ;text="OK"' }] }, | ||
| }, | ||
| } as any; | ||
| expect(parseInviteRejectionReasons(message)).toEqual(['OK']); | ||
| }); | ||
|
|
||
| it('should return the raw reason if no matching text or cause is found', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'code=486' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['code=486']); | ||
| }); | ||
|
|
||
| it('should parse multiple reasons and return only the text parts', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'text="Busy Here"' }, { raw: 'text="Server Internal Error"' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['Busy Here', 'Server Internal Error']); | ||
| }); | ||
|
|
||
| it('should return an array of parsed reasons when valid reasons are present', () => { | ||
| const mockMessage: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'SIP ;cause=200 ;text="Call completed elsewhere"' }, { raw: 'SIP ;cause=486 ;text="Busy Here"' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| const result = parseInviteRejectionReasons(mockMessage); | ||
| expect(result).toEqual(['Call completed elsewhere', 'Busy Here']); | ||
| }); | ||
|
|
||
| it('should parse multiple reasons and return the mixed text, cause and raw items, on this order', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [{ raw: 'text="Busy Here"' }, { raw: 'code=503' }, { raw: 'cause=488' }, { raw: 'text="Forbidden"' }], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['Busy Here', 'Forbidden', '488', 'code=503']); | ||
| }); | ||
|
|
||
| it('should filter out any undefined or null values from the resulting array', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [ | ||
| { raw: 'SIP ;cause=500 ;text="Server Error"' }, | ||
| { raw: null as unknown as string }, // Simulate an edge case { raw: '' } | ||
| ], | ||
| }, | ||
| }, | ||
| } as any; | ||
| expect(parseInviteRejectionReasons(message)).toEqual(['Server Error']); | ||
| }); | ||
|
|
||
| it('should handle non-string raw values gracefully and return only valid matches', () => { | ||
| const message: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [ | ||
| { raw: 'text="Service Unavailable"' }, | ||
| { raw: { notAString: true } as unknown as string }, // Intentional type misuse for testing | ||
| { raw: 'code=486' }, | ||
| ], | ||
| }, | ||
| }, | ||
| } as any; | ||
|
|
||
| expect(parseInviteRejectionReasons(message)).toEqual(['Service Unavailable', 'code=486']); | ||
| }); | ||
|
|
||
| it('should return an empty array when exceptions are thrown', () => { | ||
| // Mock the function to throw an error | ||
| const faultyMessage: SipCancel = { | ||
| request: { | ||
| headers: { | ||
| Reason: [ | ||
| { | ||
| raw: () => { | ||
| throw new Error('unexpected error'); | ||
| }, | ||
| }, | ||
| ] as any, | ||
| }, | ||
| }, | ||
| } as any; | ||
| expect(parseInviteRejectionReasons(faultyMessage)).toEqual([]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import type { Cancel as SipCancel } from 'sip.js'; | ||
|
|
||
| export function parseInviteRejectionReasons(message: SipCancel): string[] { | ||
| try { | ||
kody-ai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const reasons = message?.request?.headers?.Reason; | ||
| const parsedTextReasons: string[] = []; | ||
| const parsedCauseReasons: string[] = []; | ||
| const rawReasons: string[] = []; | ||
|
|
||
| if (reasons) { | ||
| for (const { raw } of reasons) { | ||
| if (!raw || typeof raw !== 'string') { | ||
| continue; | ||
| } | ||
|
|
||
| const textMatch = raw.match(/text="(.+)"/); | ||
| if (textMatch?.length && textMatch.length > 1) { | ||
| parsedTextReasons.push(textMatch[1]); | ||
| continue; | ||
| } | ||
| const causeMatch = raw.match(/cause=_?(\d+)/); | ||
| if (causeMatch?.length && causeMatch.length > 1) { | ||
| parsedCauseReasons.push(causeMatch[1]); | ||
| continue; | ||
| } | ||
|
|
||
| rawReasons.push(raw); | ||
| } | ||
| } | ||
|
|
||
| return [...parsedTextReasons, ...parsedCauseReasons, ...rawReasons]; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.