[ZEPPELIN-6482] Guarantee system restoration when execution fails

[gyowoo1113]

What is this PR for?

StaticRepl.execute() temporarily redirects System.out and System.err to capture user program output. However, if compiler.getTask(...) or CompilationTask.call() throws an unexpected exception before the existing restoration logic is reached, the global streams can remain redirected.

This PR wraps the redirected-stream section in an outer try/finally so System.out and System.err are always restored to their original streams. A regression test was also added to verify that the streams are restored when CompilationTask.call() throws unexpectedly.

What type of PR is it?

Bug Fix

Todos

• - Wrap stream redirection in an outer try/finally to guarantee restoration
• - Add a compiler-injection overload for deterministic failure testing
• - Add Mockito as a test dependency to the java module
• - Add a regression test for CompilationTask.call() failure

What is the Jira issue?

[ZEPPELIN-6482]

How should this be tested?

./mvnw test -pl java passes successfully.

Screenshots (if appropriate)

N/A

Questions:

• Does the license files need to update? No
• Is there breaking changes for older versions? No
• Does this needs documentation? No

[tbonelee]

Nice catch, and the fix is the right shape.

Verified locally:

• ./mvnw test -pl java passes, 7 tests (including the 6 existing ones, so no regression on the normal paths).
• Removing just the two setOut/setErr lines from the finally makes StaticReplTest fail as expected. This is a real regression test, not just a test-shaped one.

Nothing blocking here. A few things I would like to see addressed:

[tbonelee]

Would you consider pulling the redirected section out into a private helper?

The bug you are fixing is really "restoration was scattered across three places". After this change the restoration is in one place, but the body still sits in a scope where oldOut / oldErr are visible, so it can be scattered again later. Moving the body into a helper takes those variables out of scope entirely, which makes the regression structurally impossible rather than a matter of discipline, and it shrinks the try/finally that carries the invariant down to something you can verify at a glance.

    try {
      System.setOut(newOut);
      System.setErr(newErr);
      return compileAndRun(generatedClassName, compiler, compilationUnits, baosOut, baosErr,
          newErr);
    } finally {
      System.setOut(oldOut);
      System.setErr(oldErr);
    }
  }

  private static String compileAndRun(String generatedClassName,
                                      JavaCompiler compiler,
                                      Iterable<? extends JavaFileObject> compilationUnits,
                                      ByteArrayOutputStream baosOut,
                                      ByteArrayOutputStream baosErr,
                                      PrintStream newErr) throws Exception {
    // body unchanged, only the scattered setOut/setErr restores and the inner finally are dropped
  }

The trade-off is a 6-parameter helper. If that feels worse to you, the current form is functionally correct as is. I compiled this variant locally and all 7 tests pass.

Unrelated, but since if (!success) always ends in a throw, the else on line 142 is redundant. That is pre-existing cleanup rather than part of this PR, so I will leave it to you.