fix(sendDtmf): return success status and harden codec preference logic

README.md

@@ -89,6 +89,7 @@ Please see the following resources for more information on MediaStreamConstraint
   - streamId (optional): the stream to send the tone on; defaults to all published streams
   - duration (optional): tone duration in milliseconds, between 40 and 6000 (default: 100)
   - interToneGap (optional): gap between tones in milliseconds, minimum 30 (default: 70)
+- Returns: `true` if the tones were queued on at least one stream, `false` otherwise (e.g. no audio stream published yet, or the telephone-event codec has not been negotiated)
 
 ```javascript
 bandwidthRtc.sendDtmf("3");

src/bandwidthRtc.ts

@@ -211,7 +211,15 @@ class BandwidthRtc {
     return devices;
   }
 
-  sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70) {
+  /**
+   * Send DTMF tones on published audio streams via the browser's native RTCDTMFSender (RFC 4733).
+   * @param tone The DTMF tones to send - a string composed of the characters [0-9,*,#,A-D,\,]*
+   * @param streamId The optional stream id to send on; defaults to all published streams.
+   * @param duration Tone duration in milliseconds (default: 100). Must be between 40 and 6000.
+   * @param interToneGap Gap between tones in milliseconds (default: 70). Minimum 30.
+   * @returns true if the tones were queued on at least one stream, false otherwise
+   */
+  sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70): boolean {
     if (!this.delegate) {
       throw new BandwidthRtcError("You must call 'connect' before 'sendDtmf'");
     }

src/v1/bandwidthRtc.test.ts

@@ -103,17 +103,24 @@ describe("bandwidthRtcV1 sendDtmf", () => {
     expect(sender.insertDTMF).toHaveBeenCalledWith("1", 200, 80);
   });
 
-  test("does not throw when no senders are registered", () => {
+  test("returns true when tones are queued on at least one sender", () => {
     const brtc = new BandwidthRtc();
-    expect(() => brtc.sendDtmf("5")).not.toThrow();
+    (brtc as any).localDtmfSenders.set("stream-1", makeDtmfSender());
+
+    expect(brtc.sendDtmf("5")).toBe(true);
+  });
+
+  test("returns false without throwing when no senders are registered", () => {
+    const brtc = new BandwidthRtc();
+    expect(brtc.sendDtmf("5")).toBe(false);
   });
 
-  test("does nothing for an unknown streamId", () => {
+  test("returns false for an unknown streamId", () => {
     const brtc = new BandwidthRtc();
     const sender = makeDtmfSender();
     (brtc as any).localDtmfSenders.set("stream-1", sender);
 
-    brtc.sendDtmf("5", "nonexistent");
+    expect(brtc.sendDtmf("5", "nonexistent")).toBe(false);
 
     expect(sender.insertDTMF).not.toHaveBeenCalled();
   });
@@ -125,12 +132,19 @@ describe("bandwidthRtcV1 sendDtmf", () => {
     (brtc as any).localDtmfSenders.set("stream-1", notReady);
     (brtc as any).localDtmfSenders.set("stream-2", ready);
 
-    expect(() => brtc.sendDtmf("5")).not.toThrow();
+    expect(brtc.sendDtmf("5")).toBe(true);
 
     expect(notReady.insertDTMF).not.toHaveBeenCalled();
     expect(ready.insertDTMF).toHaveBeenCalledWith("5", 100, 70);
   });
 
+  test("returns false when the only sender is not ready", () => {
+    const brtc = new BandwidthRtc();
+    (brtc as any).localDtmfSenders.set("stream-1", makeDtmfSender(false));
+
+    expect(brtc.sendDtmf("5")).toBe(false);
+  });
+
   test("catches an insertDTMF error on one sender and still calls the others", () => {
     const brtc = new BandwidthRtc();
     const throwing = makeDtmfSender();
@@ -141,7 +155,7 @@ describe("bandwidthRtcV1 sendDtmf", () => {
     (brtc as any).localDtmfSenders.set("stream-1", throwing);
     (brtc as any).localDtmfSenders.set("stream-2", healthy);
 
-    expect(() => brtc.sendDtmf("5")).not.toThrow();
+    expect(brtc.sendDtmf("5")).toBe(true);
 
     expect(healthy.insertDTMF).toHaveBeenCalledWith("5", 100, 70);
   });
@@ -150,6 +164,7 @@ describe("bandwidthRtcV1 sendDtmf", () => {
 describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => {
   afterEach(() => {
     delete (global as any).RTCRtpSender;
+    delete (global as any).RTCRtpReceiver;
   });
 
   function makeTransceiver(dtmf: RTCDTMFSender | null = { insertDTMF: jest.fn() } as any) {
@@ -184,12 +199,27 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => {
     expect((brtc as any).localDtmfSenders.has("stream-1")).toBe(false);
   });
 
-  test("appends telephone-event codec when missing from audio preferences", () => {
+  test("appends telephone-event codec from receiver capabilities when missing from audio preferences", () => {
     const brtc = new BandwidthRtc();
     const transceiver = makeTransceiver();
     withPublishingPeerConnection(brtc, transceiver);
 
     const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) };
+
+    const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
+    (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] });
+
+    expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]);
+  });
+
+  test("falls back to sender capabilities for telephone-event when receiver capabilities lack it", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) };
     (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) };
 
     const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
@@ -217,35 +247,90 @@ describe("bandwidthRtcV1 addStreamToPublishingPeerConnection", () => {
     withPublishingPeerConnection(brtc, transceiver);
 
     (global as any).RTCRtpSender = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [] }) };
 
     const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
     (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] });
 
     expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]);
   });
 
-  test("forces telephone-event into codec preferences even without explicit codecPreferences", () => {
+  test("does not call setCodecPreferences when no codecPreferences are provided", () => {
     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] }) };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [opusCodec, telephoneEventCodec] }) };
 
     (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"));
 
-    expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec, telephoneEventCodec]);
+    expect(transceiver.setCodecPreferences).not.toHaveBeenCalled();
   });
 
-  test("skips setCodecPreferences when RTCRtpSender is unavailable (e.g. non-browser environment)", () => {
+  test("applies audio preferences as-is when capability APIs are 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();
+    const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
+    expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow();
 
-    expect(transceiver.setCodecPreferences).not.toHaveBeenCalled();
+    expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([opusCodec]);
+  });
+
+  test("retries with the caller's preferences when the telephone-event-augmented list is rejected", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) };
+
+    transceiver.setCodecPreferences.mockImplementationOnce(() => {
+      throw new DOMException("RTCRtpCodec sdpFmtLine badly formated", "InvalidModificationError");
+    });
+
+    const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
+    expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow();
+
+    expect(transceiver.setCodecPreferences).toHaveBeenNthCalledWith(1, [opusCodec, telephoneEventCodec]);
+    expect(transceiver.setCodecPreferences).toHaveBeenNthCalledWith(2, [opusCodec]);
+  });
+
+  test("falls back to browser default codecs without throwing when setCodecPreferences always rejects", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    const telephoneEventCodec = { mimeType: "audio/telephone-event", clockRate: 8000 };
+    (global as any).RTCRtpReceiver = { getCapabilities: jest.fn().mockReturnValue({ codecs: [telephoneEventCodec] }) };
+
+    transceiver.setCodecPreferences.mockImplementation(() => {
+      throw new DOMException("codecs do not match capabilities", "InvalidModificationError");
+    });
+
+    const opusCodec = { mimeType: "audio/opus", clockRate: 48000 };
+    expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "audio"), { audio: [opusCodec] })).not.toThrow();
+
+    expect(transceiver.setCodecPreferences).toHaveBeenCalledTimes(2);
+  });
+
+  test("does not throw when video codec preferences are rejected", () => {
+    const brtc = new BandwidthRtc();
+    const transceiver = makeTransceiver();
+    withPublishingPeerConnection(brtc, transceiver);
+
+    transceiver.setCodecPreferences.mockImplementation(() => {
+      throw new DOMException("codecs do not match capabilities", "InvalidModificationError");
+    });
+
+    const vp8Codec = { mimeType: "video/VP8", clockRate: 90000 };
+    expect(() => (brtc as any).addStreamToPublishingPeerConnection(makeMockStream("stream-1", "video"), { video: [vp8Codec] })).not.toThrow();
+
+    expect(transceiver.setCodecPreferences).toHaveBeenCalledWith([vp8Codec]);
   });
 });
 

src/v1/bandwidthRtc.ts

@@ -290,30 +290,42 @@ export class BandwidthRtc {
    * @param streamId The optional stream id to send on; defaults to all published streams.
    * @param duration Tone duration in milliseconds (default: 100). Must be between 40 and 6000.
    * @param interToneGap Gap between tones in milliseconds (default: 70). Minimum 30.
+   * @returns true if the tones were queued on at least one stream, false otherwise
    */
-  sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70) {
-    const insert = (dtmfSender: RTCDTMFSender, id: string) => {
+  sendDtmf(tone: string, streamId?: string, duration: number = 100, interToneGap: number = 70): boolean {
+    const insert = (dtmfSender: RTCDTMFSender, id: string): boolean => {
       if (!dtmfSender.canInsertDTMF) {
         logger.warn(`sendDtmf: DTMF sender for stream ${id} is not ready (canInsertDTMF is false); skipping`);
-        return;
+        return false;
       }
       try {
         dtmfSender.insertDTMF(tone, duration, interToneGap);
+        return true;
       } catch (err) {
         logger.warn(`sendDtmf: insertDTMF failed for stream ${id}`, err);
+        return false;
       }
     };
 
     if (streamId) {
       const dtmfSender = this.localDtmfSenders.get(streamId);
       if (dtmfSender) {
-        insert(dtmfSender, streamId);
-      } else {
-        logger.warn(`sendDtmf: no DTMF sender registered for stream ${streamId}`);
+        return insert(dtmfSender, streamId);
       }
-    } else {
-      this.localDtmfSenders.forEach(insert);
+      logger.warn(`sendDtmf: no DTMF sender registered for stream ${streamId}`);
+      return false;
+    }
+
+    if (this.localDtmfSenders.size === 0) {
+      logger.warn("sendDtmf: no DTMF senders registered; has an audio stream been published?");
+      return false;
     }
+
+    let sent = false;
+    this.localDtmfSenders.forEach((dtmfSender, id) => {
+      sent = insert(dtmfSender, id) || sent;
+    });
+    return sent;
   }
 
   /**
@@ -698,28 +710,79 @@ export class BandwidthRtc {
         this.localDtmfSenders.set(mediaStream.id, dtmfSender);
       }
 
-      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 = audioCapabilities?.codecs.find((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
-            transceiver.setCodecPreferences(telephoneEventCodec ? [...audioCodecs, telephoneEventCodec] : audioCodecs);
-          } else {
-            transceiver.setCodecPreferences(audioCodecs);
-          }
-        }
+      // Only restrict codecs when the caller explicitly asks for it. Every browser
+      // that supports RTCDTMFSender already includes telephone-event in its default
+      // audio offer, and setCodecPreferences behaves very differently across WebKit
+      // versions (throwing on some, silently dropping codecs on others), so touching
+      // it in the default path only adds risk.
+      if (track.kind === TRACK_KIND_AUDIO && codecPreferences?.audio) {
+        this.applyAudioCodecPreferences(transceiver, codecPreferences.audio);
       } else if (track.kind === TRACK_KIND_VIDEO && codecPreferences?.video) {
-        transceiver.setCodecPreferences(codecPreferences.video);
+        this.trySetCodecPreferences(transceiver, codecPreferences.video, TRACK_KIND_VIDEO);
       }
     });
   }
 
+  /**
+   * Apply caller-provided audio codec preferences, keeping telephone-event in the
+   * list so RTCDTMFSender can negotiate RFC 4733 DTMF. setCodecPreferences is a
+   * strict allowlist: any codec omitted from the list is dropped from the SDP offer.
+   */
+  private applyAudioCodecPreferences(transceiver: RTCRtpTransceiver, audioCodecs: RTCRtpCodec[]) {
+    const hasTelephoneEvent = audioCodecs.some((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
+    if (!hasTelephoneEvent) {
+      const telephoneEventCodec = this.findTelephoneEventCodec();
+      if (telephoneEventCodec) {
+        if (this.trySetCodecPreferences(transceiver, [...audioCodecs, telephoneEventCodec], TRACK_KIND_AUDIO)) {
+          return;
+        }
+      } else {
+        logger.warn(
+          "telephone-event codec not found in this browser's capabilities; applying audio codec preferences as-is, DTMF may not be able to negotiate",
+        );
+      }
+    }
+    this.trySetCodecPreferences(transceiver, audioCodecs, TRACK_KIND_AUDIO);
+  }
+
+  /**
+   * Per the WebRTC spec, codecs passed to setCodecPreferences must come from the
+   * receiver's capabilities; older engines matched against the sender's, so check both.
+   */
+  private findTelephoneEventCodec(): RTCRtpCodec | undefined {
+    const capabilityCodecs = [
+      typeof RTCRtpReceiver !== "undefined" && typeof RTCRtpReceiver.getCapabilities === "function"
+        ? RTCRtpReceiver.getCapabilities(TRACK_KIND_AUDIO)?.codecs
+        : undefined,
+      typeof RTCRtpSender !== "undefined" && typeof RTCRtpSender.getCapabilities === "function"
+        ? RTCRtpSender.getCapabilities(TRACK_KIND_AUDIO)?.codecs
+        : undefined,
+    ];
+    for (const codecs of capabilityCodecs) {
+      const telephoneEventCodec = codecs?.find((c) => c.mimeType.toLowerCase() === TELEPHONE_EVENT_MIME_TYPE);
+      if (telephoneEventCodec) {
+        return telephoneEventCodec;
+      }
+    }
+    return undefined;
+  }
+
+  /**
+   * setCodecPreferences must never break publishing: WebKit's matching rules vary
+   * by version (case sensitivity, receiver-only capability matching, strict
+   * sdpFmtpLine parsing) and a failure here just means the browser's default codec
+   * offer is used instead.
+   */
+  private trySetCodecPreferences(transceiver: RTCRtpTransceiver, codecs: RTCRtpCodec[], kind: string): boolean {
+    try {
+      transceiver.setCodecPreferences(codecs);
+      return true;
+    } catch (err) {
+      logger.warn(`setCodecPreferences failed for ${kind} track; falling back to browser default codecs`, err);
+      return false;
+    }
+  }
+
   private cleanupPublishedStreams(...streams: PublishedStream[]) {
     logger.debug(`cleanupPublishedStreams: ${streams}`);
     if (streams.length === 0) {