Skip to content

Commit 9953a32

Browse files
author
John Riordan
committed
New API - fix ACK error handling
ACK request failures are now getting handled. In particular failures associated with processing an SDP answer in an ACK.
1 parent 2136fcf commit 9953a32

File tree

4 files changed

+388
-118
lines changed

4 files changed

+388
-118
lines changed

‎src/api/session.ts

Lines changed: 64 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -527,12 +527,12 @@ export abstract class Session extends EventEmitter {
527527
* @internal
528528
*/
529529
public close(): void {
530+
this.logger.log(`Session[${this.id}].close`);
531+
530532
if (this.status === SessionStatus.STATUS_TERMINATED) {
531533
return;
532534
}
533535

534-
this.logger.log("closing INVITE session " + this.id);
535-
536536
// 1st Step. Terminate media.
537537
if (this._sessionDescriptionHandler) {
538538
this._sessionDescriptionHandler.close();
@@ -564,7 +564,7 @@ export abstract class Session extends EventEmitter {
564564
* they occur we ACK the response and send a BYE.
565565
* Note that the BYE is sent in the dialog associated with the response
566566
* which is not necessarily `this.dialog`. And, accordingly, the
567-
* session state is not transitioned to terminated.
567+
* session state is not transitioned to terminated and session is not closed.
568568
* @param inviteResponse - The response causing the error.
569569
* @param statusCode - Status code for he reason phrase.
570570
* @param reasonPhrase - Reason phrase for the BYE.
@@ -610,51 +610,76 @@ export abstract class Session extends EventEmitter {
610610
throw new Error("Dialog undefined.");
611611
}
612612

613-
// If the ACK doesn't have an offer/answer, nothing to be done.
614-
const body = getBody(request.message);
615-
if (!body) {
616-
this.status = SessionStatus.STATUS_CONFIRMED;
617-
return;
618-
}
619-
if (body.contentDisposition === "render") {
620-
this.renderbody = body.content;
621-
this.rendertype = body.contentType;
622-
this.status = SessionStatus.STATUS_CONFIRMED;
623-
return;
624-
}
625-
if (body.contentDisposition !== "session") {
626-
this.status = SessionStatus.STATUS_CONFIRMED;
627-
return;
628-
}
629-
630-
const options = {
631-
sessionDescriptionHandlerOptions: this.sessionDescriptionHandlerOptions,
632-
sessionDescriptionHandlerModifiers: this.sessionDescriptionHandlerModifiers
633-
};
634-
635613
switch (dialog.signalingState) {
636-
case SignalingState.Initial:
614+
case SignalingState.Initial: {
637615
// State should never be reached as first reliable response must have answer/offer.
638-
throw new Error(`Invalid signaling state ${dialog.signalingState}.`);
639-
case SignalingState.Stable:
640-
// Receved answer.
616+
// So we must have never has sent an offer.
617+
this.logger.error(`Invalid signaling state ${dialog.signalingState}.`);
618+
this.isFailed = true;
619+
const extraHeaders = ["Reason: " + Utils.getReasonHeaderValue(488, "Bad Media Description")];
620+
dialog.bye(undefined, { extraHeaders });
621+
this.stateTransition(SessionState.Terminated);
622+
this.close();
623+
return;
624+
}
625+
case SignalingState.Stable: {
626+
// State we should be in.
627+
// Either the ACK has the answer that got us here, or we were in this state prior to the ACK.
628+
const body = getBody(request.message);
629+
// If the ACK doesn't have an answer, nothing to be done.
630+
if (!body) {
631+
this.status = SessionStatus.STATUS_CONFIRMED;
632+
return;
633+
}
634+
if (body.contentDisposition === "render") {
635+
this.renderbody = body.content;
636+
this.rendertype = body.contentType;
637+
this.status = SessionStatus.STATUS_CONFIRMED;
638+
return;
639+
}
640+
if (body.contentDisposition !== "session") {
641+
this.status = SessionStatus.STATUS_CONFIRMED;
642+
return;
643+
}
644+
// Receved answer in ACK.
645+
const options = {
646+
sessionDescriptionHandlerOptions: this.sessionDescriptionHandlerOptions,
647+
sessionDescriptionHandlerModifiers: this.sessionDescriptionHandlerModifiers
648+
};
641649
this.setAnswer(body, options)
642650
.then(() => { this.status = SessionStatus.STATUS_CONFIRMED; })
643-
.catch((error: any) => {
644-
// FIXME: TODO - need to do something to handle this error
645-
this.logger.error(error);
651+
.catch((error: Error) => {
652+
this.logger.error(error.message);
653+
this.isFailed = true;
646654
const extraHeaders = ["Reason: " + Utils.getReasonHeaderValue(488, "Bad Media Description")];
647-
this.bye(undefined, { extraHeaders });
655+
dialog.bye(undefined, { extraHeaders });
656+
this.stateTransition(SessionState.Terminated);
648657
this.close();
649-
throw error;
650658
});
651659
return;
652-
case SignalingState.HaveLocalOffer:
653-
// State should never be reached as local offer would be answered by this ACK
654-
throw new Error(`Invalid signaling state ${dialog.signalingState}.`);
655-
case SignalingState.HaveRemoteOffer:
660+
}
661+
case SignalingState.HaveLocalOffer: {
662+
// State should never be reached as local offer would be answered by this ACK.
663+
// So we must have received an ACK without an answer.
664+
this.logger.error(`Invalid signaling state ${dialog.signalingState}.`);
665+
this.isFailed = true;
666+
const extraHeaders = ["Reason: " + Utils.getReasonHeaderValue(488, "Bad Media Description")];
667+
dialog.bye(undefined, { extraHeaders });
668+
this.stateTransition(SessionState.Terminated);
669+
this.close();
670+
return;
671+
}
672+
case SignalingState.HaveRemoteOffer: {
656673
// State should never be reached as remote offer would be answered in first reliable response.
657-
throw new Error(`Invalid signaling state ${dialog.signalingState}.`);
674+
// So we must have never has sent an answer.
675+
this.logger.error(`Invalid signaling state ${dialog.signalingState}.`);
676+
this.isFailed = true;
677+
const extraHeaders = ["Reason: " + Utils.getReasonHeaderValue(488, "Bad Media Description")];
678+
dialog.bye(undefined, { extraHeaders });
679+
this.stateTransition(SessionState.Terminated);
680+
this.close();
681+
return;
682+
}
658683
case SignalingState.Closed:
659684
throw new Error(`Invalid signaling state ${dialog.signalingState}.`);
660685
default:

‎test/spec/api/session-in-dialog.spec.ts

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -131,20 +131,39 @@ describe("API Session In-Dialog", () => {
131131
.then(() => alice.transport.waitSent()); // ACK
132132
});
133133

134-
it("her ua should send INVITE, ACK, BYE", () => {
135-
const spy = alice.transportSendSpy;
136-
expect(spy).toHaveBeenCalledTimes(3);
137-
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
138-
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
139-
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
140-
});
134+
if (withoutSdp) {
135+
it("her ua should send INVITE, ACK, BYE", () => {
136+
const spy = alice.transportSendSpy;
137+
expect(spy).toHaveBeenCalledTimes(3);
138+
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
139+
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
140+
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
141+
});
141142

142-
it("her ua should receive 200", () => {
143-
const spy = alice.transportReceiveSpy;
144-
expect(spy).toHaveBeenCalledTimes(2);
145-
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
146-
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
147-
});
143+
it("her ua should receive 200, 200", () => {
144+
const spy = alice.transportReceiveSpy;
145+
expect(spy).toHaveBeenCalledTimes(2);
146+
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
147+
expect(spy.calls.argsFor(1)).toEqual(SIP_200);
148+
});
149+
} else {
150+
it("her ua should send INVITE, ACK, BYE, 481", () => {
151+
const spy = alice.transportSendSpy;
152+
expect(spy).toHaveBeenCalledTimes(4);
153+
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
154+
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
155+
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
156+
expect(spy.calls.argsFor(3)).toEqual(SIP_481);
157+
});
158+
159+
it("her ua should receive 200, BYE, 481", () => {
160+
const spy = alice.transportReceiveSpy;
161+
expect(spy).toHaveBeenCalledTimes(3);
162+
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
163+
expect(spy.calls.argsFor(1)).toEqual(SIP_BYE);
164+
expect(spy.calls.argsFor(2)).toEqual(SIP_481);
165+
});
166+
}
148167

149168
it("her signaling should be closed", () => {
150169
if (!inviter.dialog) {
@@ -197,20 +216,39 @@ describe("API Session In-Dialog", () => {
197216
.then(() => alice.transport.waitSent()); // ACK
198217
});
199218

200-
it("her ua should send INVITE, ACK, BYE", () => {
201-
const spy = alice.transportSendSpy;
202-
expect(spy).toHaveBeenCalledTimes(3);
203-
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
204-
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
205-
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
206-
});
219+
if (withoutSdp) {
220+
it("her ua should send INVITE, ACK, BYE, 481", () => {
221+
const spy = alice.transportSendSpy;
222+
expect(spy).toHaveBeenCalledTimes(4);
223+
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
224+
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
225+
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
226+
expect(spy.calls.argsFor(3)).toEqual(SIP_481);
227+
});
207228

208-
it("her ua should receive 200", () => {
209-
const spy = alice.transportReceiveSpy;
210-
expect(spy).toHaveBeenCalledTimes(2);
211-
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
212-
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
213-
});
229+
it("her ua should receive 200, BYE, 481", () => {
230+
const spy = alice.transportReceiveSpy;
231+
expect(spy).toHaveBeenCalledTimes(3);
232+
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
233+
expect(spy.calls.argsFor(1)).toEqual(SIP_BYE);
234+
expect(spy.calls.argsFor(2)).toEqual(SIP_481);
235+
});
236+
} else {
237+
it("her ua should send INVITE, ACK, BYE", () => {
238+
const spy = alice.transportSendSpy;
239+
expect(spy).toHaveBeenCalledTimes(3);
240+
expect(spy.calls.argsFor(0)).toEqual(SIP_INVITE);
241+
expect(spy.calls.argsFor(1)).toEqual(SIP_ACK);
242+
expect(spy.calls.argsFor(2)).toEqual(SIP_BYE);
243+
});
244+
245+
it("her ua should receive 200, 200", () => {
246+
const spy = alice.transportReceiveSpy;
247+
expect(spy).toHaveBeenCalledTimes(2);
248+
expect(spy.calls.argsFor(0)).toEqual(SIP_200);
249+
expect(spy.calls.argsFor(1)).toEqual(SIP_200);
250+
});
251+
}
214252

215253
it("her signaling should be closed", () => {
216254
if (!inviter.dialog) {

0 commit comments

Comments
 (0)