Skip to content

Commit 54b3a97

Browse files
authored
Allow dart pub deps --json to fail without causing an explicit crash. (flutter#166778)
Closes flutter#166648. I think a better fix is to stop using `dart pub deps --json` and use @sigurdm's new `.dart_tool/` vendored solution, but that can be a follow-up change.
1 parent 4673e20 commit 54b3a97

File tree

4 files changed

+55
-21
lines changed

4 files changed

+55
-21
lines changed

packages/flutter_tools/lib/src/compute_dev_dependencies.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,13 @@ Future<Set<String>> computeExclusiveDevDependencies(
1818
required Logger logger,
1919
required FlutterProject project,
2020
}) async {
21-
final Map<String, Object?> jsonResult = await pub.deps(project);
21+
final Map<String, Object?>? jsonResult = await pub.deps(project);
22+
23+
// Avoid crashing if dart pub deps is not ready.
24+
// See https://github.com/flutter/flutter/issues/166648.
25+
if (jsonResult == null) {
26+
return <String>{};
27+
}
2228

2329
Never fail([String? reason]) {
2430
logger.printTrace(const JsonEncoder.withIndent(' ').convert(jsonResult));

packages/flutter_tools/lib/src/dart/pub.dart

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,9 @@ abstract class Pub {
158158
/// While it is guaranteed that, if successful, that the result are a valid
159159
/// JSON object, the exact contents returned are _not_ validated, and are left
160160
/// as a responsibility of the caller.
161-
Future<Map<String, Object?>> deps(FlutterProject project);
161+
///
162+
/// If `null` is returned, it should be assumed deps could not be determined.
163+
Future<Map<String, Object?>?> deps(FlutterProject project);
162164

163165
/// Runs pub in 'batch' mode.
164166
///
@@ -354,13 +356,22 @@ class _DefaultPub implements Pub {
354356
}
355357

356358
@override
357-
Future<Map<String, Object?>> deps(FlutterProject project) async {
359+
Future<Map<String, Object?>?> deps(FlutterProject project) async {
358360
final List<String> pubCommand = <String>[..._pubCommand, 'deps', '--json'];
361+
final RunResult runResult;
359362

360-
final RunResult runResult = await _processUtils.run(
361-
pubCommand,
362-
workingDirectory: project.directory.path,
363-
);
363+
// Don't treat this command as terminal if it fails.
364+
// See https://github.com/flutter/flutter/issues/166648
365+
try {
366+
runResult = await _processUtils.run(
367+
pubCommand,
368+
workingDirectory: project.directory.path,
369+
throwOnError: true,
370+
);
371+
} on io.ProcessException catch (e) {
372+
_logger.printWarning('${pubCommand.join(' ')} ${e.message}');
373+
return null;
374+
}
364375

365376
Never fail([String? reason]) {
366377
final String stdout = runResult.stdout;
@@ -374,11 +385,6 @@ class _DefaultPub implements Pub {
374385
);
375386
}
376387

377-
// Guard against dart pub deps crashing.
378-
if (runResult.exitCode != 0) {
379-
fail();
380-
}
381-
382388
// Guard against dart pub deps having explicitly invalid output.
383389
try {
384390
final Object? result = json.decode(runResult.stdout);

packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:io' as io;
6+
57
import 'package:file/file.dart';
68
import 'package:file/memory.dart';
79
import 'package:flutter_tools/src/base/logger.dart';
@@ -356,6 +358,17 @@ void main() {
356358
);
357359
});
358360

361+
test('a pub error is treated as no data available instead of terminal', () async {
362+
final ProcessManager processes = _dartPubDepsCrashes(project: project);
363+
final Set<String> dependencies = await computeExclusiveDevDependencies(
364+
pub(processes),
365+
project: project,
366+
logger: logger,
367+
);
368+
369+
expect(dependencies, isEmpty, reason: 'pub deps crashed, but was not terminal');
370+
});
371+
359372
test('throws and logs on invalid JSON', () async {
360373
final ProcessManager processes = _dartPubDepsReturns('''
361374
{
@@ -420,3 +433,18 @@ ProcessManager _dartPubDepsReturns(String dartPubDepsOutput, {required FlutterPr
420433
),
421434
]);
422435
}
436+
437+
ProcessManager _dartPubDepsCrashes({required FlutterProject project}) {
438+
return FakeProcessManager.list(<FakeCommand>[
439+
FakeCommand(
440+
command: const <String>[_dartBin, 'pub', '--suppress-analytics', 'deps', '--json'],
441+
workingDirectory: project.directory.path,
442+
exception: const io.ProcessException('pub', <String>[
443+
'pub',
444+
'--suppress-analytics',
445+
'deps',
446+
'--json',
447+
]),
448+
),
449+
]);
450+
}

packages/flutter_tools/test/general.shard/dart/pub_deps_test.dart

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ void main() {
4343
);
4444
});
4545

46-
testWithoutContext('fails on non-zero exit code', () async {
46+
testWithoutContext('returns null on non-zero exit code', () async {
4747
final BufferLogger logger = BufferLogger.test();
4848
final MemoryFileSystem fileSystem = MemoryFileSystem.test();
4949
final ProcessManager processManager = _dartPubDepsFails(
@@ -62,14 +62,8 @@ void main() {
6262
);
6363

6464
await expectLater(
65-
() => pub.deps(FlutterProject.fromDirectoryTest(fileSystem.currentDirectory)),
66-
throwsA(
67-
isA<StateError>().having(
68-
(StateError e) => e.message,
69-
'message',
70-
contains('dart pub --suppress-analytics deps --json failed'),
71-
),
72-
),
65+
pub.deps(FlutterProject.fromDirectoryTest(fileSystem.currentDirectory)),
66+
completion(isNull),
7367
);
7468
});
7569

0 commit comments

Comments
 (0)