Handle incorrect Scalac options and prevent printing ScalacWorker stacktraces #1606
Conversation
…known exception instead of allowing to None.get (Scala3) or NullPointerException (Scala2)
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Thanks, @WojciechMazur, overall looks good to me. Could you please go over my comments/questions and also please format code (in some places it's off).
| 2>&1 | grep -v "$expected" | ||
| } | ||
|
|
||
| test_logs_contains_no_stacktrace() { |
| @@ -88,7 +98,8 @@ public void checkPermission(Permission permission) { | |||
| code = e.code; | |||
| } catch (Exception e) { | |||
| System.err.println(e.getMessage()); | |||
| e.printStackTrace(); | |||
| if (e instanceof Interface.WorkerException) {} | |||
There was a problem hiding this comment.
Maybe here instead of empty block you could do System.err.println(e.getMessage());?
I think message is printed with e.printStackTrace(); in an else statement as well. If not then leave this as is.
There was a problem hiding this comment.
Sure, the e.printStackTrace would print the e.getMessage so previously there have been exception message printed twice. Let's only print either only exception message or stack trace
Description
Motivation
Fixes #1605
When passing invalid scalac options, eg.
-source:not-existingthe Scala compiler would either:NoneformDriver.setupleading to runtime exception inNone.get(Scala 3)Driver.newCompilerrequired to createProtoReporterorDepsTrackingReporterleading toreporterbeing not initialized (Scala 2)This fix handles both of this cases by termination executing earlier with a known exception.
In each of cases Scalac compiler would always show error message about which setting was invalid. This is expected behaviour. Because of that, there is no reason to print stack traces on invalid scalac setting from
ScalaInvokerexposing internal implementaiton to the user. It can be handled by either not filling stack traces or introducing a common subtype handled in the worker.The later option was chosen and additionally extended to handle expected compiler errors. This should reduce amount of boilerplate logs.