Skip to content

Commit

Permalink
Move Throwable catch to ExtraProtobufGenerator.run (#1637)
Browse files Browse the repository at this point in the history
This catches errors that happen at any point in
`ExtraProtobufGenerator.run`, not just within
`handleCodeGeneratorRequest`. This, in turn, prevents more cases of
`ProtoScalaPBRule` aspect workers hanging. Part of #1482.

As I started experimenting with updating gRPC and ScalaPB libs, I bumped
the ScalaPB libs to 1.0.0-alpha.1 to try to resolve a problem. The
`ProtoScalaPBRule` workers then started hanging again.

After pulling the `catch ... Throwable` block from #1630 up into
`ExtraProtobufGenerator.run`, I got the following stack trace:

```txt
$ bazel build //test/proto/...

ERROR: .../external/com_google_protobuf/BUILD.bazel:334:14:
  ProtoScalaPBRule
  external/com_google_protobuf/wrappers_proto_jvm_extra_protobuf_generator_scalapb.srcjar
  failed: (Exit 1): scalapb_worker failed: error executing command
  (from target @com_google_protobuf//:wrappers_proto)

bazel-out/.../bin/src/scala/scripts/scalapb_worker
  ... (remaining 2 arguments skipped)

--jvm_extra_protobuf_generator_out: java.lang.IllegalAccessError:
  class scalapb.options.Scalapb$ScalaPbOptions tried to access method
  'com.google.protobuf.LazyStringArrayList
    com.google.protobuf.LazyStringArrayList.emptyList()'
  (scalapb.options.Scalapb$ScalaPbOptions and
   com.google.protobuf.LazyStringArrayList are in unnamed module
   of loader 'app')
      at scalapb.options.Scalapb$ScalaPbOptions.<init>(Scalapb.java:5021)
      at scalapb.options.Scalapb$ScalaPbOptions.<clinit>(Scalapb.java:11165)
      at scalapb.options.Scalapb.<clinit>(Scalapb.java:24184)
      at scalapb.options.compiler.Scalapb$.registerAllExtensions(Scalapb.scala:8)
      at scalarules.test.extra_protobuf_generator.ExtraProtobufGenerator$.run(ExtraProtobufGenerator.scala:48)
      at protocbridge.frontend.PluginFrontend$.$anonfun$runWithBytes$1(PluginFrontend.scala:51)
      at scala.util.Try$.apply(Try.scala:213)
      at protocbridge.frontend.PluginFrontend$.runWithBytes(PluginFrontend.scala:51)
      at protocbridge.frontend.PluginFrontend$.runWithInputStream(PluginFrontend.scala:121)
      at protocbridge.frontend.PosixPluginFrontend$.$anonfun$prepare$2(PosixPluginFrontend.scala:40)
      at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
      at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1$$anon$2.block(ExecutionContextImpl.scala:75)
      at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3118)
      at scala.concurrent.impl.ExecutionContextImpl$DefaultThreadFactory$$anon$1.blockOn(ExecutionContextImpl.scala:87)
      at scala.concurrent.package$.blocking(package.scala:146)
      at protocbridge.frontend.PosixPluginFrontend$.$anonfun$prepare$1(PosixPluginFrontend.scala:38)
      at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
      at scala.concurrent.Future$.$anonfun$apply$1(Future.scala:659)
      at scala.util.Success.$anonfun$map$1(Try.scala:255)
      at scala.util.Success.map(Try.scala:213)
      at scala.concurrent.Future.$anonfun$map$1(Future.scala:292)
      at scala.concurrent.impl.Promise.$anonfun$transform$1(Promise.scala:42)
      at scala.concurrent.impl.CallbackRunnable.run(Promise.scala:74)
      at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
      at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
      at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
      at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
      at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
      at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:183)

java.lang.RuntimeException: Exit with code 1
     at scala.sys.package$.error(package.scala:30)
     at scripts.ScalaPBWorker$.work(ScalaPBWorker.scala:44)
     at io.bazel.rulesscala.worker.Worker.persistentWorkerMain(Worker.java:96)
     at io.bazel.rulesscala.worker.Worker.workerMain(Worker.java:49)
     at scripts.ScalaPBWorker$.main(ScalaPBWorker.scala:39)
     at scripts.ScalaPBWorker.main(ScalaPBWorker.scala)

ERROR: .../external/com_google_protobuf/BUILD.bazel:334:14
  scala @com_google_protobuf//:wrappers_proto failed:
  (Exit 1): scalapb_worker failed: error executing command
  (from target @com_google_protobuf//:wrappers_proto)

bazel-out/.../bin/src/scala/scripts/scalapb_worker
  ... (remaining 2 arguments skipped)
```

Bumping to protobuf v28.2 resolved the issue, at the expense of
sacrificing Bazel 6 support, as v21.7 was the last to support Bazel 6.
But this issue is orthogonal to the original gRPC test failure issue,
which I'll address in a future change.
  • Loading branch information
mbland authored Nov 4, 2024
1 parent 35353b7 commit 716210f
Showing 1 changed file with 22 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,30 @@ class CustomProtobufGenerator(

object ExtraProtobufGenerator extends ProtocCodeGenerator {
override def run(req: Array[Byte]): Array[Byte] = {
val registry = ExtensionRegistry.newInstance()
Scalapb.registerAllExtensions(registry)
val request = CodeGeneratorRequest.parseFrom(req)
handleCodeGeneratorRequest(request).toByteArray
val b = CodeGeneratorResponse.newBuilder

try {
val registry = ExtensionRegistry.newInstance()
Scalapb.registerAllExtensions(registry)
val request = CodeGeneratorRequest.parseFrom(req)
handleCodeGeneratorRequest(request, b)

} catch {
case e: Throwable =>
// Yes, we want to catch _all_ errors and send them back to the
// requestor. Otherwise uncaught errors will cause the generator to
// die and the worker invoking it to hang.
val stackStream = new java.io.ByteArrayOutputStream
e.printStackTrace(new java.io.PrintStream(stackStream))
b.setError(stackStream.toString())
}
b.build.toByteArray
}

def handleCodeGeneratorRequest(request: CodeGeneratorRequest): CodeGeneratorResponse = {
val b = CodeGeneratorResponse.newBuilder
def handleCodeGeneratorRequest(
request: CodeGeneratorRequest,
b: CodeGeneratorResponse.Builder
) = {
ProtobufGenerator.parseParameters(request.getParameter) match {
case Right(params) =>
try {
Expand All @@ -73,18 +89,9 @@ object ExtraProtobufGenerator extends ProtocCodeGenerator {
} catch {
case e: GeneratorException =>
b.setError(e.message)
case e: Throwable =>
// Yes, we want to catch _all_ errors and send them back to the
// requestor. Otherwise uncaught errors will cause the generator to
// die and the worker invoking it to hang.
val stackStream = new java.io.ByteArrayOutputStream
e.printStackTrace(new java.io.PrintStream(stackStream))
b.setError(stackStream.toString())
}
case Left(error) =>
b.setError(error)
}
b.build
}

}

0 comments on commit 716210f

Please sign in to comment.