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.

Sign up for GitHub

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

Jump to bottom

blind call transfer improvement #556

Open
ikq opened this issue Dec 23, 2018 · 22 comments
Open

blind call transfer improvement #556

ikq opened this issue Dec 23, 2018 · 22 comments

Comments

@ikq
Copy link

ikq commented Dec 23, 2018 •
edited
Loading

I found problem with the blind call transfer scenario

  1. 1st phone send REFER to 2nd

  2. 2nd: REFER accepted, and start calling to 3rd

  3. 1st phone send BYE to 2nd
    Notes:
    According RFC 5589 1st phone should not here send BYE to 2nd until receive NOTIFY with calling result.
    But the specification does not provide realistic case when operator in the 1st phone initiate
    call transfer for the 2nd, and don't have time to wait result.

  4. Now 3rd phone accept incoming from 2nd phone call
    Then the 2nd phone will send NOTIFY to 1st.
    Because the dialog is already terminated, there will be error during the sending,
    and the "2nd-3rd" call will be closed.
    (there will be also some exception)

Seems the unusual call transfer case is not tested/implemented.

But it could be, for example CISCO implemented the case: "https://www.cisco.com/c/en/us/td/docs/ios/voice/sip/configuration/guide/12_4t/sip_12_4t_book/sip_cg-call_x-fer.pdf", Page 8. Figure 2.

Proposed fix

Don't send NOTIFY and returns without error, if dialog (that receive REFER) is terminated.
So dialog termination works as unsubscribe for NOTIFY subscription.

File: ReferNotifier.js Function: notify

`
notify(code, reason)
{
debug('notify()');

if (this._active === false)
{
  return;
}

//----------- fix begin ------------ 
if( this._session.status === this._session.C.STATUS_TERMINATED) 
{
	this._active = false;
	return;
}	
//----------- fix end --------------

reason = reason || JsSIP_C.REASON_PHRASE[code] || '';

`
@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

The last NOTIFY arrived to 1st phone should simply not reach the dialog since it should not exist, in case this was terminated.

Please attach here the full JsSIP logs for 1st phone.

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

Ok, then provide us the full JsSIP logs for the 2nd phone.

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

Only logs are needed, not any .js file.

The issue is that JsSIP is trying to send a NOTIFY upon failure event. By that moment the RTCSession is already closed.

Anyway is not clear to me why the session is failing. Please log the error message on the RTCSession::failed event callback and attach the logs for the 2nd phone.

@ibc
Copy link
Member

ibc commented Dec 24, 2018

The BYE from phone2 to phone3 has 488 reason:

15:44:33.639 jssip.js:26610 sending message: 

BYE sip:[email protected]:443;transport=wss SIP/2.0 
Via: SIP/2.0/WSS sl69lvifk4ak.invalid;branch=z9hG4bK4638329 
Max-Forwards: 69 
To: <sip:[email protected]>;tag=mop1o7bhvo 
From: "Web2" <sip:[email protected]>;tag=8d3q28jvla 
Call-ID: 8r16j37smnl8j76n64m9 
CSeq: 8503 BYE 
Reason: SIP ;cause=488; text="Not Acceptable Here" 
Allow: INVITE,ACK,CANCEL,BYE,UPDATE,MESSAGE,OPTIONS,REFER,INFO 
Supported: outbound 
User-Agent: AudioCodes WebRTC phone 
Content-Length: 0

However I don't know if such a 488 "Not Acceptable Here" is caused due the NOTIFY failure.

15:44:33.640 jssip.js:26610 send() 
15:44:33.640 jssip.js:26610 session failed 
15:44:33.641 jssip.js:26610 close() 
15:44:33.642 jssip.js:26610 emit "failed" 
15:44:33.642 jssip.js:26610 notify() 
15:44:33.643 jssip.js:26610 sendRequest() 
15:44:33.644 jssip.js:18113 Uncaught (in promise) TypeError: Cannot read property 'sendRequest' of undefined 
at RTCSession.sendRequest (jssip.js:18113) 
at ReferNotifier.notify (jssip.js:20566) 
at RTCSession.<anonymous> (jssip.js:18989) 
at emitOne (jssip.js:25814) 
at RTCSession.emit (jssip.js:25887) 
at RTCSession._failed (jssip.js:20051)

@versatica versatica deleted a comment from ikq Dec 24, 2018
jmillan added a commit that referenced this issue Dec 24, 2018
@jmillan
#556. RTCSession. Emit private '_failed' event before closing
e3fdf42
@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

Please @ikq,

Build and try JsSIP master branch. It should be fixed.

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

Let us know if master branch is working as expected.

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@jmillan
Copy link
Member

jmillan commented Dec 24, 2018 •
edited
Loading

The change was made to master branch and there is no new release yet.

Have you built JsSIP out of master and tried?

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@jmillan
Copy link
Member

jmillan commented Dec 24, 2018

Follow the building instructions and use the resulting .js in /dist folder on your app.

@ikq
Copy link
Author

ikq commented Dec 24, 2018 via email •
edited
Loading

@ikq
Copy link
Author

ikq commented Dec 24, 2018 •
edited
Loading

The problem can be seen directly in debugger if set breakpoint as I described before in Chrome dev tools.
I set the breakpoint after hangup 1st call and before accept call in 3rd phone.

If you need testing, i'm happy to help
Regards,
ikq.

@ikq
Copy link
Author

ikq commented Dec 25, 2018 •
edited
Loading

Other proposed fix, according yours comment : "// Put this in a try/catch block."

File ReferNotifier.js: Method: notify()

` notify(code, reason)
{
debug('notify()');

if (this._active === false)
{
  return;
}

reason = reason || JsSIP_C.REASON_PHRASE[code] || '';

let state;

if (code >= 200)
{
  state = 'terminated;reason=noresource';
}
else
{
  state = `active;expires=${this._expires}`;
}

// Put this in a try/catch block.
try {
  this._session.sendRequest(JsSIP_C.NOTIFY, {
    extraHeaders : [
      `Event: ${C.event_type};id=${this._id}`,
      `Subscription-State: ${state}`,
      `Content-Type: ${C.body_type}`
    ],
    body          : `SIP/2.0 ${code} ${reason}`,
    eventHandlers : {
    // If a negative response is received, subscription is canceled.
      onErrorResponse() { this._active = false; }
    }
  });
} catch(e){
	debug('sendRequest exception - ignored', e);
}

}
`

You can see in the phone2 log that the fix described problem.
During sending NOTIFY was exception, but it was catched and ignored.

. . . . . ..
13:43:41.546 jssip.js:26614 session accepted
13:43:41.546 jssip.js:26614 emit "accepted"
13:43:41.546 jssip.js:26614 notify()
13:43:41.546 jssip.js:26614 sendRequest()
13:43:41.556 jssip.js:26614 sendRequest exception - ignored TypeError: Cannot read property 'sendRequest' of undefined
at RTCSession.sendRequest (jssip.js:18113)
at ReferNotifier.notify (jssip.js:20567)
at RTCSession. (jssip.js:18982)
at emitOne (jssip.js:25818)
at RTCSession.emit (jssip.js:25891)
at RTCSession._accepted (jssip.js:20012)
at jssip.js:19330
13:43:41.557 jssip.js:26614 sendRequest()
13:43:41.559 jssip.js:26614 send()
13:43:41.560 jssip.js:26614 sending message:

ACK sip:[email protected]:443;transport=wss SIP/2.0
. . . . . .

@ibc
Copy link
Member

ibc commented Dec 25, 2018

Could you please avoid so large comments? We don't need to see the entire and repeated log in every new response.

@ikq
Copy link
Author

ikq commented Dec 25, 2018 •
edited
Loading

OK, edited and removed.

@versatica versatica deleted a comment from heimiao Aug 5, 2019
@versatica versatica deleted a comment from heimiao Aug 9, 2019
@versatica versatica deleted a comment from heimiao Aug 9, 2019
@jmillan
Copy link
Member

jmillan commented Aug 12, 2019

@ikq,

Can you please send a full JsSIP log with the latest version showing this issue happening?

@ikq
Copy link
Author

ikq commented Aug 12, 2019 •
edited
Loading

The issue is happening in 3.3.6 version

Scenario:
web1 call to web3
web3 accept call

web3 initiate blind transfer to web2 (send REFER)
web1 send NOTIFY to web3
web1 calling to web2

web3 hangup call (send BYE)

web2 accept call
web1 try to send NOTIFY to web3 and throws exception.
web1 send BYE to web2

Here full log with original jssip 3.3.6. You can see exception at end of log.

Note:
In log web1 BYE shown before exception, but really it initiated by the exception.

If use jssip where "this._session.sendRequest(JsSIP_C.NOTIFY, ... " enclosed to try / catch
and ignore catched exception, web1 to web2 call will continue.

blind_transfer.log

@ikq
Copy link
Author

ikq commented Aug 12, 2019 •
edited
Loading

The issue is happening also in the latest 3.3.7 version
blind_transfer_3.3.7.txt

@heimiao
Copy link

heimiao commented Aug 13, 2019 via email

lcl987512 pushed a commit to lcl987512/JsSIP that referenced this issue Aug 21, 2020
@jmillan @lcl987512
versatica#556. RTCSession. Emit private '_failed' event before closing
4ccb432
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ibc @jmillan @ikq @heimiao