Skip to content

Commit

Permalink
Fix #107. Stop spamming provisional responses
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmillan
Gavin Llewellyn authored and jmillan committed Jul 10, 2013
1 parent 7528f90 commit 3fc4efa
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "jssip",
"title": "JsSIP",
"description": "the Javascript SIP library",
"version": "0.3.6",
"version": "0.3.7",
"homepage": "http://jssip.net",
"author": "José Luis Millán <jmillan@aliax.net>",
"contributors": [
Expand Down
3 changes: 2 additions & 1 deletion src/Timers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ Timers = {
TIMER_J: 0 * T1,
TIMER_K: 0 * T4,
TIMER_L: 64 * T1,
TIMER_M: 64 * T1
TIMER_M: 64 * T1,
PROVISIONAL_RESPONSE_INTERVAL: 60000 // See RFC 3261 Section 13.3.1.1
};

JsSIP.Timers = Timers;
Expand Down
45 changes: 21 additions & 24 deletions src/Transactions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,29 +399,20 @@ var InviteServerTransactionPrototype = function() {

console.log(LOG_PREFIX +'transport error occurred, deleting INVITE server transaction ' + this.id);

window.clearTimeout(this.reliableProvisionalTimer);
if (this.resendProvisionalTimer !== null) {
window.clearInterval(this.resendProvisionalTimer);
this.resendProvisionalTimer = null;
}
window.clearTimeout(this.L);
window.clearTimeout(this.H);
window.clearTimeout(this.I);
delete this.ua.transactions.ist[this.id];
}
};

this.timer_reliableProvisional = function(retransmissions) {
var
tr = this,
response = this.last_response,
timeout = JsSIP.Timers.T1 * (Math.pow(2, retransmissions + 1));

if(retransmissions > 8) {
window.clearTimeout(this.reliableProvisionalTimer);
} else {
retransmissions += 1;
if(!this.transport.send(response)) {
this.onTransportError();
}
this.reliableProvisionalTimer = window.setTimeout(function() {
tr.timer_reliableProvisional(retransmissions);}, timeout);
this.resend_provisional = function() {
if(!this.transport.send(this.last_response)) {
this.onTransportError();
}
};

Expand All @@ -440,11 +431,11 @@ var InviteServerTransactionPrototype = function() {
}
}

if(status_code > 100 && status_code <= 199) {
// Trigger the reliableProvisionalTimer only for the first non 100 provisional response.
if(!this.reliableProvisionalTimer) {
this.reliableProvisionalTimer = window.setTimeout(function() {
tr.timer_reliableProvisional(1);}, JsSIP.Timers.T1);
if(status_code > 100 && status_code <= 199 && this.state === C.STATUS_PROCEEDING) {
// Trigger the resendProvisionalTimer only for the first non 100 provisional response.
if(this.resendProvisionalTimer === null) {
this.resendProvisionalTimer = window.setInterval(function() {
tr.resend_provisional();}, JsSIP.Timers.PROVISIONAL_RESPONSE_INTERVAL);
}
} else if(status_code >= 200 && status_code <= 299) {
switch(this.state) {
Expand All @@ -454,7 +445,10 @@ var InviteServerTransactionPrototype = function() {
this.L = window.setTimeout(function() {
tr.timer_L();
}, JsSIP.Timers.TIMER_L);
window.clearTimeout(this.reliableProvisionalTimer);
if (this.resendProvisionalTimer !== null) {
window.clearInterval(this.resendProvisionalTimer);
this.resendProvisionalTimer = null;
}
/* falls through */
case C.STATUS_ACCEPTED:
// Note that this point will be reached for proceeding tr.state also.
Expand All @@ -471,7 +465,10 @@ var InviteServerTransactionPrototype = function() {
} else if(status_code >= 300 && status_code <= 699) {
switch(this.state) {
case C.STATUS_PROCEEDING:
window.clearTimeout(this.reliableProvisionalTimer);
if (this.resendProvisionalTimer !== null) {
window.clearInterval(this.resendProvisionalTimer);
this.resendProvisionalTimer = null;
}
if(!this.transport.send(response)) {
this.onTransportError();
if (onFailure) {
Expand Down Expand Up @@ -564,7 +561,7 @@ Transactions.InviteServerTransaction = function(request, ua) {

ua.transactions.ist[this.id] = this;

this.reliableProvisionalTimer = null;
this.resendProvisionalTimer = null;

request.reply(100);
};
Expand Down

10 comments on commit 3fc4efa

@saghul
Copy link
Contributor

@saghul saghul commented on 3fc4efa Jul 10, 2013

Choose a reason for hiding this comment

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

Why did you raise the version in a bugfix patch?!

@jmillan
Copy link
Member

Choose a reason for hiding this comment

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

In order to avoid an extra commit for each commit in master

@saghul
Copy link
Contributor

@saghul saghul commented on 3fc4efa Jul 10, 2013

Choose a reason for hiding this comment

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

That's terrible. Also, why do we need to bump the version number with every commit?

@jmillan
Copy link
Member

Choose a reason for hiding this comment

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

This is master. A new commit means a bug has been fixed and goes with the corresponding version increase.

This is defined in the release policy

@saghul
Copy link
Contributor

@saghul saghul commented on 3fc4efa Jul 10, 2013

Choose a reason for hiding this comment

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

I don't think 1 commit == 1 release is defined anywhere. Please bump the version in separated commits, when a release is about to be made.

@ibc
Copy link
Member

@ibc ibc commented on 3fc4efa Jul 10, 2013

Choose a reason for hiding this comment

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

I agree with @saghul. There is no need to make a new release for each fixed bug.

@gavllew
Copy link
Contributor

Choose a reason for hiding this comment

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

It's useful to have some sort of release or stable branch, where a pull will always give you the most up-to-date (and well-tested) release. If master is intended for work-in-progress commits, perhaps an additional branch is needed?

@jmillan
Copy link
Member

Choose a reason for hiding this comment

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

Hi,

Such branch is develop. It's the next stable branch.

@jmillan
Copy link
Member

Choose a reason for hiding this comment

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

master branch contains the last stable version plus bugfixes and develop contains the next stable version.

@gavllew
Copy link
Contributor

Choose a reason for hiding this comment

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

But the above discussion suggests that 1 commit != 1 release, which means that the latest master commit may not be a released version. That's why I made the suggestion.

Please sign in to comment.