fix: stop voice entry posting an unsaved detail id, and name failures right

Review findings on this branch.

The lab-case save could be posted against a detail the server has never
seen. persistDraft returns a *preview* treatment instead of saving when any
detail lacks a treatment type — the blank one the workspace opens with is
enough — and a preview's detail id falls back to the client id. Recording
straight after opening a visit and confirming a result with a lab or due
date would send that id and fail the whole save. It now checks what came
back rather than the precondition, so it holds for every early return
persistDraft has.

stop() optional-chained into a no-op when the recorder was already gone,
leaving the bar recording forever with a live timer and only Cancel as a
way out.

Three "this browser cannot record" paths reported VOICE_MIC_DENIED — no
MediaRecorder at all, no container the API accepts, and a recorder that
throws after permission was already granted. Telling clinicians their
microphone was denied sends them hunting for a permission nothing asked
for; they now report VOICE_UNSUPPORTED_FORMAT.

The voice route's large-body match stripped every trailing slash while
Express ignores exactly one, so '/api/voice/extract//' bought a 10 MB
buffer for a request that then 404s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-21 17:44:51 +08:00
parent 5d8393c1eb
commit 9ec23d0a58
4 changed files with 32 additions and 5 deletions

View File

@@ -93,6 +93,9 @@ describe('createJsonBodyParser', () => {
'/api/voice/extract/extra', '/api/voice/extract/extra',
'/api/voice', '/api/voice',
'/voice/extract', '/voice/extract',
// Express ignores one trailing slash, not two — this one never routes, so it must
// not get the large parser either.
'/api/voice/extract//',
]) { ]) {
const res = await request(buildApp()).post(path).send(bodyOfKb(300)); const res = await request(buildApp()).post(path).send(bodyOfKb(300));
expect(res.status).toBe(413); expect(res.status).toBe(413);

View File

@@ -31,7 +31,10 @@ export const VOICE_BODY_LIMIT = '10mb';
* recording — a failure that looks like a broken microphone, not a routing detail. * recording — a failure that looks like a broken microphone, not a routing detail.
*/ */
function isVoiceExtractPath(path: string): boolean { function isVoiceExtractPath(path: string): boolean {
return path.toLowerCase().replace(/\/+$/, '') === VOICE_EXTRACT_PATH; // Exactly one trailing slash, because that is exactly what Express ignores. Stripping
// every trailing slash would hand the 10 MB parser to `/api/voice/extract//`, which
// buffers the body and then 404s — memory spent on a request that never routes.
return path.toLowerCase().replace(/\/$/, '') === VOICE_EXTRACT_PATH;
} }
export function createJsonBodyParser(): RequestHandler { export function createJsonBodyParser(): RequestHandler {

View File

@@ -2006,6 +2006,14 @@ export function TreatmentWorkspace({
void (async () => { void (async () => {
try { try {
const saved = await persistDraft({ force: true }); const saved = await persistDraft({ force: true });
// persistDraft returns a *preview* treatment rather than saving when the
// details are not persistable — one blank detail, the kind the workspace opens
// with, is enough. A preview's detail id falls back to the client id, so
// posting lab cases against it would send the server an id it has never seen
// and fail the whole save. Check what came back, not the precondition, so this
// holds for every early return persistDraft has.
const savedDetail = saved.details.find((d) => d.clientId === detail.clientId);
if (!savedDetail?.id || savedDetail.id === detail.clientId) return;
await persistLabCases(saved, updatedLabCases); await persistLabCases(saved, updatedLabCases);
} catch (error: unknown) { } catch (error: unknown) {
showError(getUserFacingError(error, tErrors, t('errorSaveLabShipments'))); showError(getUserFacingError(error, tErrors, t('errorSaveLabShipments')));

View File

@@ -139,8 +139,16 @@ export function useVoiceCapture({
); );
const stop = useCallback(() => { const stop = useCallback(() => {
// No recorder means nothing will fire `onstop`, so nothing else will move the phase.
// Optional-chaining into a no-op here left the bar recording forever with a running
// timer, and only Cancel could get out of it.
if (!recorderRef.current) {
teardown();
setPhase('idle');
return;
}
try { try {
recorderRef.current?.stop(); recorderRef.current.stop();
} catch { } catch {
teardown(); teardown();
setPhase('idle'); setPhase('idle');
@@ -150,7 +158,9 @@ export function useVoiceCapture({
const onStart = useCallback(() => { const onStart = useCallback(() => {
if (phase !== 'idle' || startingRef.current) return; if (phase !== 'idle' || startingRef.current) return;
if (!isMediaRecorderSupported()) { if (!isMediaRecorderSupported()) {
onError(clientError('VOICE_MIC_DENIED')); // Not a permission problem: this browser cannot record at all. Saying "microphone
// denied" sends the clinician to hunt for a permission nothing ever asked for.
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
return; return;
} }
@@ -177,8 +187,9 @@ export function useVoiceCapture({
const mimeType = pickRecordingMimeType(); const mimeType = pickRecordingMimeType();
if (mimeType === null) { if (mimeType === null) {
// The browser records, but in no container the transcription API accepts.
stream.getTracks().forEach((track) => track.stop()); stream.getTracks().forEach((track) => track.stop());
onError(clientError('VOICE_MIC_DENIED')); onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
return; return;
} }
@@ -226,7 +237,9 @@ export function useVoiceCapture({
// recording indicator stays lit until the workspace unmounts. // recording indicator stays lit until the workspace unmounts.
teardown(); teardown();
setPhase('idle'); setPhase('idle');
onError(clientError('VOICE_MIC_DENIED')); // The permission was already granted by this point — what failed is the recorder
// itself, so this is "this browser cannot record", not "you denied the mic".
onError(clientError('VOICE_UNSUPPORTED_FORMAT'));
} finally { } finally {
startingRef.current = false; startingRef.current = false;
} }