mirror of
https://github.com/jambonz/jambonz-feature-server.git
synced 2026-10-04 02:04:12 +00:00
Fix/siprec survives fs transfer (#1586)
* fix: carry siprec recording state across a feature server transfer transferCallToFeatureServer() stored the application, callInfo and remaining tasks, but not the SIPREC recording state, so the receiving session started at RecordingOff while the SBC was still recording. From there every live call control on the recording was rejected locally before an INFO was ever sent: stop and pause fail the guards in notifyRecordOptions, and a fresh startCallRecording is refused by the SBC as a duplicate. Carry recordOptions and the record state through the transfer and restore both on the receiving session, which also keeps propagateAnswer from issuing a second startCallRecording for a call that is already being recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: add standalone smoke test for siprec across a feature server transfer Drives the failing flow end to end - answer, start SIPREC, enqueue, create the agent call on a different feature server, dequeue by callSid - while acting as the SIPREC recorder, so it can tell whether the recording kept receiving media across the REFER and whether it was BYEd when the call ended. Needs two or more feature servers and a real inbound call, so it lives outside the mocha suite and is run by hand; npm test does not pick it up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: let the siprec smoke test originate both call legs itself Waiting for a human to dial in with music playing made the test hard to run and easy to make meaningless (a muted caller sends no RTP, so "the recording is silent" proves nothing). Add a built-in SIP endpoint that answers and streams a PCMU tone, and have the test place both legs through createCall, so a run needs only an account_sid and no carrier, DID or softphone. That makes the default mode exercise the transfer path in sbc-outbound; the previous flow is kept as SMOKE_MODE=inbound for the sbc-inbound path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: hand the inherited recording state to one session only Review of the previous commit turned up two ways the carried state leaks, both because it was read off the shared application object and left there: - a call transferred twice kept a stale siprecRecording from the first hop. transferCallToFeatureServer copies cs.application, and the new guard only wrote the key, so a call recorded FS1->FS2, stopped there, then moved to FS3 arrived believing it was still recording: startCallRecording rejected as "already started", stopCallRecording sent an INFO for a session that was gone. - child legs got it too. dial passes cs.application straight to ConfirmCallSession and place-outdial spreads it for the adulting session, so those sessions came up with recordState=recording_on and the parent's SRS options while their own dialog had no siprec session at all. The transferred session now takes the state off the application as it adopts it, so exactly one session owns it. Also fixes the smoke test's silence measurement, which was computed per stream and reported a whole quiet window as a gap - a legitimately silent second stream failed the run - and splits the README's requirements by mode, since the default rest mode needs no portal application and no external caller. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: drop the standalone smoke script, the harness test covers it smoke-tests/siprec-fs-transfer was a self-contained driver for the cross-feature -server SIPREC flow, written before the same scenario landed in the smoke-tester harness as TestVerb_Siprec_SurvivesFeatureServerTransfer (plus a REST-leg variant for the sbc-outbound path). The harness version is the one that runs in CI, asserts the recorded audio with Deepgram rather than counting packets, and is what proved both fixes on a real cluster - keeping a second implementation of the same flow here only invites the two to drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f29aa0faef
commit
6cc3dc45f2
@@ -106,6 +106,16 @@ class CallSession extends Emitter {
|
||||
assert(rootSpan);
|
||||
|
||||
this._recordState = RecordState.RecordingOff;
|
||||
|
||||
/* A call transferred from another feature server is still being recorded by the SBC, so
|
||||
this session inherits that state - and takes it off the application, which is handed
|
||||
on to child legs (dial confirm, adulting) and to any later transfer. */
|
||||
const inheritedRecording = application?.siprecRecording;
|
||||
if (inheritedRecording) {
|
||||
delete application.siprecRecording;
|
||||
this._recordState = inheritedRecording.state || RecordState.RecordingOff;
|
||||
if (inheritedRecording.options) this.recordOptions = inheritedRecording.options;
|
||||
}
|
||||
this._notifyEvents = false;
|
||||
|
||||
this.tmpFiles = new Set();
|
||||
|
||||
+7
-1
@@ -3,7 +3,7 @@ const crypto = require('crypto');
|
||||
const {TaskPreconditions} = require('../utils/constants');
|
||||
const { normalizeJambones } = require('@jambonz/verb-specifications');
|
||||
const WsRequestor = require('../utils/ws-requestor');
|
||||
const {TaskName} = require('../utils/constants');
|
||||
const {TaskName, RecordState} = require('../utils/constants');
|
||||
const {trace} = require('@opentelemetry/api');
|
||||
|
||||
/**
|
||||
@@ -345,6 +345,12 @@ class Task extends Emitter {
|
||||
delete obj.notifier;
|
||||
obj.tasks = cs.getRemainingTaskData();
|
||||
obj.callInfo = cs.callInfo.toJSON();
|
||||
|
||||
/* the SBC keeps the siprec session across the REFER, so the receiving session must
|
||||
inherit the state or pause/resume/stop there will be rejected as out of sync */
|
||||
if (cs.recordState !== RecordState.RecordingOff) {
|
||||
obj.siprecRecording = {state: cs.recordState, options: cs.recordOptions};
|
||||
}
|
||||
if (opts && obj.tasks.length > 0) {
|
||||
const key = Object.keys(obj.tasks[0])[0];
|
||||
Object.assign(obj.tasks[0][key], {_: opts});
|
||||
|
||||
Reference in New Issue
Block a user