Fix a bug in build_test where the resolve* apis would leak unhandled …

…async errors (#2599)

Fixes #2596

Specifically if the `action` callback throws any exceptions then we would leak those as unhandled async errors.

We can't await the call to `runBuilder` because that would block on the `tearDown` future which would hang tests.

The solution is to just swallow all errors from the unawaited future, and rely on the `onDone` Future to report most errors. We try/catch the action callback and forward the errors to the `onDone` completer.

build_runner_core/test/asset/file_based_test.dart

@@ -102,8 +102,8 @@ void main() {
 
     test('can read from the SDK', () async {
       expect(
-          await reader
-              .canRead(makeAssetId(r'$sdk|lib/dev_compiler/amd/dart_sdk.js')),
+          await reader.canRead(
+              makeAssetId(r'$sdk|lib/dev_compiler/kernel/amd/dart_sdk.js')),
           true);
     });
   });

build_test/CHANGELOG.md

@@ -1,6 +1,7 @@
-## 0.10.12-dev
+## 0.10.12
 
-- Internal changes.
+- Fix a bug with the `resolve*` apis where they would leak unhandled async
+  errors to client code if the provided action callback threw an error.
 
 ## 0.10.11
 

build_test/lib/src/resolve_source.dart

@@ -193,14 +193,21 @@ Future<T> _resolveAssets<T>(
     sourceAssets: inputAssets,
     rootPackage: rootPackage,
   );
-  // We don't care about the results of this build.
+  // We don't care about the results of this build, but we also can't await
+  // it because that would block on the `tearDown` of the `resolveBuilder`.
+  //
+  // We also dont want to leak errors as unhandled async errors so we swallow
+  // them here.
+  //
+  // Errors will still be reported through the resolver itself as well as the
+  // `onDone` future that we return.
   unawaited(runBuilder(
     resolveBuilder,
     inputAssets.keys,
     MultiAssetReader([inMemory, assetReader]),
     InMemoryAssetWriter(),
     resolvers ?? defaultResolvers,
-  ));
+  ).catchError((_) {}));
   return resolveBuilder.onDone.future;
 }
 
@@ -220,8 +227,11 @@ class _ResolveSourceBuilder<T> implements Builder {
   @override
   Future<void> build(BuildStep buildStep) async {
     if (_resolverFor != buildStep.inputId) return;
-    var result = await _action(buildStep.resolver);
-    onDone.complete(result);
+    try {
+      onDone.complete(await _action(buildStep.resolver));
+    } catch (e, s) {
+      onDone.completeError(e, s);
+    }
     await _tearDown;
   }
 

build_test/pubspec.yaml

@@ -1,6 +1,6 @@
 name: build_test
 description: Utilities for writing unit tests of Builders.
-version: 0.10.12-dev
+version: 0.10.12
 homepage: https://github.com/dart-lang/build/tree/master/build_test
 
 environment:

build_test/test/_files/example_lib.dart

@@ -4,4 +4,6 @@
 
 library example_lib;
 
+part 'example_part.dart';
+
 class Example {}

build_test/test/_files/example_part.dart

@@ -0,0 +1 @@
+part of 'example_lib.dart';

build_test/test/resolve_source_test.dart

@@ -119,6 +119,18 @@ void main() {
       expect(libExample.getType('Example'), isNotNull);
     });
   });
+
+  group('error handling', () {
+    test('getting the library for a part file', () async {
+      var partAsset = AssetId('build_test', 'test/_files/example_part.dart');
+      await resolveAsset(partAsset, (resolver) async {
+        expect(
+            () => resolver.libraryFor(partAsset),
+            throwsA(isA<NonLibraryAssetException>()
+                .having((e) => e.assetId, 'assetId', partAsset)));
+      });
+    });
+  });
 }
 
 String _toStringId(InterfaceType t) =>