Skip to content

Commit 4ff7714

Browse files
authored
fix: audio recording race condition (#3430)
## 🎯 Goal This PR addresses an issue with voice recording, where if we let go of the long press gesture just after `onLongPress` has fired, but before `startRecording` has actually been invoked, we would get the player stuck in a bad state where we would neither be able to cancel nor lock the microphone in unless we long press again (which is not really intuitive). We implement a simple locking/semaphore mechanism to make sure this does not happen. ## 🛠 Implementation details <!-- Provide a description of the implementation --> ## 🎨 UI Changes <!-- Add relevant screenshots --> <details> <summary>iOS</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> <details> <summary>Android</summary> <table> <thead> <tr> <td>Before</td> <td>After</td> </tr> </thead> <tbody> <tr> <td> <!--<img src="" /> --> </td> <td> <!--<img src="" /> --> </td> </tr> </tbody> </table> </details> ## 🧪 Testing <!-- Explain how this change can be tested (or why it can't be tested) --> ## ☑️ Checklist - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [ ] PR targets the `develop` branch - [ ] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android
1 parent 2a51aa3 commit 4ff7714

File tree

4 files changed

+329
-24
lines changed

4 files changed

+329
-24
lines changed

package/src/components/MessageInput/components/AudioRecorder/AudioRecordingButton.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,14 @@ export const AudioRecordingButtonWithContext = (props: AudioRecordingButtonProps
151151
});
152152

153153
const onTouchGestureEnd = useStableCallback(() => {
154+
if (cancellableDuration) {
155+
onEarlyReleaseHandler();
156+
return;
157+
}
154158
if (status === 'recording') {
155-
if (cancellableDuration) {
156-
onEarlyReleaseHandler();
157-
} else {
158-
uploadVoiceRecording(asyncMessagesMultiSendEnabled);
159-
}
159+
uploadVoiceRecording(asyncMessagesMultiSendEnabled);
160+
} else {
161+
resetAudioRecording();
160162
}
161163
});
162164

package/src/components/MessageInput/hooks/useAudioRecorder.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,12 @@ export const useAudioRecorder = ({
2727
* side only. Meant to be used as a pure function (during unmounting for instance)
2828
* hence this approach.
2929
*/
30-
const stopVoiceRecording = useCallback(async () => {
31-
const { status } = audioRecorderManager.state.getLatestValue();
32-
if (status !== 'recording') return;
33-
await audioRecorderManager.stopRecording();
34-
}, [audioRecorderManager]);
30+
const stopVoiceRecording = useCallback(
31+
async (withDelete?: boolean) => {
32+
await audioRecorderManager.stopRecording(withDelete);
33+
},
34+
[audioRecorderManager],
35+
);
3536

3637
// This effect stop the player from playing and stops audio recording on
3738
// the audio SDK side on unmount.
@@ -62,9 +63,8 @@ export const useAudioRecorder = ({
6263
* Function to delete voice recording.
6364
*/
6465
const deleteVoiceRecording = useCallback(async () => {
65-
await stopVoiceRecording();
66-
audioRecorderManager.reset();
67-
}, [audioRecorderManager, stopVoiceRecording]);
66+
await stopVoiceRecording(true);
67+
}, [stopVoiceRecording]);
6868

6969
/**
7070
* Function to upload or send voice recording.
Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
1+
import { AudioReturnType, NativeHandlers } from '../../native';
2+
import { AudioRecorderManager } from '../audio-recorder-manager';
3+
4+
const createDeferred = <T>() => {
5+
let resolve!: (value: T | PromiseLike<T>) => void;
6+
let reject!: (reason?: unknown) => void;
7+
const promise = new Promise<T>((res, rej) => {
8+
resolve = res;
9+
reject = rej;
10+
});
11+
return { promise, reject, resolve };
12+
};
13+
14+
const getMockRecording = () =>
15+
({
16+
getStatusAsync: jest.fn(),
17+
getURI: jest.fn(),
18+
pauseAsync: jest.fn(),
19+
recording: 'recording-id',
20+
setProgressUpdateInterval: jest.fn(),
21+
stopAndUnloadAsync: jest.fn(),
22+
}) as const;
23+
24+
describe('AudioRecorderManager race conditions', () => {
25+
const originalAudioHandler = NativeHandlers.Audio;
26+
27+
beforeEach(() => {
28+
jest.clearAllMocks();
29+
});
30+
31+
afterEach(() => {
32+
NativeHandlers.Audio = originalAudioHandler;
33+
});
34+
35+
it('keeps initial recorder state', () => {
36+
const manager = new AudioRecorderManager();
37+
expect(manager.state.getLatestValue()).toEqual({
38+
duration: 0,
39+
isStarting: false,
40+
micLocked: false,
41+
recording: undefined,
42+
status: 'idle',
43+
waveformData: [],
44+
});
45+
});
46+
47+
it('starts successfully and transitions to recording only after native start resolves', async () => {
48+
const recording = getMockRecording();
49+
const startRecording = jest.fn().mockResolvedValue({
50+
accessGranted: true,
51+
recording,
52+
} satisfies AudioReturnType);
53+
const stopRecording = jest.fn().mockResolvedValue(undefined);
54+
55+
NativeHandlers.Audio = {
56+
audioRecordingConfiguration: {},
57+
startRecording,
58+
stopRecording,
59+
};
60+
61+
const manager = new AudioRecorderManager();
62+
const startPromise = manager.startRecording();
63+
64+
expect(manager.state.getLatestValue().isStarting).toBe(true);
65+
await startPromise;
66+
67+
const latest = manager.state.getLatestValue();
68+
expect(latest.isStarting).toBe(false);
69+
expect(latest.status).toBe('recording');
70+
expect(latest.recording).toBe(recording);
71+
expect(recording.setProgressUpdateInterval).toHaveBeenCalledWith(expect.any(Number));
72+
});
73+
74+
it('stops during in-flight start and does not enter recording when start resolves', async () => {
75+
const recording = getMockRecording();
76+
const deferred = createDeferred<AudioReturnType>();
77+
const startRecording = jest.fn().mockImplementation(() => deferred.promise);
78+
const stopRecording = jest.fn().mockResolvedValue(undefined);
79+
80+
NativeHandlers.Audio = {
81+
audioRecordingConfiguration: {},
82+
startRecording,
83+
stopRecording,
84+
};
85+
86+
const manager = new AudioRecorderManager();
87+
const startPromise = manager.startRecording();
88+
89+
expect(manager.state.getLatestValue().isStarting).toBe(true);
90+
91+
await manager.stopRecording();
92+
expect(manager.state.getLatestValue()).toEqual({
93+
duration: 0,
94+
isStarting: false,
95+
micLocked: false,
96+
recording: undefined,
97+
status: 'idle',
98+
waveformData: [],
99+
});
100+
101+
deferred.resolve({ accessGranted: true, recording });
102+
await startPromise;
103+
104+
expect(stopRecording).toHaveBeenCalledTimes(1);
105+
expect(manager.state.getLatestValue().status).toBe('idle');
106+
expect(manager.state.getLatestValue().recording).toBeUndefined();
107+
});
108+
109+
it('does not call native stop after pending stop if start resolves without access', async () => {
110+
const deferred = createDeferred<AudioReturnType>();
111+
const startRecording = jest.fn().mockImplementation(() => deferred.promise);
112+
const stopRecording = jest.fn().mockResolvedValue(undefined);
113+
114+
NativeHandlers.Audio = {
115+
audioRecordingConfiguration: {},
116+
startRecording,
117+
stopRecording,
118+
};
119+
120+
const manager = new AudioRecorderManager();
121+
const startPromise = manager.startRecording();
122+
await manager.stopRecording();
123+
124+
deferred.resolve({ accessGranted: false, recording: undefined });
125+
await startPromise;
126+
127+
expect(stopRecording).not.toHaveBeenCalled();
128+
expect(manager.state.getLatestValue().status).toBe('idle');
129+
});
130+
131+
it('resets state when native start throws', async () => {
132+
const startRecording = jest.fn().mockRejectedValue(new Error('start failed'));
133+
const stopRecording = jest.fn().mockResolvedValue(undefined);
134+
135+
NativeHandlers.Audio = {
136+
audioRecordingConfiguration: {},
137+
startRecording,
138+
stopRecording,
139+
};
140+
141+
const manager = new AudioRecorderManager();
142+
const result = await manager.startRecording();
143+
144+
expect(result).toBe(false);
145+
expect(manager.state.getLatestValue()).toEqual({
146+
duration: 0,
147+
isStarting: false,
148+
micLocked: false,
149+
recording: undefined,
150+
status: 'idle',
151+
waveformData: [],
152+
});
153+
});
154+
155+
it('does not let stale start completion overwrite newer recording session', async () => {
156+
const recording1 = getMockRecording();
157+
const recording2 = getMockRecording();
158+
const deferred1 = createDeferred<AudioReturnType>();
159+
const deferred2 = createDeferred<AudioReturnType>();
160+
161+
const startRecording = jest
162+
.fn()
163+
.mockImplementationOnce(() => deferred1.promise)
164+
.mockImplementationOnce(() => deferred2.promise);
165+
const stopRecording = jest.fn().mockResolvedValue(undefined);
166+
167+
NativeHandlers.Audio = {
168+
audioRecordingConfiguration: {},
169+
startRecording,
170+
stopRecording,
171+
};
172+
173+
const manager = new AudioRecorderManager();
174+
const firstStart = manager.startRecording();
175+
await manager.stopRecording();
176+
const secondStart = manager.startRecording();
177+
178+
deferred1.resolve({ accessGranted: true, recording: recording1 });
179+
await firstStart;
180+
181+
// First (stale) completion should not put stale recording into state.
182+
expect(manager.state.getLatestValue().recording).toBeUndefined();
183+
expect(manager.state.getLatestValue().status).toBe('idle');
184+
185+
deferred2.resolve({ accessGranted: true, recording: recording2 });
186+
await secondStart;
187+
188+
expect(manager.state.getLatestValue().recording).toBe(recording2);
189+
expect(manager.state.getLatestValue().status).toBe('recording');
190+
});
191+
192+
it('stopRecording with delete clears state instead of setting stopped', async () => {
193+
const recording = getMockRecording();
194+
const startRecording = jest.fn().mockResolvedValue({
195+
accessGranted: true,
196+
recording,
197+
} satisfies AudioReturnType);
198+
const stopRecording = jest.fn().mockResolvedValue(undefined);
199+
200+
NativeHandlers.Audio = {
201+
audioRecordingConfiguration: {},
202+
startRecording,
203+
stopRecording,
204+
};
205+
206+
const manager = new AudioRecorderManager();
207+
await manager.startRecording();
208+
await manager.stopRecording(true);
209+
210+
expect(manager.state.getLatestValue()).toEqual({
211+
duration: 0,
212+
isStarting: false,
213+
micLocked: false,
214+
recording: undefined,
215+
status: 'idle',
216+
waveformData: [],
217+
});
218+
});
219+
});

0 commit comments

Comments
 (0)