Skip to content

Commit cdc8ec7

Browse files
committed
fix(sync): address PR review — guard Troubleshoot during sync, narrow temp-dir fallback
- Disable the Troubleshoot Sync Advanced entry while a sync is in flight (matches the former Reset tile), so recovery can't race an active sync. The error-banner route stays open (error != syncing). - resolveSyncTempDir now falls back to systemTemp ONLY when path_provider's plugin infra is absent (MissingPluginException / uninitialized test binding), not on any exception, so a genuine runtime failure surfaces instead of silently reintroducing the /tmp EPERM.
1 parent 1548ef0 commit cdc8ec7

3 files changed

Lines changed: 50 additions & 9 deletions

File tree

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,31 @@
11
import 'dart:io';
22

3+
import 'package:flutter/foundation.dart' show FlutterError;
4+
import 'package:flutter/services.dart' show MissingPluginException;
35
import 'package:path_provider/path_provider.dart';
46

57
/// Directory for streaming-sync temp files (base export + assembled base parts).
68
///
79
/// Prefers the app-container temp dir via path_provider. A sandboxed or
810
/// hardened-runtime macOS app is denied `Directory.systemTemp` (`/tmp`) ->
911
/// `PathAccessException` errno 1 (issue #509), so `/tmp` must never be used in
10-
/// production. Falls back to `systemTemp` ONLY when path_provider is
11-
/// unavailable -- e.g. a plain unit test with no mocked plugin channel -- where
12-
/// `/tmp` is writable, so the fallback keeps such tests green without every one
13-
/// of them mocking path_provider. In production getTemporaryDirectory always
14-
/// resolves, so the fallback never runs there.
12+
/// production.
13+
///
14+
/// Falls back to `systemTemp` ONLY when path_provider's plugin infrastructure
15+
/// is absent -- a plain unit test with no mocked channel
16+
/// ([MissingPluginException]) or no initialized test binding at all (the
17+
/// [FlutterError] "Binding has not yet been initialized"). In tests `/tmp` is
18+
/// writable, so this keeps them green without every one mocking path_provider.
19+
/// Any OTHER failure (e.g. a real [PlatformException] from the native side) is
20+
/// a genuine runtime problem and propagates, rather than silently
21+
/// reintroducing the `/tmp` EPERM this fix exists to avoid. In production
22+
/// getTemporaryDirectory resolves normally, so no fallback runs.
1523
Future<Directory> resolveSyncTempDir() async {
1624
try {
1725
return await getTemporaryDirectory();
18-
} catch (_) {
26+
} on MissingPluginException {
27+
return Directory.systemTemp;
28+
} on FlutterError {
1929
return Directory.systemTemp;
2030
}
2131
}

lib/features/settings/presentation/pages/cloud_sync_page.dart

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,11 @@ class CloudSyncPage extends ConsumerWidget {
11831183
}
11841184

11851185
Widget _buildAdvancedSection(BuildContext context, WidgetRef ref) {
1186+
// Recovery resets sync identity/cursors and cloud files; doing that while a
1187+
// sync is writing races it. Disable the entry during an active sync, as the
1188+
// former "Reset Sync State" tile did. A sync ERROR (the banner route) is not
1189+
// `syncing`, so a stuck-error user can still reach Troubleshoot.
1190+
final isSyncing = ref.watch(syncStateProvider).status == SyncStatus.syncing;
11861191
return Column(
11871192
crossAxisAlignment: CrossAxisAlignment.start,
11881193
children: [
@@ -1199,9 +1204,14 @@ class CloudSyncPage extends ConsumerWidget {
11991204
leading: const Icon(Icons.build),
12001205
title: const Text('Troubleshoot Sync'),
12011206
subtitle: const Text('Fix a stuck sync or free cloud space'),
1202-
onTap: () => Navigator.of(context).push(
1203-
MaterialPageRoute(builder: (_) => const TroubleshootSyncPage()),
1204-
),
1207+
enabled: !isSyncing,
1208+
onTap: isSyncing
1209+
? null
1210+
: () => Navigator.of(context).push(
1211+
MaterialPageRoute(
1212+
builder: (_) => const TroubleshootSyncPage(),
1213+
),
1214+
),
12051215
),
12061216
ListTile(
12071217
leading: const Icon(Icons.logout),

test/features/settings/presentation/pages/cloud_sync_page_test.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,27 @@ void main() {
12601260
expect(find.text('Repair Sync'), findsOneWidget);
12611261
});
12621262

1263+
testWidgets('Troubleshoot Sync entry is disabled during an active sync', (
1264+
tester,
1265+
) async {
1266+
// Syncing shows an indeterminate spinner that never settles.
1267+
await pumpPage(
1268+
tester,
1269+
syncState: const SyncState(status: SyncStatus.syncing),
1270+
settle: false,
1271+
);
1272+
await tester.pump();
1273+
1274+
final tile = tester.widget<ListTile>(
1275+
find.ancestor(
1276+
of: find.text('Troubleshoot Sync'),
1277+
matching: find.byType(ListTile),
1278+
),
1279+
);
1280+
expect(tile.enabled, isFalse);
1281+
expect(tile.onTap, isNull);
1282+
});
1283+
12631284
testWidgets('tapping the sync error banner opens Troubleshoot Sync', (
12641285
tester,
12651286
) async {

0 commit comments

Comments
 (0)