gapic-generator-typescript icon indicating copy to clipboard operation
gapic-generator-typescript copied to clipboard

unhandled exception ends up with error message indicating proto3_optional isn't supported

Open noahdietz opened this issue 2 years ago • 2 comments

If there is an unhandled exception thrown by gapic-generator-typescript generating an API leveraging proto3_optional, the logs (at least in bazel) indicate that there was an error because the generator did not respond to protoc with the fact that it supports proto3_optional, see the following logs: https://screenshot.googleplex.com/4AQCdHQoJDU8uGS.

This might actually make sense because the plugin itself never "responds" to protoc (because it crashed), and protoc then logs that the plugin failed because it doesn't support proto3_optional.

This might actually need to be fixed in protoc, but perhaps the gapic-generator-typescript plugin runner should capture all exceptions, respond to protoc with an error response (and a proper flag indicating it supports proto3_optional) using the "unhandled" exception. I'm not sure, but it did add indirection to debugging this http://yaqs/5695674741042446336.

noahdietz avatar Jun 30 '22 16:06 noahdietz

Yeah, we don't have a catch-all try .. catch:

https://github.com/googleapis/gapic-generator-typescript/blob/68f5160bf5781848a38957a550c0f4eeb83e2fd1/typescript/src/protoc-plugin.ts#L21..L33

In a fire-and-forget code such as a generator we normally prefer not to catch any exceptions but throw a lot, so that we just crash immediately in any suspicious case (thus ignoring the protoc expectations on the error reporting from a plugin). It actually gives more, not less, information because this way we see a stack that immediately points to a place where the error happened, compared to the protoc error interface that only has optional string error = 1;.

But if this makes Bazel happier I can totally wrap the plugin invocation in try .. catch :)

alexander-fenster avatar Jun 30 '22 17:06 alexander-fenster

Yeah the protoc error interface is not that robust. Could we just stuff the entire exception trace in the error string returned to protoc? Or just log the entire trace from the generator prior to responding with an error? Or is there stuff that typescript/nodejs itself is doing that we'd lose by doing that?

noahdietz avatar Jun 30 '22 17:06 noahdietz