record_web: _onStopCompleter lifecycle bugs — stop() future can hang forever; "Bad state: Future already completed" crash in production

[mjew]

Package versions

record_web 2.1.1 (both bugs also present in 1.3.0) / record 7.1.1

Platform

Web (any browser using MediaRecorderDelegate)

Summary

MediaRecorderDelegate manages _onStopCompleter in a way that produces two
related failures:

Bug A — await stop() can hang forever (deterministic repro below).
stop() queues the recorder's onstop event, but if start() is called
before that event dispatches, start()'s _reset() synchronously nulls the
old recorder's onstop handler. The event is then dropped, _onStop() never
runs for it, and the completer returned by stop() never completes — the
caller awaits forever. Additionally, stop() unconditionally overwrites any
pending _onStopCompleter, orphaning the earlier caller's future.

Bug B — unhandled Bad state: Future already completed.
_onStop() completes _onStopCompleter without an isCompleted guard and
never nulls the field, and it has await gaps (fixWebmDuration,
_reset()) inside the browser event handler. When a suspended _onStop()
from one stop cycle resumes after stop() of the next cycle has replaced
the completer, both runs complete the same completer and the second throws.
Because the throw happens inside the JS event callback rather than in the
caller's await, no application-side try/catch around
AudioRecorder.stop() can catch it — it surfaces as an unhandled global
error. We have ~9,600 Sentry events of this crash from production web users;
the minified stack ends in Completer.complete →
throw StateError("Future already completed") inside the onstop async chain.
This one is timing-dependent (we could not force it in a headless-Chrome
test), but it shares the same root cause: an unmanaged completer lifecycle.

Code (lib/src/recorder/delegate/media_recorder_delegate.dart)

Future<String?> stop() async {
  if (_isRecording()) {
    _onStopCompleter = Completer();   // overwrites any pending completer
    _mediaRecorder?.stop();
    return _onStopCompleter!.future;
  }
  return null;
}

void _onStop() async {
  ...
  } finally {
    await _reset();                          // async gap in the event handler
    onStateChanged(RecordState.stop);
    _onStopCompleter?.complete(audioUrl);    // unguarded, never nulled
  }
}

Deterministic repro for Bug A

Passes against stock record_web 2.1.1 in headless Chrome
(--use-fake-ui-for-media-stream --use-fake-device-for-media-stream):

final delegate = MediaRecorderDelegate(onStateChanged: (_) {});

await delegate.start(config, path: '');
await Future<void>.delayed(const Duration(milliseconds: 300));

final stopFuture = delegate.stop();     // onstop queued...
await delegate.start(config, path: ''); // ...but _reset() nulls the handler

var orphaned = false;
await stopFuture.timeout(
  const Duration(seconds: 5),
  onTimeout: () { orphaned = true; return null; },
);
// orphaned == true on stock record_web: the stop future never completes.

Same sequence through the public API: unawaited(recorder.stop()); followed
by recorder.start(...) — a rapid re-record flow any app can hit.

Suggested fix (two hunks, fixes both bugs)

// stop(): share the in-flight completer instead of replacing it
final completer = _onStopCompleter ??= Completer();
_mediaRecorder?.stop();
return completer.future;

// _onStop(): take-and-null with a guard
final completer = _onStopCompleter;
_onStopCompleter = null;
if (completer != null && !completer.isCompleted) {
  completer.complete(audioUrl);
}

With this patch (verified in the same headless-Chrome harness):

• the Bug A repro's stop future completes normally (a later stop() shares
  and completes the pending completer), and
• 300 fuzzed stop/start/stop cycles produce zero double-completes.

We are running this patch vendored in production. Happy to open a PR.

[llfbandit]

Thanks for the report.
PR is welcome of course. I'll be back in couple of weeks.