From b0271fa953ee62dff2e938ac86991f181fa71480 Mon Sep 17 00:00:00 2001 From: freetlab Date: Sat, 1 Aug 2026 18:50:17 +0200 Subject: [PATCH] feat(audio): log AudioService.asyncError instead of swallowing it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AudioService.asyncError` had ZERO subscribers app-wide. The plugin funnels every asynchronous failure of its own observers into that stream and nowhere else — `_observePlaybackState`, `_observeMediaItem` and `_observeQueue` each wrap their whole body in `catch (e) { _asyncError.add(e); }`, and the artwork path uses `.catchError(_asyncError.add)` — and a `PublishSubject` with no listeners simply drops what it is given. The platform-side exception behind "the media playback notification disappeared" was therefore being discarded without a single log line, which is why that report arrives with no evidence attached. `observarErroresAudio` is a pure, injectable seam in `arranque_audio.dart` (stream in, logger callback out), matching the seam convention this codebase already uses for `esperarArranqueAudio`, `decidirAvanceCola` and `debeReaplicarEcualizador`: the unit tests exercise the wiring with a plain `StreamController`, never the real plugin. The default logger emits one `[PluriWave]`-prefixed `developer.log` line at `level: 900`, the same level and prefix `servicio_audio.dart` already uses, so one logcat filter catches both. Wired from `lib/main.dart`, not from `arranque_audio.dart`: main.dart is the module that genuinely owns handler lifecycle — it is the only caller of `AudioService.init`, `registrarHandler` and `ServicioAudioSession`, and both the on-time and the degraded/timeout startup branches converge on its `conectarHandler` closure. `arranque_audio.dart` owns only the timeout race and the degraded loading shell; it never creates or registers a handler (`alListo` is injected into it from main.dart), so it has no lifecycle to hang a subscription on. Subscribing happens before `AudioService.init` — the getter only touches a static subject — so nothing reported during the MediaBrowser handshake is missed, and one subscription covers both paths. The subscription is cancellable and its `cancel` is registered into the handler via `registrarLimpiezaArranque`, mirroring the existing `registrarHandler` / `registrarFuenteNavegacion` / `registrarFuenteMusicaLocal` registration convention. `onTaskRemoved` — the only handler teardown in this app — runs it, so the subscription cannot outlive what it instruments. The dependency points bootstrap -> service, so `servicio_audio.dart` never has to import the bootstrap module or the plugin's static stream. Zero behaviour change: nothing but log output is added. --- lib/main.dart | 20 ++++++ lib/servicios/arranque_audio.dart | 48 +++++++++++++ lib/servicios/servicio_audio.dart | 24 +++++++ test/servicios/arranque_audio_test.dart | 89 +++++++++++++++++++++++++ 4 files changed, 181 insertions(+) diff --git a/lib/main.dart b/lib/main.dart index bda6676..a073073 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -55,6 +55,21 @@ Future main() async { // permanently hidden (`hayCarpetaConfigurada()` has no fuente to ask). registrarFuenteMusicaLocal(FuenteMusicaLocalAutoImpl(prefs: prefs)); + // Silent-error channel (fix/notificacion-media): `AudioService.asyncError` + // had ZERO subscribers app-wide, and a `PublishSubject` with no listeners + // drops what it is given — so every exception `audio_service` catches + // internally was discarded without a trace, which is exactly why the + // "media notification disappeared" report came with no evidence attached. + // Subscribed BEFORE `AudioService.init` below (the getter only touches a + // static subject, so it needs no initialisation) so nothing reported + // during the MediaBrowser handshake is missed, and placed here rather than + // in `conectarHandler` so ONE subscription covers both the on-time and the + // degraded/timeout startup paths. + final subErroresAudio = observarErroresAudio( + AudioService.asyncError, + registrar: registrarErrorAudioService, + ); + // Design "Timeout without re-init": AudioService.init is started exactly // ONCE here and `handlerFuturo` is the only future ever awaited for it — // the plugin caches state internally, so a double-configure call is @@ -69,6 +84,11 @@ Future main() async { // degraded/late-completion paths below. void conectarHandler(PluriWaveAudioHandler handler) { registrarHandler(handler); + // The handler is the only thing this app ever tears down + // (`onTaskRemoved`), so the asyncError subscription's `cancel` travels + // with it and can never leak — same "register from main.dart" convention + // as `registrarHandler` itself. + registrarLimpiezaArranque(subErroresAudio.cancel); final sesionAudio = ServicioAudioSession(objetivo: handler); unawaited(sesionAudio.configurar()); } diff --git a/lib/servicios/arranque_audio.dart b/lib/servicios/arranque_audio.dart index 6ed7fc3..23ac4be 100644 --- a/lib/servicios/arranque_audio.dart +++ b/lib/servicios/arranque_audio.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:developer' as developer; import 'package:flutter/material.dart'; @@ -58,6 +59,53 @@ Future> esperarArranqueAudio( } } +/// Subscribes to [errores] — in production `AudioService.asyncError` — and +/// hands every event to [registrar]. Returns the [StreamSubscription] so the +/// caller can cancel it when the handler is torn down. +/// +/// Why this exists: `audio_service` funnels EVERY asynchronous failure of its +/// own observers into that stream and nothing else +/// (`_observePlaybackState`/`_observeMediaItem`/`_observeQueue` each wrap +/// their whole body in `catch (e) { _asyncError.add(e); }`, and the artwork +/// path uses `.catchError(_asyncError.add)`), yet this app had ZERO +/// subscribers on it. A `PublishSubject` with no listeners simply drops +/// events, so the platform-side exception behind "the media notification +/// disappeared" — a rejected `setState`, a failed `setMediaItem`, an +/// Android 12+ `ForegroundServiceStartNotAllowedException` surfacing through +/// the plugin — was being discarded without a single log line. This makes +/// that channel audible. +/// +/// [errores] and [registrar] are both injected — this function never touches +/// the real `audio_service` plugin, so it is testable with a plain +/// [StreamController] (same seam convention as [esperarArranqueAudio] above, +/// and as `decidirAvanceCola`/`debeReaplicarEcualizador` elsewhere). +StreamSubscription observarErroresAudio( + Stream errores, { + required void Function(Object error) registrar, +}) { + return errores.listen( + registrar, + // The plugin only ever feeds this subject through `add`, never + // `addError`, so this branch is purely defensive: a stream-level error + // would otherwise escape as an unhandled zone error, which is strictly + // worse than one more log line. + onError: (Object error, StackTrace _) => registrar(error), + cancelOnError: false, + ); +} + +/// Default [observarErroresAudio] logger: one `[PluriWave]`-prefixed +/// `developer.log` line per swallowed plugin exception, at the same +/// `level: 900` (SEVERE) that `servicio_audio.dart`'s existing error lines +/// use, so a single logcat/DevTools filter catches both. +void registrarErrorAudioService(Object error) { + developer.log( + '[PluriWave] AudioService.asyncError: $error', + name: 'ArranqueAudio', + level: 900, + ); +} + /// Minimal branded bootstrap widget for the degraded path (Design "still /// call runApp, but with a minimal bootstrap widget that keeps waiting on /// the SAME original future"). Shows [_CargandoArranqueAudio] while diff --git a/lib/servicios/servicio_audio.dart b/lib/servicios/servicio_audio.dart index e9ef72b..933c792 100644 --- a/lib/servicios/servicio_audio.dart +++ b/lib/servicios/servicio_audio.dart @@ -60,6 +60,23 @@ void registrarFuenteMusicaLocal(FuenteMusicaLocalAuto fuente) { _fuenteMusicaLocalGlobal = fuente; } +/// Teardown hook for whatever `main.dart` wired around the handler and must +/// be undone when the handler itself dies — today only the +/// `AudioService.asyncError` subscription (`observarErroresAudio`). Registered +/// from `main.dart`, mirroring [registrarHandler] and the two browse-source +/// registrations above; run exactly once from +/// [PluriWaveAudioHandler.onTaskRemoved]. +/// +/// The direction of the dependency matters: the bootstrap layer injects its +/// cleanup INTO the service layer, so `servicio_audio.dart` never has to +/// import `arranque_audio.dart` (nor the plugin's static error stream) just to +/// be able to close it. +Future Function()? _limpiezaArranqueGlobal; + +void registrarLimpiezaArranque(Future Function() limpieza) { + _limpiezaArranqueGlobal = limpieza; +} + /// Builds the phone-initiated "play a station" `MediaItem` (item 3, Android /// Auto fallback artwork): reuses [artUriPara] (`navegacion_auto.dart`) so a /// station with no usable favicon gets the SAME on-brand rotating fallback @@ -1201,6 +1218,13 @@ class PluriWaveAudioHandler extends BaseAudioHandler await _androidAudioSessionIdSub?.cancel(); await _player.dispose(); await _androidAudioSessionIdController.close(); + // Handler teardown: release the bootstrap-owned `AudioService.asyncError` + // subscription too, so it cannot outlive the handler it was instrumenting. + // Never throws out of teardown — a failing cleanup hook must not prevent + // the rest of `onTaskRemoved` from having completed above. + try { + await _limpiezaArranqueGlobal?.call(); + } catch (_) {} } Emisora _emisoraDesdeMediaItem(MediaItem mediaItem) { diff --git a/test/servicios/arranque_audio_test.dart b/test/servicios/arranque_audio_test.dart index 288e8a8..c67aa83 100644 --- a/test/servicios/arranque_audio_test.dart +++ b/test/servicios/arranque_audio_test.dart @@ -63,4 +63,93 @@ void main() { expect(handler, 'handler-tardio'); }); }); + + /// fix/notificacion-media — commit 1: `AudioService.asyncError` had zero + /// subscribers, so every exception `audio_service` swallows internally was + /// dropped on the floor. These cover the injectable seam only (Design + /// "Testability" — the stream and the logger are both injected), never the + /// real plugin. + group('observarErroresAudio', () { + test('reenvia al logger cada error emitido, en orden', () async { + final controlador = StreamController.broadcast(); + final registrados = []; + + final sub = observarErroresAudio( + controlador.stream, + registrar: registrados.add, + ); + + controlador.add('fallo-1'); + controlador.add(StateError('fallo-2')); + await controlador.close(); + + expect(registrados, hasLength(2)); + expect(registrados.first, 'fallo-1'); + expect(registrados.last, isA()); + + await sub.cancel(); + }); + + test('cancelar la suscripcion corta el logging — no puede filtrarse ' + 'tras el teardown del handler', () async { + final controlador = StreamController.broadcast(); + final registrados = []; + + final sub = observarErroresAudio( + controlador.stream, + registrar: registrados.add, + ); + + controlador.add('antes-del-cancel'); + // Deja que el evento se entregue antes de cancelar (los broadcast + // controllers entregan en un microtask, no de forma sincrona). + await Future.delayed(Duration.zero); + await sub.cancel(); + + controlador.add('despues-del-cancel'); + await controlador.close(); + + expect( + registrados, + ['antes-del-cancel'], + reason: + 'tras cancelar, la suscripcion no debe seguir viva ni registrar ' + 'nada mas', + ); + }); + + test( + 'un evento de error del propio stream tambien llega al logger', + () async { + final controlador = StreamController.broadcast(); + final registrados = []; + + final sub = observarErroresAudio( + controlador.stream, + registrar: registrados.add, + ); + + // Rama defensiva: el plugin solo usa `add`, nunca `addError`, pero un + // error de stream sin manejar seria una excepcion no capturada. + controlador.addError(const FormatException('stream roto')); + await controlador.close(); + + expect(registrados, hasLength(1)); + expect(registrados.single, isA()); + + await sub.cancel(); + }, + ); + + test('el logger por defecto acepta cualquier objeto sin lanzar', () { + expect( + () => registrarErrorAudioService(StateError('cualquier cosa')), + returnsNormally, + ); + expect( + () => registrarErrorAudioService('un string suelto'), + returnsNormally, + ); + }); + }); }