fix: harden DTMF sending and always negotiate telephone-event codec (#8)

Guard sendDtmf against unready or throwing RTCDTMFSender instances so
one bad sender doesn't block others, and apply codec preferences
unconditionally on audio tracks so telephone-event is always offered
regardless of caller-provided codecPreferences. Also warn when an
audio SDP offer lacks telephone-event, since DTMF can never negotiate
for that session.

Co-authored-by: smoghe-bw <smoghe-bw>

Author: smoghe-bw

src/v1/bandwidthRtc.test.ts

@@ -61,8 +61,8 @@ describe("bandwidthRtcV1 sendDtmf", () => {
     setupMocks();
   });
 
-  function makeDtmfSender() {
-    return { insertDTMF: jest.fn() };
+  function makeDtmfSender(canInsertDTMF: boolean = true) {
+    return { insertDTMF: jest.fn(), canInsertDTMF };
   }
 
   test("calls insertDTMF on all registered senders when no streamId given", () => {
@@ -117,9 +117,41 @@ describe("bandwidthRtcV1 sendDtmf", () => {
 
     expect(sender.insertDTMF).not.toHaveBeenCalled();
   });
+
+  test("skips a sender that is not ready (canInsertDTMF false) without throwing", () => {
+    const brtc = new BandwidthRtc();
+    const notReady = makeDtmfSender(false);
+    const ready = makeDtmfSender(true);
+    (brtc as any).localDtmfSenders.set("stream-1", notReady);
+    (brtc as any).localDtmfSenders.set("stream-2", ready);
+
+    expect(() => brtc.sendDtmf("5")).not.toThrow();
+
+    expect(notReady.insertDTMF).not.toHaveBeenCalled();
+    expect(ready.insertDTMF).toHaveBeenCalledWith("5", 100, 70);
+  });
+
+  test("catches an insertDTMF error on one sender and still calls the others", () => {
+    const brtc = new BandwidthRtc();
+    const throwing = makeDtmfSender();
+    throwing.insertDTMF.mockImplementation(() => {
+      throw new DOMException("not ready", "InvalidStateError");
+    });
+    const healthy = makeDtmfSender();
+    (brtc as any).localDtmfSenders.set("stream-1", throwing);
+    (brtc as any).localDtmfSenders.set("stream-2", healthy);
+
+    expect(() => brtc.sendDtmf("5")).not.toThrow();
+
+    expect(healthy.insertDTMF).toHaveBeenCalledWith("5", 100, 70);
+  });
 });
 
 describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => {
+  afterEach(() => {
+    delete (global as any).RTCRtpSender;
+  });
+
   function makeTransceiver(dtmf: RTCDTMFSender | null = { insertDTMF: jest.fn() } as any) {
     return { sender: { dtmf }, setCodecPreferences: jest.fn() };
   }
@@ -191,6 +223,30 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => {
 
     expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]);
   });
+
+  test("forces telephone-event into codec preferences even without explicit codecPreferences", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
+    const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 };
+    (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [opusCodec, telephoneEventCodec] }) };
+
+    (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"));
+
+    expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]);
+  });
+
+  test("skips setCodecPreferences when RTCRtpSender is unavailable (e.g. non-browser environment)", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"))).not.toThrow();
+
+    expect(transceiver.setCodecPreferences).not.toHaveBeenCalled();
+  });
 });
 
 describe("bandwidthRtcV1 connect method", () => {

src/v1/bandwidthRtc.ts

@@ -292,10 +292,27 @@ export class BandwidthRtc {
    * @param interToneGap Gap between tones in milliseconds (default: 70). Minimum 30.
    */
   sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70) {
+    const insert = (dtmfSender: RTCDTMFSender, id: string) => {
+      if (!dtmfSender.canInsertDTMF) {
+        logger.warn(`sendDtmf: DTMF sender for stream ${id} is not ready (canInsertDTMF is false); skipping`);
+        return;
+      }
+      try {
+        dtmfSender.insertDTMF(tone, duration, interToneGap);
+      } catch (err) {
+        logger.warn(`sendDtmf: insertDTMF failed for stream ${id}`, err);
+      }
+    };
+
     if (streamId) {
-      this.localDtmfSenders.get(streamId)?.insertDTMF(tone, duration, interToneGap);
+      const dtmfSender = this.localDtmfSenders.get(streamId);
+      if (dtmfSender) {
+        insert(dtmfSender, streamId);
+      } else {
+        logger.warn(`sendDtmf: no DTMF sender registered for stream ${streamId}`);
+      }
     } else {
-      this.localDtmfSenders.forEach((dtmfSender) => dtmfSender.insertDTMF(tone, duration, interToneGap));
+      this.localDtmfSenders.forEach(insert);
     }
   }
 
@@ -386,6 +403,12 @@ export class BandwidthRtc {
         iceRestart: restartIce,
       });
 
+      // Diagnostic only: if an audio m-line is offered without telephone-event, DTMF
+      // can never negotiate for this session regardless of how long sendDtmf waits.
+      if (localSdpOffer.sdp?.includes("m=audio") && !localSdpOffer.sdp.includes(TELEPHONE_EVENT_MIME_TYPE.split("/")[1])) {
+        logger.warn("Publish SDP offer has an audio track but no telephone-event codec; DTMF will not be able to negotiate for this session");
+      }
+
       let publishMetadata = {
         mediaStreams: {},
         dataChannels: {},
@@ -675,23 +698,24 @@ export class BandwidthRtc {
         this.localDtmfSenders.set(mediaStream.id, dtmfSender);
       }
 
-      if (codecPreferences) {
-        if (track.kind === TRACK_KIND_AUDIO && codecPreferences.audio) {
-          // setCodecPreferences is a strict allowlist: any codec omitted from the
-          // list is dropped from the SDP offer. telephone-event must always be
-          // present so that RTCDTMFSender can send RFC 4733 DTMF packets.
-          const hasTelephoneEvent = codecPreferences.audio.some((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
+      if (track.kind === TRACK_KIND_AUDIO) {
+        // setCodecPreferences is a strict allowlist: any codec omitted from the list is
+        // dropped from the SDP offer. Apply it unconditionally (not just when the caller
+        // passes codecPreferences) so telephone-event is always present and RTCDTMFSender
+        // can send RFC 4733 DTMF packets, regardless of the browser's default codec offer.
+        const audioCapabilities = typeof RTCRtpSender !== "undefined" ? RTCRtpSender.getCapabilities(TRACK_KIND_AUDIO) : undefined;
+        const audioCodecs = codecPreferences?.audio ?? audioCapabilities?.codecs;
+        if (audioCodecs) {
+          const hasTelephoneEvent = audioCodecs.some((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
           if (!hasTelephoneEvent) {
-            const telephoneEventCodec = RTCRtpSender.getCapabilities(TRACK_KIND_AUDIO)?.codecs.find(
-              (c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE,
-            );
-            transceiver.setCodecPreferences(telephoneEventCodec ? [...codecPreferences.audio, telephoneEventCodec] : codecPreferences.audio);
+            const telephoneEventCodec = audioCapabilities?.codecs.find((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
+            transceiver.setCodecPreferences(telephoneEventCodec ? [...audioCodecs, telephoneEventCodec] : audioCodecs);
           } else {
-            transceiver.setCodecPreferences(codecPreferences.audio);
+            transceiver.setCodecPreferences(audioCodecs);
           }
-        } else if (track.kind === TRACK_KIND_VIDEO && codecPreferences.video) {
-          transceiver.setCodecPreferences(codecPreferences.video);
         }
+      } else if (track.kind === TRACK_KIND_VIDEO && codecPreferences?.video) {
+        transceiver.setCodecPreferences(codecPreferences.video);
       }
     });
   }