Skip to content

fix: start local audio level observer after Krisp track swap (VAP-17046) #175

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kyle-vapi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: start local audio level observer after Krisp track swap (VAP-17046) #175

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions __tests__/vapi.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,3 +420,138 @@ describe("Vapi audio processing failures", () => {
expect(emitted?.error?.message).toBe("Canceled");
});
});

// Daily tears the local audio level observer down on any local track change, and
// its teardown closes the AudioContext that the observer's in-flight
// audioWorklet.addModule() is still loading into. Chrome and Firefox reject that
// load ("AbortError: Unable to load a worklet's module") and Daily responds by
// stopping the observer for the whole call. Enabling noise cancellation swaps the
// microphone track, so starting the observer before that swap loses the race.
// Upstream: https://github.com/daily-co/daily-js/issues/317
describe("Vapi local audio level observer", () => {
const webCall = {
id: "call_test",
webCallUrl: "https://example.daily.co/test",
};

// A pending updateInputSettings() stands in for Krisp still initializing, so a
// test can assert what the SDK does on each side of the track swap.
function deferredInputSettings() {
let settle = () => {};
const pending = new Promise<void>((resolve) => {
settle = () => resolve();
});
return { updateInputSettings: jest.fn(() => pending), settle };
}

afterEach(() => {
mockDailyCall = null;
});

it("does not start the observer when nothing listens for local-volume-level", async () => {
mockDailyCall = createMockDailyCall(jest.fn().mockResolvedValue(undefined));
const vapi = new Vapi("dummy_token");

await vapi.start("dummy_assistant_id");
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).not.toHaveBeenCalled();
// The assistant's level is a separate observer and stays unconditional.
expect(
mockDailyCall.startRemoteParticipantsAudioLevelObserver
).toHaveBeenCalledWith(100);
});

it("starts the observer when a local-volume-level listener is registered", async () => {
mockDailyCall = createMockDailyCall(jest.fn().mockResolvedValue(undefined));
const vapi = new Vapi("dummy_token");
vapi.on("local-volume-level", () => {});

await vapi.start("dummy_assistant_id");
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).toHaveBeenCalledWith(100);
});

it("waits for the noise cancellation processor to settle before starting", async () => {
const { updateInputSettings, settle } = deferredInputSettings();
mockDailyCall = createMockDailyCall(updateInputSettings as jest.Mock);
const vapi = new Vapi("dummy_token");
vapi.on("local-volume-level", () => {});

await vapi.start("dummy_assistant_id");
await flushRejections();

// Krisp is still initializing: starting now is what loses the race.
expect(mockDailyCall.startLocalAudioLevelObserver).not.toHaveBeenCalled();

settle();
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).toHaveBeenCalledWith(100);
});

it("starts the observer even when noise cancellation fails", async () => {
const updateInputSettings = jest.fn(() => {
return Promise.reject(new Error("Canceled"));
});
mockDailyCall = createMockDailyCall(updateInputSettings as jest.Mock);
const vapi = new Vapi("dummy_token");
vapi.on("local-volume-level", () => {});
// EventEmitter rethrows out of emit('error') with no listener registered,
// which is reported separately from the observer start for that reason.
vapi.on("error", () => {});

await vapi.start("dummy_assistant_id");
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).toHaveBeenCalledWith(100);
});

it("reports a rejected observer start rather than letting it escape", async () => {
mockDailyCall = createMockDailyCall(jest.fn().mockResolvedValue(undefined));
mockDailyCall.startLocalAudioLevelObserver.mockRejectedValue(
new Error("Unable to load a worklet's module.")
);
const vapi = new Vapi("dummy_token");
vapi.on("local-volume-level", () => {});
const observerErrors: any[] = [];
vapi.on("local-audio-level-observer-error", (error) => {
observerErrors.push(error);
});

const call = await vapi.start("dummy_assistant_id");
await flushRejections();

// Non-fatal: the call still starts.
expect(call).not.toBeNull();
expect(observerErrors[0]?.message).toBe("Unable to load a worklet's module.");
});

it("does not start the observer on reconnect() when nothing listens", async () => {
mockDailyCall = createMockDailyCall(jest.fn().mockResolvedValue(undefined));
const vapi = new Vapi("dummy_token");

await vapi.reconnect(webCall);
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).not.toHaveBeenCalled();
});

it("waits for the processor to settle on reconnect() too", async () => {
const { updateInputSettings, settle } = deferredInputSettings();
mockDailyCall = createMockDailyCall(updateInputSettings as jest.Mock);
const vapi = new Vapi("dummy_token");
vapi.on("local-volume-level", () => {});

await vapi.reconnect(webCall);
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).not.toHaveBeenCalled();

settle();
await flushRejections();

expect(mockDailyCall.startLocalAudioLevelObserver).toHaveBeenCalledWith(100);
});
});
94 changes: 72 additions & 22 deletions vapi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,36 @@ export default class Vapi extends VapiEventEmitter {
});
}

/**
* Starts Daily's local (microphone) audio level observer.
*
* Only runs when something is listening for 'local-volume-level'. The observer
* costs an AudioContext plus an AudioWorklet for the lifetime of the call and
* nothing else in the SDK reads the level, so starting it unconditionally
* charges every consumer for a feature almost none of them use. Listeners
* attached after the call is under way should call the public
* `startLocalAudioLevelObserver()`.
*
* Must run after the noise-cancellation processor has settled. Krisp replaces
* the microphone track, Daily reacts to any local track change by closing the
* AudioContext its in-flight `audioWorklet.addModule()` is still loading into,
* and Chrome and Firefox reject that load with "AbortError: Unable to load a
* worklet's module". Daily answers by stopping the observer for the rest of
* the call, so losing this race costs the feature, not just console noise.
* Tracked upstream at https://github.com/daily-co/daily-js/issues/317.
*/
private async maybeStartLocalAudioLevelObserver(): Promise<void> {
if (!this.call || this.listenerCount('local-volume-level') === 0) {
return;
}

try {
await this.call.startLocalAudioLevelObserver(100);
} catch (error) {
this.emit('local-audio-level-observer-error', serializeError(error));
}
}

async start(
assistant?: CreateAssistantDTO | string,
assistantOverrides?: AssistantOverrides,
Expand Down Expand Up @@ -835,7 +865,6 @@ export default class Vapi extends VapiEventEmitter {

try {
this.call.startRemoteParticipantsAudioLevelObserver(100);
this.call.startLocalAudioLevelObserver(100);
const audioObserverDuration = Date.now() - audioObserverStartTime;
this.emit('call-start-progress', {
stage: 'audio-observer-setup',
Expand Down Expand Up @@ -905,17 +934,28 @@ export default class Vapi extends VapiEventEmitter {
const audioProcessingStartTime = Date.now();

try {
this.call
.updateInputSettings({
audio: {
processor: {
type: 'noise-cancellation',
},
const audioProcessingUpdate = this.call.updateInputSettings({
audio: {
processor: {
type: 'noise-cancellation',
},
})
.catch((error) => {
this.emitAudioProcessingError('audio-processing-setup', error);
});
},
});

audioProcessingUpdate.catch((error) => {
this.emitAudioProcessingError('audio-processing-setup', error);
});

// The observer waits for this update rather than starting alongside the
// remote one above, so that Krisp has already swapped the microphone
// track. See maybeStartLocalAudioLevelObserver().
//
// A separate chain, not another link on the one above: EventEmitter
// rethrows out of emit('error') when a consumer registered no 'error'
// listener, and whether the observer starts must not hinge on that.
audioProcessingUpdate
.catch(() => {})
.then(() => this.maybeStartLocalAudioLevelObserver());

const audioProcessingDuration = Date.now() - audioProcessingStartTime;
this.emit('call-start-progress', {
Expand Down Expand Up @@ -1758,7 +1798,6 @@ export default class Vapi extends VapiEventEmitter {

try {
this.call.startRemoteParticipantsAudioLevelObserver(100);
this.call.startLocalAudioLevelObserver(100);
const audioObserverDuration = Date.now() - audioObserverStartTime;
this.emit('call-start-progress', {
stage: 'audio-observer-setup',
Expand Down Expand Up @@ -1789,17 +1828,28 @@ export default class Vapi extends VapiEventEmitter {
const audioProcessingStartTime = Date.now();

try {
this.call
.updateInputSettings({
audio: {
processor: {
type: 'noise-cancellation',
},
const audioProcessingUpdate = this.call.updateInputSettings({
audio: {
processor: {
type: 'noise-cancellation',
},
})
.catch((error) => {
this.emitAudioProcessingError('audio-processing-setup', error);
});
},
});

audioProcessingUpdate.catch((error) => {
this.emitAudioProcessingError('audio-processing-setup', error);
});

// The observer waits for this update rather than starting alongside the
// remote one above, so that Krisp has already swapped the microphone
// track. See maybeStartLocalAudioLevelObserver().
//
// A separate chain, not another link on the one above: EventEmitter
// rethrows out of emit('error') when a consumer registered no 'error'
// listener, and whether the observer starts must not hinge on that.
audioProcessingUpdate
.catch(() => {})
.then(() => this.maybeStartLocalAudioLevelObserver());

const audioProcessingDuration = Date.now() - audioProcessingStartTime;
this.emit('call-start-progress', {
Expand Down
Loading