fix(#528): surface late repeater CLI replies instead of discarding them
A repeater reply that arrived after its command's window closed hit `if (commandId.isEmpty) return;` in RepeaterCommandService.handleResponse and was dropped with no log and no UI. Since the command timeout can be shorter than the app's own message-retrieval budget, that made "the command ran but the response was never reported" the normal outcome rather than an edge case, and it affected every repeater screen, not just the CLI one. RepeaterCommandService now remembers a timed-out command's prefix for two minutes, so a reply arriving afterwards can be attributed to the request it answers. Replies that reach no waiting command are handed to a new onUnmatchedResponse sink and logged through appLogger. No path through handleResponse returns without either completing a command, surfacing the payload, or logging why it could not. The CLI screen renders these as a distinct history entry naming the original command and how late it was. The settings screen applies the value if it is a `get` reply and tells the user it arrived late. repeater_status_screen already parsed responses independently of the service, so it had no silent-loss path to fix. Part of epic #473. Does not change the timeout window itself (#529), the reported duration (#531), or the stale-prefix fallback (#532).pull/561/head
Strycher
2 months ago
parent
d5789d8e96
commit
ff320c17b6
@ -0,0 +1,117 @@
|
||||
// #528 (epic #473): a repeater CLI reply that arrives with no command waiting
|
||||
// for it used to hit `if (commandId.isEmpty) return;` and vanish. No log, no
|
||||
// UI, nothing. Since a command's window can close before the app has even
|
||||
// finished fetching the message (MSG_WAITING then SYNC_NEXT_MESSAGE), that
|
||||
// made "the command ran but the response was never reported" the normal
|
||||
// outcome rather than an edge case.
|
||||
//
|
||||
// These tests pin the contract that no reply is ever dropped silently.
|
||||
|
||||
import 'dart:typed_data';
|
||||
|
||||
import 'package:flutter_test/flutter_test.dart';
|
||||
import 'package:meshcore_open/connector/meshcore_connector.dart';
|
||||
import 'package:meshcore_open/models/contact.dart';
|
||||
import 'package:meshcore_open/services/repeater_command_service.dart';
|
||||
import 'package:meshcore_open/storage/prefs_manager.dart';
|
||||
import 'package:shared_preferences/shared_preferences.dart';
|
||||
|
||||
Contact _repeater() => Contact(
|
||||
publicKey: Uint8List.fromList(List<int>.generate(32, (i) => i)),
|
||||
name: 'Test Repeater',
|
||||
type: 2,
|
||||
pathLength: 0,
|
||||
path: Uint8List(0),
|
||||
lastSeen: DateTime.fromMillisecondsSinceEpoch(0),
|
||||
);
|
||||
|
||||
void main() {
|
||||
TestWidgetsFlutterBinding.ensureInitialized();
|
||||
|
||||
late RepeaterCommandService service;
|
||||
late Contact repeater;
|
||||
late List<UnmatchedRepeaterResponse> surfaced;
|
||||
|
||||
setUp(() async {
|
||||
SharedPreferences.setMockInitialValues({});
|
||||
PrefsManager.reset();
|
||||
await PrefsManager.initialize();
|
||||
|
||||
service = RepeaterCommandService(MeshCoreConnector());
|
||||
repeater = _repeater();
|
||||
surfaced = [];
|
||||
service.onUnmatchedResponse = surfaced.add;
|
||||
});
|
||||
|
||||
tearDown(() => service.dispose());
|
||||
|
||||
test('a reply with no pending command is surfaced, not discarded', () {
|
||||
service.handleResponse(repeater, 'Version: 1.2.3');
|
||||
|
||||
expect(surfaced, hasLength(1));
|
||||
expect(surfaced.single.response, 'Version: 1.2.3');
|
||||
expect(surfaced.single.repeaterKeyHex, repeater.publicKeyHex);
|
||||
});
|
||||
|
||||
test('a reply for a command that already timed out names that command', () {
|
||||
service.recordExpiredCommandForTest('A3|', 'ver');
|
||||
|
||||
service.handleResponse(repeater, 'A3|Version: 1.2.3');
|
||||
|
||||
expect(surfaced, hasLength(1));
|
||||
final reply = surfaced.single;
|
||||
expect(reply.isLateReply, isTrue);
|
||||
expect(reply.command, 'ver');
|
||||
expect(reply.response, 'Version: 1.2.3', reason: 'prefix must be stripped');
|
||||
expect(reply.sinceTimeout, isNotNull);
|
||||
});
|
||||
|
||||
test('an unattributable reply is still surfaced, just without a command', () {
|
||||
service.handleResponse(repeater, 'ZZ|unsolicited chatter');
|
||||
|
||||
expect(surfaced, hasLength(1));
|
||||
expect(surfaced.single.isLateReply, isFalse);
|
||||
expect(surfaced.single.command, isNull);
|
||||
expect(surfaced.single.response, 'unsolicited chatter');
|
||||
});
|
||||
|
||||
test('an expired command is only consumed once', () {
|
||||
service.recordExpiredCommandForTest('A3|', 'ver');
|
||||
|
||||
service.handleResponse(repeater, 'A3|first');
|
||||
service.handleResponse(repeater, 'A3|second');
|
||||
|
||||
expect(surfaced, hasLength(2));
|
||||
expect(surfaced[0].command, 'ver');
|
||||
expect(
|
||||
surfaced[1].command,
|
||||
isNull,
|
||||
reason: 'the record is consumed by the first reply that claims it',
|
||||
);
|
||||
});
|
||||
|
||||
test('a reply is never swallowed when no callback is wired', () {
|
||||
service.onUnmatchedResponse = null;
|
||||
|
||||
// The contract is that this cannot throw and cannot hang. The logging path
|
||||
// still runs; the absence of a listener must not resurrect the silent drop
|
||||
// as an unhandled error.
|
||||
expect(
|
||||
() => service.handleResponse(repeater, 'orphan reply'),
|
||||
returnsNormally,
|
||||
);
|
||||
});
|
||||
|
||||
test('dispose clears the late-reply records', () {
|
||||
service.recordExpiredCommandForTest('A3|', 'ver');
|
||||
service.dispose();
|
||||
|
||||
// Re-arm a listener on the disposed service and confirm the stale record is
|
||||
// gone, so a reply after disposal cannot be mis-attributed to it.
|
||||
final seen = <UnmatchedRepeaterResponse>[];
|
||||
service.onUnmatchedResponse = seen.add;
|
||||
service.handleResponse(repeater, 'A3|Version: 1.2.3');
|
||||
|
||||
expect(seen.single.command, isNull);
|
||||
});
|
||||
}
|
||||
Loading…
Reference in new issue