-
Notifications
You must be signed in to change notification settings - Fork 307
[JS & Python] Add cancellation and structured error handling to live audio streaming #690
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.
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
0da8ab7
21194a3
1d340ab
83e8dc4
2cf7df8
df4260c
eee2cbe
42b40e5
6ab38c4
143a8b6
d5683c3
187810b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
| // | ||
| // Usage: node app.js | ||
|
|
||
| import { FoundryLocalManager } from 'foundry-local-sdk'; | ||
| import { FoundryLocalManager, CoreError } from 'foundry-local-sdk'; | ||
|
|
||
| console.log('╔══════════════════════════════════════════════════════════╗'); | ||
| console.log('║ Foundry Local — Live Audio Transcription (JS SDK) ║'); | ||
|
|
@@ -39,9 +39,16 @@ console.log('Loading model...'); | |
| await model.load(); | ||
| console.log('✓ Model loaded'); | ||
|
|
||
| // Graceful-shutdown coordinator. Set ONCE on the session via | ||
| // createLiveTranscriptionSession(signal) — every subsequent | ||
| // start() / append() / getTranscriptionStream() / stop() call picks it | ||
| // up automatically, so we don't have to thread the signal through every | ||
| // callsite. | ||
| const shutdown = new AbortController(); | ||
|
|
||
| // Create live transcription session (same pattern as C# sample). | ||
| const audioClient = model.createAudioClient(); | ||
| const session = audioClient.createLiveTranscriptionSession(); | ||
| const session = audioClient.createLiveTranscriptionSession(shutdown.signal); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of creating a new class to represent the parameters passed into the session, can we instead make an optional parameter for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, I will update the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 6ab38c4. Dropped const shutdown = new AbortController();
const session = audioClient.createLiveTranscriptionSession(shutdown.signal);(was JS 19/19 tests pass; sample updated. |
||
| session.settings.sampleRate = 16000; // Default is 16000; shown here for clarity | ||
| session.settings.channels = 1; | ||
|
|
@@ -67,9 +74,22 @@ const readPromise = (async () => { | |
| } | ||
| } | ||
| } catch (err) { | ||
| if (err.name !== 'AbortError') { | ||
| console.error('Stream error:', err.message); | ||
| // AbortError is expected on Ctrl+C; ignore quietly. | ||
| if (err.name === 'AbortError') return; | ||
|
|
||
| // CoreError surfaces native-core failure metadata (code + isTransient). | ||
| // Use it to retry quietly on transient blips instead of dying on the | ||
| // first hiccup. Without CoreError the only signal would be err.message. | ||
| if (err instanceof CoreError) { | ||
| if (err.isTransient) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it expected for FoundryLocalCore to throw transient errors? What do transient errors mean? If we detect transient errors, and there is no action that the application/user should take during such errors, can we catch them before they reach the application layer and continue as necessary? |
||
| console.warn(`\n⚠ Transient ASR error (${err.code}): ${err.message}. Continuing...`); | ||
| return; | ||
| } | ||
| console.error(`\n✗ Stream error [${err.code}]: ${err.message}`); | ||
| return; | ||
| } | ||
|
|
||
| console.error('\n✗ Stream error:', err.message); | ||
| } | ||
| })(); | ||
|
|
||
|
|
@@ -108,14 +128,18 @@ try { | |
| try { | ||
| while (appendQueue.length > 0) { | ||
| const pcm = appendQueue.shift(); | ||
| // Session-level signal (set in createLiveTranscriptionSession) | ||
| // applies automatically — no need to pass it here. | ||
| await session.append(pcm); | ||
| } | ||
| } catch (err) { | ||
| // Aborted via Ctrl+C — exit quietly. | ||
| if (err.name === 'AbortError') return; | ||
| console.error('append error:', err.message); | ||
| } finally { | ||
| pumping = false; | ||
| // Handle race where new data arrived after loop exit. | ||
| if (appendQueue.length > 0) { | ||
| if (appendQueue.length > 0 && !shutdown.signal.aborted) { | ||
| void pumpAudio(); | ||
| } | ||
| } | ||
|
|
@@ -182,9 +206,14 @@ try { | |
| process.exit(0); | ||
| } | ||
|
|
||
| // Handle graceful shutdown | ||
| // Handle graceful shutdown. | ||
| // | ||
| // The AbortController fires the shared `shutdown` signal so any in-flight | ||
| // session.append() / getTranscriptionStream() resolves promptly with an | ||
| // AbortError instead of waiting for stop() to finish draining the queue. | ||
| process.on('SIGINT', async () => { | ||
| console.log('\n\nStopping...'); | ||
| shutdown.abort(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is my understanding correct that shutdown and session.stop do the same thing but shutdown is similar to session.stop(hard=true) so we avoid draining the queue? Should we use the same API to represent that, or do we need separate handlers for shutdown vs session.stop()?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They're not the same thing — they're orthogonal:
|
||
| if (audioInput) { | ||
| audioInput.quit(); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CoreError seems to imply that the error is a generic error from the Core? Is that the case, or is the error specific to real time audio transcription?
If the latter, then I suggest we rename the error type? If the former, then we should address this in a bigger PR that throws a common error across all APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right — the name overpromises. Currently
CoreErroris only thrown by LiveAudioTranscriptionSession; chat, audio-filetranscription, and the Responses API all still throw plain Error. We can do this ?
Rename to scope-honest:
LiveAudioStreamError— keeps it where it lives, no false promises.