From db23460fd595074354197b378efdcfea35c06bec Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Thu, 1 Oct 2020 13:15:05 +0200 Subject: [PATCH] Implement proper script runtime exception rendering with tests #KT-42335 fixed --- .../experimental/jvmhost/test/ReplTest.kt | 17 ++++++ .../jvm/util/runtimeExceptionReporting.kt | 44 +++++++++++++++ .../AbstractScriptEvaluationExtension.kt | 29 ++++------ .../integration/exceptionWithCause.kts | 9 +++ .../plugin/ScriptingWithCliCompilerTest.kt | 28 ++++++++++ .../compiler/test/ScriptEvaluationTest.kt | 56 +++++++++++++++++++ .../compiler/test/scriptTestsUtil.kt | 2 +- 7 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 libraries/scripting/jvm/src/kotlin/script/experimental/jvm/util/runtimeExceptionReporting.kt create mode 100644 plugins/scripting/scripting-compiler/testData/integration/exceptionWithCause.kts create mode 100644 plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptEvaluationTest.kt diff --git a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt index ea28e2bae81..90d5bab0b97 100644 --- a/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt +++ b/libraries/scripting/jvm-host-test/test/kotlin/script/experimental/jvmhost/test/ReplTest.kt @@ -87,6 +87,22 @@ class ReplTest : TestCase() { ) } + @Test + fun testEvalWithExceptionWithCause() { + checkEvaluateInRepl( + sequenceOf( + """ + try { + throw Exception("Error!") + } catch (e: Exception) { + throw Exception("Oh no", e) + } + """.trimIndent() + ), + sequenceOf(Exception("Oh no", Exception("Error!"))) + ) + } + @Test fun testEvalWithErrorWithLocation() { checkEvaluateInReplDiags( @@ -240,6 +256,7 @@ class ReplTest : TestCase() { is ResultValue.Error -> Assert.assertTrue( "#$index: Expected $expectedVal, got Error: ${actualVal.error}", expectedVal is Throwable && expectedVal.message == actualVal.error.message + && expectedVal.cause?.message == actualVal.error.cause?.message ) is ResultValue.NotEvaluated -> Assert.assertEquals( "#$index: Expected $expectedVal, got NotEvaluated", diff --git a/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/util/runtimeExceptionReporting.kt b/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/util/runtimeExceptionReporting.kt new file mode 100644 index 00000000000..505e25f7826 --- /dev/null +++ b/libraries/scripting/jvm/src/kotlin/script/experimental/jvm/util/runtimeExceptionReporting.kt @@ -0,0 +1,44 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package kotlin.script.experimental.jvm.util + +import java.io.PrintStream +import kotlin.script.experimental.api.ResultValue + +fun ResultValue.Error.renderError(stream: PrintStream) { + var trace = error.stackTrace + val wrappingTrace = wrappingException?.stackTrace + if (wrappingException == null || trace.size < wrappingTrace!!.size) { + error.printStackTrace(stream) + } else { + // subtracting wrapping message stacktrace from error stacktrace to show only user-specific part of it + + fun PrintStream.printTrace(stackTrace: Array, dropLastFrames: Int) { + for (element in stackTrace.dropLast(dropLastFrames)) { + println("\tat $element") + } + } + + stream.println(error) + stream.printTrace(trace, wrappingTrace.size) + + var current: Throwable? = error.cause + var wrapping = error + val cyclesDetection = hashSetOf(wrapping) + while (current != null && cyclesDetection.add(current)) { + trace = current.stackTrace + val sameFramesCount = + trace.asList().asReversed().asSequence() + .zip(wrapping.stackTrace.asList().asReversed().asSequence()) + .takeWhile { it.first == it.second } + .count() + stream.println("Caused by: $current") + stream.printTrace(trace, sameFramesCount) + wrapping = current + current = current.cause + } + } +} diff --git a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/AbstractScriptEvaluationExtension.kt b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/AbstractScriptEvaluationExtension.kt index b7f576951c3..c21b0b850dd 100644 --- a/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/AbstractScriptEvaluationExtension.kt +++ b/plugins/scripting/scripting-compiler/src/org/jetbrains/kotlin/scripting/compiler/plugin/AbstractScriptEvaluationExtension.kt @@ -19,10 +19,11 @@ import org.jetbrains.kotlin.config.CompilerConfiguration import org.jetbrains.kotlin.scripting.configuration.ScriptingConfigurationKeys import org.jetbrains.kotlin.scripting.definitions.ScriptDefinitionProvider import java.io.File -import java.io.PrintStream +import java.util.* import kotlin.script.experimental.api.* import kotlin.script.experimental.host.StringScriptSource import kotlin.script.experimental.host.toScriptSource +import kotlin.script.experimental.jvm.util.renderError abstract class AbstractScriptEvaluationExtension : ScriptEvaluationExtension { @@ -70,13 +71,19 @@ abstract class AbstractScriptEvaluationExtension : ScriptEvaluationExtension { val extensionHint = if (configuration.get(ScriptingConfigurationKeys.SCRIPT_DEFINITIONS)?.let { it.size == 1 && it.first().isDefault } == true) " (.kts)" else "" - messageCollector.report(CompilerMessageSeverity.ERROR, "$error; Specify path to the script file$extensionHint as the first argument") + messageCollector.report( + CompilerMessageSeverity.ERROR, + "$error; Specify path to the script file$extensionHint as the first argument" + ) return ExitCode.COMPILATION_ERROR } script } else -> { - messageCollector.report(CompilerMessageSeverity.ERROR, "Illegal set of arguments: either -script or -expression arguments expected at this point") + messageCollector.report( + CompilerMessageSeverity.ERROR, + "Illegal set of arguments: either -script or -expression arguments expected at this point" + ) return ExitCode.COMPILATION_ERROR } } @@ -150,19 +157,3 @@ fun ScriptDiagnostic.Severity.toCompilerMessageSeverity(): CompilerMessageSeveri ScriptDiagnostic.Severity.DEBUG -> CompilerMessageSeverity.LOGGING } -private fun ResultValue.Error.renderError(stream: PrintStream) { - val fullTrace = error.stackTrace - if (wrappingException == null || fullTrace.size < wrappingException!!.stackTrace.size) { - error.printStackTrace(stream) - } else { - // subtracting wrapping message stacktrace from error stacktrace to show only user-specific part of it - // TODO: consider more reliable logic, e.g. comparing traces, fallback to full error printing in case of mismatch - // TODO: write tests - stream.println(error) - val scriptTraceSize = fullTrace.size - wrappingException!!.stackTrace.size - for (i in 0 until scriptTraceSize) { - stream.println("\tat " + fullTrace[i]) - } - } -} - diff --git a/plugins/scripting/scripting-compiler/testData/integration/exceptionWithCause.kts b/plugins/scripting/scripting-compiler/testData/integration/exceptionWithCause.kts new file mode 100644 index 00000000000..ef7e44a6d16 --- /dev/null +++ b/plugins/scripting/scripting-compiler/testData/integration/exceptionWithCause.kts @@ -0,0 +1,9 @@ +try { + try { + throw Exception("Error!") + } catch (e: Exception) { + throw Exception("Oh no", e) + } +} catch (e: Exception) { + throw Exception("Top", e) +} diff --git a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/plugin/ScriptingWithCliCompilerTest.kt b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/plugin/ScriptingWithCliCompilerTest.kt index 8090dce4b2e..165a629cca6 100644 --- a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/plugin/ScriptingWithCliCompilerTest.kt +++ b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/plugin/ScriptingWithCliCompilerTest.kt @@ -5,7 +5,10 @@ package org.jetbrains.kotlin.scripting.compiler.plugin +import org.jetbrains.kotlin.cli.common.CLITool import org.jetbrains.kotlin.cli.common.environment.setIdeaIoUseFallback +import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler +import org.jetbrains.kotlin.scripting.compiler.test.linesSplitTrim import org.junit.Assert import org.junit.Test import java.io.File @@ -84,6 +87,31 @@ class ScriptingWithCliCompilerTest { ) } + @Test + fun testExceptionWithCause() { + val (_, err, _) = captureOutErrRet { + CLITool.doMainNoExit( + K2JVMCompiler(), + arrayOf( + "-script", + "$TEST_DATA_DIR/integration/exceptionWithCause.kts" + ) + ) + } + val filteredErr = err.linesSplitTrim().filterNot { it.startsWith("WARN: ") } + Assert.assertEquals( + """ + java.lang.Exception: Top + at ExceptionWithCause.(exceptionWithCause.kts:8) + Caused by: java.lang.Exception: Oh no + at ExceptionWithCause.(exceptionWithCause.kts:5) + Caused by: java.lang.Exception: Error! + at ExceptionWithCause.(exceptionWithCause.kts:3) + """.trimIndent().linesSplitTrim(), + filteredErr + ) + } + private fun getMainKtsClassPath(): List { return listOf( File("dist/kotlinc/lib/kotlin-main-kts.jar").also { diff --git a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptEvaluationTest.kt b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptEvaluationTest.kt new file mode 100644 index 00000000000..7348b8829d6 --- /dev/null +++ b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/ScriptEvaluationTest.kt @@ -0,0 +1,56 @@ +import junit.framework.TestCase +import kotlinx.coroutines.runBlocking +import org.jetbrains.kotlin.scripting.compiler.plugin.impl.ScriptJvmCompilerIsolated +import org.jetbrains.kotlin.scripting.compiler.test.assertEqualsTrimmed +import java.io.ByteArrayOutputStream +import java.io.PrintStream +import kotlin.script.experimental.api.* +import kotlin.script.experimental.host.toScriptSource +import kotlin.script.experimental.jvm.BasicJvmScriptEvaluator +import kotlin.script.experimental.jvm.defaultJvmScriptingHostConfiguration +import kotlin.script.experimental.jvm.util.renderError + +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + + +class ScriptEvaluationTest : TestCase() { + + fun testExceptionWithCause() { + checkEvaluate( + """ + try { + throw Exception("Error!") + } catch (e: Exception) { + throw Exception("Oh no", e) + } + """.trimIndent().toScriptSource("exceptionWithCause.kts"), + """ + java.lang.Exception: Oh no + at ExceptionWithCause.(exceptionWithCause.kts:4) + Caused by: java.lang.Exception: Error! + at ExceptionWithCause.(exceptionWithCause.kts:2) + """.trimIndent() + ) + } + + private fun checkEvaluate(script: SourceCode, expectedOutput: String) { + val compilationConfiguration = ScriptCompilationConfiguration() + val compiler = ScriptJvmCompilerIsolated(defaultJvmScriptingHostConfiguration) + val compiled = compiler.compile(script, compilationConfiguration).valueOrThrow() + val evaluationConfiguration = ScriptEvaluationConfiguration() + val evaluator = BasicJvmScriptEvaluator() + val res = runBlocking { + evaluator.invoke(compiled, evaluationConfiguration).valueOrThrow() + } + assert(res.returnValue is ResultValue.Error) + ByteArrayOutputStream().use { os -> + val ps = PrintStream(os) + (res.returnValue as ResultValue.Error).renderError(ps) + ps.flush() + assertEqualsTrimmed(expectedOutput, os.toString()) + } + } +} diff --git a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/scriptTestsUtil.kt b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/scriptTestsUtil.kt index b206e462787..e3253644eb9 100644 --- a/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/scriptTestsUtil.kt +++ b/plugins/scripting/scripting-compiler/tests/org/jetbrains/kotlin/scripting/compiler/test/scriptTestsUtil.kt @@ -53,7 +53,7 @@ internal fun captureOut(body: () -> Unit): String { return outStream.toString() } -private fun String.linesSplitTrim() = +internal fun String.linesSplitTrim() = split('\n', '\r').map(String::trim).filter(String::isNotBlank) internal fun assertEqualsTrimmed(expected: String, actual: String) =