build_test fails with an out-of-call-stack exception with build_resolvers 1.3.1

[kevmoo]

build_resolvers: 1.3.1
build_test: 0.10.11

See commit b6fdcb0

This is the failing test:

https://github.com/kevmoo/source_gen_test/blob/c1aff801cba9f46c27a1da854fbeed75fb7b2a1e/test/init_library_reader_test.dart#L54-L62

[jakemac53]

https://github.com/dart-lang/build/blob/master/build_test/lib/src/resolve_source.dart#L197 😢

[kevmoo]

https://github.com/dart-lang/build/blob/master/build_test/lib/src/resolve_source.dart#L197 😢

I tried just awaiting that – still has the problem!

[jakemac53]

Awaiting that fixes the issue but then the test has to be updated to expect the new exception type (which is the documented one, previously it threw a different one). I think it might be breaking for other use cases though to await there, I need to look into it a bit more.

cc @matanlurey

[jakemac53]

Ya if we awaited there we would block on the tearDown future being resolved so this would just hang tests that use that.

I don't really know what the correct solution is going to be here.

[natebosch]

Reopening for more discussion. This change can cause some legitimate failures to get lost.

When I drop the .catchError here I can't repro any failure with the source_gen_test tests. @kevmoo - do you expect that something changed in those tests?

I'm not sure how much retaining these specific errors would help though. If the async error comes after the builder completes normally we'd still ignore those errors.

build/build/lib/src/builder/logging.dart

Line 28 in 2d36ee3

if (done.isCompleted) return;

[jakemac53]

@natebosch should we close this at this point?

[natebosch]

I think #3253 and #3245 are examples of things that we can miss because of this design.

I found both of those examples while I was experimenting with migrating those tests to package:checks.

I think this is still a likely issue when using expect, but with checks we'd more likely get an unawaited_future, and the await resolves the issue.

I think that the complexity to solve this is not justified for continued safety with a library that has a foot-gun around async expectations. If we invest in async safety for package:matcher I'd first look at adding lints to use expectLater with AsyncMatcher which I think also would add enough safety for this problem.