Panic in prost-build when using custom service generator with non-standard behavior

[anelson]

I'm not sure if you'll consider this a bug or intended behavior, but I'm opening the issue so it's documented either way.

Let me start by saying thank you for your efforts on Prost. Prost has powered Elastio's internal services since day 1. Without Prost I doubt we would have had the will to adopt gRPC internally to the extent that we have.

We wrap prost and prost-build as part of an internal framework that generates gRPC clients and servers with some Elastio-specific instrumentation. Due to the extensibility of Prost via the ServiceGenerator trait, we're able to substantially modify the code gen behavior without forking and modifying Prost itself.

Recently we upgraded from 0.9 to 0.11. We've started to get a panic at build time, with a stack trace like the following:

  thread 'main' panicked at 'Module scalez::stor::blobs::health::v1 was not generated', /home/cornelius/sources/prost/prost-build/src/lib.rs:1188:40
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/std/src/panicking.rs:584:5
     1: core::panicking::panic_fmt
               at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/panicking.rs:142:14
     2: prost_build::Config::generate::{{closure}}
               at /home/cornelius/sources/prost/prost-build/src/lib.rs:1188:40
     3: core::option::Option<T>::unwrap_or_else
               at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/option.rs:825:21
     4: prost_build::Config::generate
               at /home/cornelius/sources/prost/prost-build/src/lib.rs:1186:27
     5: prost_build::Config::compile_fds
               at /home/cornelius/sources/prost/prost-build/src/lib.rs:934:23
     6: prost_build::Config::compile_protos
               at /home/cornelius/sources/prost/prost-build/src/lib.rs:1081:9
     7: matroskin_build::prost::generate_code
               at /home/cornelius/.cargo/registry/src/dl.cloudsmith.io-2dc4edbccd98c64c/matroskin-build-6.0.0/src/prost/mod.rs:179:5
     8: matroskin_build::Config::build
               at /home/cornelius/.cargo/registry/src/dl.cloudsmith.io-2dc4edbccd98c64c/matroskin-build-6.0.0/src/lib.rs:417:9
     9: scalez_stor_rpc_build::build_config_or_exit_err
               at /home/cornelius/projects/elastio/scalez/scalez-stor-rpc-build/src/lib.rs:95:23
    10: build_script_build::main
               at ./build.rs:50:5
    11: core::ops::function::FnOnce::call_once
               at /rustc/897e37553bba8b42751c67658967889d11ecd120/library/core/src/ops/function.rs:248:5

The line of code that is causing this panic is the modules.get_mut(&module).unwrap() call here:

prost/prost-build/src/lib.rs

Lines 1179 to 1184 in d5395ba

	if let Some(ref mut service_generator) = self.service_generator {
	for (module, package) in packages {
	let buf = modules.get_mut(&module).unwrap();
	service_generator.finalize_package(&package, buf);
	}
	}

The reason is that one of our custom generators does all of its code generation in ServiceGenerator::finalize_package; it doesn't produce anything in ServiceGenerator::finalize. Therefore the code slightly earlier in the generate function removes the module:

prost/prost-build/src/lib.rs

Lines 1173 to 1176 in d5395ba

	if buf.is_empty() {
	// Did not generate any code, remove from list to avoid inclusion in include file or output file list
	modules.remove(&request_module);
	}

So by the time modules.get_mut is called, the module in question has already been removed.

In prior versions this worked, I think this new behavior was introduced in #726.

For now I will work around this by making our ServiceGenerator::finalize implementation produce some non-empty no-op code like /* NO OP */, however this isn't ideal.

It's not explicit in the documentation of ServiceGenerator::finalize that generators must produce some output by the time finalize returns or a panic will occur. I assume this isn't intentional behavior, but maybe you don't consider it a bug. I'd be interested to hear your thoughts.

It looks like another user @matze hit this in #747, but they closed that issue as being a very specific case. Maybe a second user hitting this same issue generalizes it a bit ;)