mirror of
https://github.com/jambonz/jambonz-feature-server.git
synced 2026-10-03 17:54:12 +00:00
fix(status): don't re-send pre-transfer call statuses after a transfer (#1584)
A call transferred between feature servers arrives at the receiving server as a fresh INVITE, so the session runs through trying, ringing, early-media and answered again as it accepts the REFER. The application was already told all of that by the server that first handled the call, so it sees a second round of events for a call it believes is long since answered. Skip only the application-facing status callback for those four statuses when the session is a transferred call. callInfo, the redis call record and record-all-calls still run exactly as before, so the call record stays accurate and answering a transferred call still starts recording. Fixes #1392 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6cc3dc45f2
commit
a4e7afac83
@@ -482,6 +482,24 @@ class CallSession extends Emitter {
|
||||
return this.application.transferredCall === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* returns true if this status was already reported to the application by the feature
|
||||
* server that handled the call before it was transferred here.
|
||||
*
|
||||
* A transferred call arrives as a fresh INVITE, so this session runs through trying,
|
||||
* ringing and answered again as it accepts the REFER. The application has already seen
|
||||
* all of those from the original server, so sending them a second time looks like
|
||||
* duplicate events for a call it believes is long since answered.
|
||||
*/
|
||||
_isStatusAlreadyReportedBeforeTransfer(callStatus) {
|
||||
return this.isTransferredCall && [
|
||||
CallStatus.Trying,
|
||||
CallStatus.Ringing,
|
||||
CallStatus.EarlyMedia,
|
||||
CallStatus.InProgress
|
||||
].includes(callStatus);
|
||||
}
|
||||
|
||||
/**
|
||||
* returns true if this session is an inbound call session
|
||||
*/
|
||||
@@ -3151,7 +3169,16 @@ Duration=${duration} `
|
||||
sipReasonHeader ?? reasonHeaderFromSipMessage(msg));
|
||||
if (typeof duration === 'number') this.callInfo.duration = duration;
|
||||
if (headers) this.callInfo.sipHeaders = headers;
|
||||
this.executeStatusCallback(callStatus, sipStatus);
|
||||
|
||||
/* callInfo and the redis call record are still updated below; only the
|
||||
application-facing notification is skipped */
|
||||
if (this._isStatusAlreadyReportedBeforeTransfer(callStatus)) {
|
||||
this.logger.debug({callStatus},
|
||||
'CallSession:_notifyCallStatusChange - suppressing status already sent before transfer');
|
||||
}
|
||||
else {
|
||||
this.executeStatusCallback(callStatus, sipStatus);
|
||||
}
|
||||
|
||||
// update calls db
|
||||
//this.logger.debug(`updating redis with ${JSON.stringify(this.callInfo)}`);
|
||||
|
||||
@@ -0,0 +1,103 @@
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert');
|
||||
|
||||
/* call-session decrypts credentials at require time, so it needs a secret present */
|
||||
process.env.ENCRYPTION_SECRET = process.env.ENCRYPTION_SECRET || 'foobar';
|
||||
process.env.JAMBONES_LOGLEVEL = process.env.JAMBONES_LOGLEVEL || 'error';
|
||||
|
||||
const CallSession = require('../../lib/session/call-session');
|
||||
const {CallStatus} = require('../../lib/utils/constants');
|
||||
|
||||
/* a CallSession is far too entangled to construct here, and none of what
|
||||
_notifyCallStatusChange touches needs the constructor to have run */
|
||||
const makeSession = ({transferredCall = false, recordAllCalls = false} = {}) => {
|
||||
const calls = {statusCallback: [], redis: [], callInfo: [], recorderStarted: 0, recorderStopped: 0};
|
||||
const session = Object.create(CallSession.prototype);
|
||||
|
||||
Object.assign(session, {
|
||||
callMoved: false,
|
||||
notifiedComplete: false,
|
||||
serviceUrl: 'http://127.0.0.1:3000',
|
||||
application: {transferredCall, record_all_calls: false},
|
||||
accountInfo: {account: {record_all_calls: recordAllCalls}},
|
||||
backgroundTaskManager: {
|
||||
newTask: (name) => {
|
||||
if (name === 'record') calls.recorderStarted++;
|
||||
},
|
||||
stop: (name) => {
|
||||
if (name === 'record') calls.recorderStopped++;
|
||||
}
|
||||
},
|
||||
callInfo: {
|
||||
updateCallStatus: (...args) => calls.callInfo.push(args),
|
||||
toJSON: () => ({callStatus: 'x'})
|
||||
},
|
||||
logger: {debug: () => {}, info: () => {}, error: () => {}},
|
||||
executeStatusCallback: (callStatus, sipStatus) => calls.statusCallback.push({callStatus, sipStatus}),
|
||||
updateCallStatus: async (obj) => {
|
||||
calls.redis.push(obj);
|
||||
}
|
||||
});
|
||||
|
||||
return {session, calls};
|
||||
};
|
||||
|
||||
const notified = (calls) => calls.statusCallback.map(({callStatus}) => callStatus);
|
||||
|
||||
const EARLY_STATUSES = [
|
||||
[CallStatus.Trying, 100],
|
||||
[CallStatus.Ringing, 180],
|
||||
[CallStatus.EarlyMedia, 183],
|
||||
[CallStatus.InProgress, 200]
|
||||
];
|
||||
|
||||
test('a transferred call does not re-notify statuses the first server already sent', async () => {
|
||||
const {session, calls} = makeSession({transferredCall: true});
|
||||
|
||||
for (const [callStatus, sipStatus] of EARLY_STATUSES) {
|
||||
await session._notifyCallStatusChange({callStatus, sipStatus});
|
||||
}
|
||||
|
||||
assert.deepStrictEqual(notified(calls), [],
|
||||
'no early status should reach the application for a transferred call');
|
||||
});
|
||||
|
||||
test('a normal call still notifies every one of those statuses', async () => {
|
||||
const {session, calls} = makeSession({transferredCall: false});
|
||||
|
||||
for (const [callStatus, sipStatus] of EARLY_STATUSES) {
|
||||
await session._notifyCallStatusChange({callStatus, sipStatus});
|
||||
}
|
||||
|
||||
assert.deepStrictEqual(notified(calls), EARLY_STATUSES.map(([s]) => s),
|
||||
'suppression must apply only to transferred calls');
|
||||
});
|
||||
|
||||
test('a transferred call still notifies completion', async () => {
|
||||
const {session, calls} = makeSession({transferredCall: true});
|
||||
|
||||
await session._notifyCallStatusChange({callStatus: CallStatus.Completed, sipStatus: 200, duration: 12});
|
||||
|
||||
assert.deepStrictEqual(notified(calls), [CallStatus.Completed],
|
||||
'only the statuses sent before the transfer are duplicates');
|
||||
});
|
||||
|
||||
test('suppressing the notification still updates callInfo and the redis call record', async () => {
|
||||
const {session, calls} = makeSession({transferredCall: true});
|
||||
|
||||
await session._notifyCallStatusChange({callStatus: CallStatus.InProgress, sipStatus: 200});
|
||||
|
||||
assert.strictEqual(calls.statusCallback.length, 0);
|
||||
assert.strictEqual(calls.callInfo.length, 1, 'callInfo must still track the status');
|
||||
assert.strictEqual(calls.redis.length, 1, 'the call record must still be written to redis');
|
||||
});
|
||||
|
||||
test('suppressing the notification still starts record-all-calls', async () => {
|
||||
const {session, calls} = makeSession({transferredCall: true, recordAllCalls: true});
|
||||
|
||||
await session._notifyCallStatusChange({callStatus: CallStatus.InProgress, sipStatus: 200});
|
||||
|
||||
assert.strictEqual(calls.statusCallback.length, 0);
|
||||
assert.strictEqual(calls.recorderStarted, 1,
|
||||
'answering a transferred call must still start recording when record_all_calls is set');
|
||||
});
|
||||
Reference in New Issue
Block a user