Add a BuildFailure class, and catch in the runners

[matanlurey]

We should not display the stack trace (or do it as FINE) for these type of errors.

/cc @natebosch

• Add the class to build
• Add support in build_runner
• Add support in bazel_codegen (Add BuildError support dart-archive/bazel#212)

[natebosch]

Some other options for completeness:

• Stop printing stack traces by default and only print them when some option is passed
• Check the type, if its a subclass of Error don't print the stack trace, and if it's a subclass of Exception print it.

[matanlurey]

I personally like BuildError, because it could also do other nice things:

• Capture the current primaryInput (?)
• Be extended by clients, i.e. TemplateBuildError, InjectionBuildError for us

[matanlurey]

@jakemac53 any thoughts here?

[jakemac53]

+1 I am in favor, we should also automagically wrap any normal exceptions from builders in one of these

[leonsenft]

I'm considering trying this before #2624, but it's unclear to me where such errors should be handled in build_runner and bazel_codegen.

build_runner (and friends) in particular has so much indirection I can't quite figure out where an individual builder is even invoked. Do you have any pointers on where to get started? Thanks!

[jakemac53]

https://github.com/dart-lang/build/blob/master/build_runner_core/lib/src/generate/build_impl.dart#L486 is roughly where the builder actually gets ran - that is calling this api from build https://github.com/dart-lang/build/blob/master/build/lib/src/generate/run_builder.dart.

[natebosch]

Builders are invoked here:

build/build_runner_core/lib/src/generate/build_impl.dart

Line 486 in 04ba219

() => runBuilder(

I would expect that special handling for this exception might involve the logging handler:

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

Line 28 in 04ba219

onError: (Object e, StackTrace s) {

[leonsenft]

Great, thanks for the help! One last quick question before I get started. The proposal suggests an Error (based on the name), but this would be better suited as an Exception because it's intended to be caught, right?

[natebosch]

Yes. This should be an exception. I would implement that interface but not include it in the name BuildFailure is my first choice.

[leonsenft]

Some other options for completeness:

• Stop printing stack traces by default and only print them when some option is passed

@natebosch Is this effectively done now? Looks like the stack trace is only printed in verbose mode:

build/build_runner/lib/src/logging/std_io_logging.dart

Lines 36 to 42 in feceaa4

	if (record.stackTrace != null && verbose) {
	var trace = Trace.from(record.stackTrace).foldFrames((f) {
	return f.package == 'build_runner' || f.package == 'build';
	}, terse: true);
	lines.add(trace);
	}

[natebosch]

Is this effectively done now?

Yeah I think so. We could consider having an even more suppressed stack trace for this exception type specifically, but I don't know that it is necessary.

[leonsenft]

In that case, what differentiates an exception that is specifically a BuildFailure versus one that is not? Won't both be caught, logged, and have their stack trace shown depending on whether the verbose flag is set or not?

[natebosch]

We'd have to do something like if (verbose && error is! BuildFailure) printStackTrace(). I think it's probably not really necessary though.

It's entirely possible that we don't even need to define BuildFailure and we could leave it at getting more concrete about how authors should handle failures. Things like not adding unnecessary catch and letting the exception, of any type, bubble up.

One benefit of having a single type that everyone agrees means "this isn't a bug, the build is bad" is that if builder authors do want to do some munging of exceptions they could use a pattern like:

try {
  await doBuilderWork();
} on BuildFailure {
  rethrow;
} catch (e, st) {
  throw MyBuilderHasABug(); // nice error message about filing a bug
}