diff --git a/lib/flutter_midi_command.dart b/lib/flutter_midi_command.dart index f97d134e..846e248c 100644 --- a/lib/flutter_midi_command.dart +++ b/lib/flutter_midi_command.dart @@ -368,6 +368,13 @@ class MidiCommand { timeout: awaitConnectionTimeout, ); final connectionEstablished = _awaitConnectedOrFailed(device); + // A disconnection can complete this future with an error before the real + // handler below is attached (e.g. a disconnect racing an in-flight connect). + // Guard it eagerly so the race never surfaces as an unhandled async error + // to PlatformDispatcher.onError (issue #160). Dart futures allow multiple + // independent listeners, so the `await` below still observes and handles + // the same error inside its try/catch. + unawaited(connectionEstablished.catchError((_) {})); try { final route = _resolveDeviceRoute(device); @@ -399,7 +406,6 @@ class MidiCommand { ); } } catch (_) { - unawaited(connectionEstablished.catchError((_) {})); _cancelBlePlatformHandoff(device.id); _activeDeviceRouteById.remove(device.id); if (device.connectionState != MidiConnectionState.disconnected) { diff --git a/packages/flutter_midi_command_android/android/src/main/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDevice.kt b/packages/flutter_midi_command_android/android/src/main/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDevice.kt index d8e30403..94b0587f 100644 --- a/packages/flutter_midi_command_android/android/src/main/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDevice.kt +++ b/packages/flutter_midi_command_android/android/src/main/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDevice.kt @@ -7,6 +7,18 @@ import com.invisiblewrench.fluttermidicommand.pigeon.MidiHostDevice import com.invisiblewrench.fluttermidicommand.pigeon.MidiPacket import com.invisiblewrench.fluttermidicommand.pigeon.MidiSetupChange +/** + * Runs each teardown [actions] in order, swallowing any error, so a failure in + * one step (for example an `IOException: EPIPE` from a port whose device was + * unplugged) cannot abort the remaining steps or the disconnection + * notifications that follow. Never throws. + */ +internal fun runTeardownQuietly(vararg actions: () -> Unit) { + for (action in actions) { + runCatching { action() } + } +} + class ConnectedDevice( device: MidiDevice, private val logicalDeviceId: String, @@ -71,12 +83,20 @@ class ConnectedDevice( } override fun close() { - this.inputPort?.flush() - this.inputPort?.close() - this.outputPort?.close() - this.outputPort?.disconnect(this.receiver) + // When a device is physically removed (e.g. USB unplugged) its file + // descriptor is already gone, so any teardown call can throw + // (IOException: EPIPE). runTeardownQuietly never propagates, so one + // failing step cannot abort the others or escape the framework's + // onDeviceRemoved callback, and the disconnection notifications below + // always fire. + runTeardownQuietly( + { this.inputPort?.flush() }, + { this.inputPort?.close() }, + { this.outputPort?.close() }, + { this.outputPort?.disconnect(this.receiver) }, + { this.midiDevice.close() }, + ) this.receiver = null - this.midiDevice.close() onSetupChanged(MidiSetupChange.DEVICE_DISCONNECTED) onConnectionChanged(id, false) diff --git a/packages/flutter_midi_command_android/android/src/test/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDeviceCloseTest.kt b/packages/flutter_midi_command_android/android/src/test/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDeviceCloseTest.kt new file mode 100644 index 00000000..fb0c64ec --- /dev/null +++ b/packages/flutter_midi_command_android/android/src/test/kotlin/com/invisiblewrench/fluttermidicommand/ConnectedDeviceCloseTest.kt @@ -0,0 +1,52 @@ +package com.invisiblewrench.fluttermidicommand + +import java.io.IOException +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * Regression tests for issue #158: unplugging a connected USB MIDI device + * crashed the app because `ConnectedDevice.close()` called `inputPort.flush()` + * on a dead file descriptor and the resulting `IOException: EPIPE` aborted the + * rest of teardown and the disconnection notifications. + * + * `close()` routes every fragile Android teardown call through + * [runTeardownQuietly] and then unconditionally fires the notifications, so + * these tests lock in the two guarantees that keep the crash fixed: a throwing + * step never aborts the later steps, and the helper never propagates. + */ +class ConnectedDeviceCloseTest { + + @Test + fun runsEveryStepEvenWhenSomeThrow() { + val ran = mutableListOf() + + runTeardownQuietly( + { + ran.add("flush") + throw IOException("write failed: EPIPE (Broken pipe)") + }, + { ran.add("closeInput") }, + { + ran.add("closeOutput") + throw IllegalStateException("already closed") + }, + { ran.add("closeDevice") }, + ) + + assertEquals( + listOf("flush", "closeInput", "closeOutput", "closeDevice"), + ran, + ) + } + + @Test + fun neverPropagates() { + // If this threw, the test would fail; passing proves the EPIPE is + // swallowed so the notifications after the teardown block in close() + // always run. + runTeardownQuietly( + { throw IOException("write failed: EPIPE (Broken pipe)") }, + ) + } +} diff --git a/packages/flutter_midi_command_ble/lib/flutter_midi_command_ble.dart b/packages/flutter_midi_command_ble/lib/flutter_midi_command_ble.dart index f186c1f8..69e2c2d4 100644 --- a/packages/flutter_midi_command_ble/lib/flutter_midi_command_ble.dart +++ b/packages/flutter_midi_command_ble/lib/flutter_midi_command_ble.dart @@ -12,6 +12,14 @@ const midiCharacteristicId = "7772E5DB-3868-4112-A1A9-F2669D106BF3"; enum _DeviceState { none, interrogating, available, irrelevant } +/// True when [error] is the universal_ble error surfaced when a peripheral has +/// discarded its side of a previous bond (iOS `CBErrorPeerRemovedPairingInformation`, +/// "Peer removed pairing information"). It arrives as an untyped +/// `UniversalBleException` with `unknownError`, so match on the message. +bool _isPairingInfoRemoved(Object error) => + error is UniversalBleException && + error.message.toLowerCase().contains('pairing information'); + enum _BleHandlerState { header, timestamp, @@ -396,11 +404,23 @@ class _BleMidiDevice extends MidiDevice { } await _prepareMidiReadiness(timeout: timeout); connected = true; - } catch (_) { + } catch (error) { connected = false; try { await disconnect(); } catch (_) {} + if (_isPairingInfoRemoved(error)) { + // Best-effort clear of the stale bond so a later reconnect can re-pair + // cleanly. Unsupported on iOS (CoreBluetooth has no unpair API), so + // ignore failures; the surfaced exception tells the user what to do. + try { + await UniversalBle.unpair(deviceId); + } catch (_) {} + throw MidiPairingInfoRemovedException( + deviceId: deviceId, + cause: error, + ); + } rethrow; } finally { _readinessInProgress = false; diff --git a/packages/flutter_midi_command_ble/test/flutter_midi_command_ble_test.dart b/packages/flutter_midi_command_ble/test/flutter_midi_command_ble_test.dart index 23965878..953e4370 100644 --- a/packages/flutter_midi_command_ble/test/flutter_midi_command_ble_test.dart +++ b/packages/flutter_midi_command_ble/test/flutter_midi_command_ble_test.dart @@ -8,6 +8,7 @@ import 'package:universal_ble/universal_ble.dart'; class _FakeUniversalBlePlatform extends UniversalBlePlatform { AvailabilityState availabilityState = AvailabilityState.poweredOn; final Set failingConnectIds = {}; + final Set pairingRemovedConnectIds = {}; final Set failingReadIds = {}; final Set failingSubscribeIds = {}; final Set rejectedPairIds = {}; @@ -20,6 +21,7 @@ class _FakeUniversalBlePlatform extends UniversalBlePlatform { final List connectCalls = []; final List disconnectCalls = []; final List pairCalls = []; + final List unpairCalls = []; final List readCalls = []; final List subscribeCalls = []; int startScanCalls = 0; @@ -65,6 +67,14 @@ class _FakeUniversalBlePlatform extends UniversalBlePlatform { }) async { connectCalls.add(deviceId); await Future.delayed(const Duration(milliseconds: 1)); + if (pairingRemovedConnectIds.contains(deviceId)) { + updateConnection(deviceId, false, 'Peer removed pairing information'); + _connectionByDevice[deviceId] = BleConnectionState.disconnected; + throw UniversalBleException( + code: UniversalBleErrorCode.unknownError, + message: 'Peer removed pairing information', + ); + } if (failingConnectIds.contains(deviceId)) { updateConnection(deviceId, false, 'connection-failed'); _connectionByDevice[deviceId] = BleConnectionState.disconnected; @@ -159,6 +169,7 @@ class _FakeUniversalBlePlatform extends UniversalBlePlatform { @override Future unpair(String deviceId) async { + unpairCalls.add(deviceId); _pairedByDevice[deviceId] = false; updatePairingState(deviceId, false); } @@ -319,6 +330,30 @@ void main() { expect(await transport.devices, isEmpty); }); + test( + 'connectToDevice maps removed pairing information to a typed exception', + () async { + fakePlatform.servicesByDevice['ble-stale-bond'] = midiServices(); + fakePlatform.pairingRemovedConnectIds.add('ble-stale-bond'); + fakePlatform.emitScanDevice( + BleDevice( + deviceId: 'ble-stale-bond', + name: 'Stale Bond Device', + services: [], + ), + ); + final device = (await transport.devices).single; + + await expectLater( + transport.connectToDevice(device), + throwsA(isA()), + ); + expect(device.connected, isFalse); + // The stale bond is cleared best-effort so a later reconnect re-pairs. + expect(fakePlatform.unpairCalls, contains('ble-stale-bond')); + }, + ); + test('disconnectDevice forwards to BLE backend', () async { fakePlatform.servicesByDevice['ble-3'] = midiServices(); fakePlatform.emitScanDevice( diff --git a/packages/flutter_midi_command_platform_interface/lib/method_channel_midi_command.dart b/packages/flutter_midi_command_platform_interface/lib/method_channel_midi_command.dart index 96b95063..c46f096c 100644 --- a/packages/flutter_midi_command_platform_interface/lib/method_channel_midi_command.dart +++ b/packages/flutter_midi_command_platform_interface/lib/method_channel_midi_command.dart @@ -111,7 +111,15 @@ class MethodChannelMidiCommand extends MidiCommandPlatform ); device.name = hostDevice.name ?? '-'; device.type = _fromHostDeviceType(hostDevice.type); - device.connected = hostDevice.connected ?? false; + // A passive device-list refresh must not collapse a transitional state. The + // `connected` setter can only express connected/disconnected, so applying it + // while a connect/disconnect is in flight would clobber the connecting/ + // disconnecting state those explicit flows (and their native callbacks) own. + final state = device.connectionState; + if (state != MidiConnectionState.connecting && + state != MidiConnectionState.disconnecting) { + device.connected = hostDevice.connected ?? false; + } device.inputPorts = _fromHostPorts(hostDevice.inputs, MidiPortType.IN); device.outputPorts = _fromHostPorts(hostDevice.outputs, MidiPortType.OUT); if (id.isNotEmpty) { diff --git a/packages/flutter_midi_command_platform_interface/lib/midi_connection_exception.dart b/packages/flutter_midi_command_platform_interface/lib/midi_connection_exception.dart index f1f2c18a..6ecf77e4 100644 --- a/packages/flutter_midi_command_platform_interface/lib/midi_connection_exception.dart +++ b/packages/flutter_midi_command_platform_interface/lib/midi_connection_exception.dart @@ -58,6 +58,16 @@ class MidiPairingFailedException extends MidiConnectionException { : super(stage: MidiConnectionStage.pairing, message: 'Pairing failed.'); } +class MidiPairingInfoRemovedException extends MidiConnectionException { + MidiPairingInfoRemovedException({required super.deviceId, super.cause}) + : super( + stage: MidiConnectionStage.pairing, + message: + 'The device removed its pairing information. Forget the device in ' + 'the system Bluetooth settings, then pair again.', + ); +} + class MidiServiceDiscoveryException extends MidiConnectionException { MidiServiceDiscoveryException({required super.deviceId, super.cause}) : super( diff --git a/packages/flutter_midi_command_platform_interface/test/method_channel_midi_command_test.dart b/packages/flutter_midi_command_platform_interface/test/method_channel_midi_command_test.dart index 821dcf10..e602678f 100644 --- a/packages/flutter_midi_command_platform_interface/test/method_channel_midi_command_test.dart +++ b/packages/flutter_midi_command_platform_interface/test/method_channel_midi_command_test.dart @@ -194,6 +194,34 @@ void main() { }, ); + test( + 'devices does not clobber the connecting state of an in-progress device', + () async { + // The host still reports connected:false while a connect is in flight. + final host = + _FakeHostApi() + ..listedDevices = [ + pigeon.MidiHostDevice( + id: 'serial-1', + name: 'Serial', + type: pigeon.MidiDeviceType.serial, + connected: false, + ), + ]; + final platform = MethodChannelMidiCommand(hostApi: host); + + final device = (await platform.devices)!.single; + await platform.connectToDevice(device); + expect(device.connectionState, MidiConnectionState.connecting); + + // A concurrent device-list refresh must reuse the same instance without + // collapsing the transitional state back to disconnected (issue #159). + final refreshed = (await platform.devices)!.single; + expect(identical(refreshed, device), isTrue); + expect(device.connectionState, MidiConnectionState.connecting); + }, + ); + test( 'disconnect triggers host disconnect and transitions to disconnected', () async { diff --git a/test/flutter_midi_command_test.dart b/test/flutter_midi_command_test.dart index 62980668..bc719b2b 100644 --- a/test/flutter_midi_command_test.dart +++ b/test/flutter_midi_command_test.dart @@ -166,6 +166,19 @@ class _FailedConnectPlatform extends _FakePlatform { } } +class _HangingConnectPlatform extends _FakePlatform { + @override + Future connectToDevice( + MidiDevice device, { + List? ports, + }) async { + connected.add(device.id); + // Stay in flight without ever marking the device connected, so a + // disconnection can race the connect while it is still pending. + await Future.delayed(const Duration(milliseconds: 20)); + } +} + class _FakePlatformWithBleHost extends _FakePlatform { @override Future?> get devices async => [ @@ -501,6 +514,50 @@ void main() { ); }); + test( + 'disconnecting while connecting does not raise an unhandled async error', + () async { + final uncaught = []; + await runZonedGuarded( + () async { + final platform = _HangingConnectPlatform(); + MidiCommand.setPlatformOverride(platform); + final midi = MidiCommand(); + + final device = (await midi.devices)!.first; + + Object? callerError; + final future = midi + .connectToDevice( + device, + awaitConnectionTimeout: const Duration(seconds: 1), + ) + .catchError((Object error) { + callerError = error; + }); + + // Race a disconnection against the still-in-flight connect. + device.setConnectionState(MidiConnectionState.disconnected); + + await future; + + // The caller's handler receives the failure... + expect(callerError, isA()); + }, + (error, _) { + uncaught.add(error); + }, + ); + + // ...and nothing leaks to the zone's uncaught-error handler (issue #160). + expect( + uncaught, + isEmpty, + reason: 'connect/disconnect race must not surface an unhandled error', + ); + }, + ); + test('dispose releases singleton and allows creating a new instance', () { final platform = _FakePlatform(); MidiCommand.setPlatformOverride(platform);