fix: start local audio level observer after Krisp track swap (VAP-17046) - #175
Open
kyle-vapi wants to merge 1 commit into
Open
Conversation
Gate startLocalAudioLevelObserver() behind a 'local-volume-level' listener and chain it after updateInputSettings() settles, so Krisp has already swapped the microphone track before the AudioWorklet loads. Previously the observer started before noise cancellation was applied. The resulting track change made daily-js close the AudioContext while the worklet module was still loading, aborting the load and killing the observer for the rest of the call. Measured on unpatched builds as a close landing 1ms before a 3ms load window. Upstream: daily-co/daily-js#317 Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes VAP-17046
Problem
vapi.start()calledstartLocalAudioLevelObserver()unconditionally, and before noise cancellation was applied. Krisp then swaps the microphone track, which makes daily-js runstopProcessing()→startProcessing()— closing theAudioContextwhile the observer's AudioWorklet module is still loading. The load aborts and the observer is dead for the rest of the call.Reported by a customer on
@vapi-ai/web2.6.1. Two console errors on every call, in Chrome and Firefox but not Safari:Introduced in d4d52d7 (#162). Benign for call audio, transcription and the microphone itself, but it permanently breaks the
local-volume-levelfeature — and it fired for every integration whether or not they consume that event.Fix
local-volume-levellistener is registered.updateInputSettings()settles, so Krisp has already swapped the track before the worklet loads.The observer start hangs off a separate chain from the error-reporting
.catch(), becauseEventEmitterrethrows out ofemit('error')when the consumer registered no'error'listener — whether the observer starts must not hinge on that.Verification
12 real web calls in Chrome with
AudioWorklet.addModule()andAudioContext.close()instrumented in-page: 3 rounds × 2 scenarios × 2 builds.Unpatched, every call closed the context 1ms before the observer's worklet load:
A 3ms window with a 1ms margin. One run showed a load taking 89ms rather than 3ms, which is the variance that makes this fire on customer machines and not on ours.
Patched, every call:
Zero context closes. The fix removes the event that opens the race rather than narrowing it.
Both scenarios were checked, because gating alone could have "fixed" the console by disabling the feature:
AudioContextcloseslocal-volume-leveleventsKrisp genuinely engaged throughout (
inputSettingsends at{"audio":{"processor":{"type":"noise-cancellation"}}}), so this was a realistic environment rather than one where noise cancellation silently no-opped.Unit coverage added for the gate, the ordering, and failure reporting, on both
start()andreconnect(). Full suite: 61 passing,tsc --noEmitclean.Upstream
Filed as daily-co/daily-js#317. Two defects there:
stopProcessing()closes theAudioContextwithout cancelling or awaiting an in-flightaddModule().this.blobURLis instance state shared across attempts, so a losing attempt's cleanup can revoke the blob URL of a successful retry.Reproduced on daily-js 0.87.0, 0.91.0 and 0.92.2. This fix avoids the race and does not depend on the upstream one landing.