Skip to content

Commit

Permalink
Fix #118: Don't print stacktrace on SQL syntax error.
Browse files Browse the repository at this point in the history
  • Loading branch information
Ondrej Zizka committed Dec 1, 2024
1 parent 1b5e659 commit fd4b06e
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/main/kotlin/cz/dynawest/csvcruncher/Cruncher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ class Cruncher(private val options: Options) {
dirToCreate.mkdirs()

// Get the columns info: Perform the SQL, LIMIT 1.
val columnsDef: Map<String, String> = dbHelper.extractColumnsInfoFrom1LineSelect(sql)
val columnsDef: Map<String, String> = dbHelper.extractColumnsInfoFrom1LineSelect(sql, export.sqlQuery)
output.columnNamesAndTypes = columnsDef


Expand Down
32 changes: 24 additions & 8 deletions src/main/kotlin/cz/dynawest/csvcruncher/HsqlDbHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class HsqlDbHelper(val jdbcConn: Connection) {
* Get the columns info: Perform the SQL, LIMIT 1.
*/
@Throws(SQLException::class)
fun extractColumnsInfoFrom1LineSelect(sql: String): Map<String, String> {
fun extractColumnsInfoFrom1LineSelect(sql: String, userSql: String?): Map<String, String> {

val sql = setOrReplaceLimit(sql, "LIMIT 1")

Expand All @@ -97,14 +97,30 @@ class HsqlDbHelper(val jdbcConn: Connection) {
jdbcConn.prepareStatement(sql)
}
catch (ex: SQLSyntaxErrorException) {
// TBD: This would be nice to refactor for wherever SQL is executed.

val isWholeSqlInExMsg = ex.message?.contains("[$sql]") ?: false
val isSqlSameAsUserSql = sql == userSql

if (ex.message!!.contains("unexpected token:")) {
throw SqlSyntaxCruncherException("""
| The SQL contains syntax error:
| ${ex.message}
| $sql
| This may be your SQL error or caused by alteration by CsvCruncher.
| See https://github.com/OndraZizka/csv-cruncher/issues for known bugs.
| """.trimMargin())
var msg = """
| The SQL contains syntax error:
| ${ex.message}
"""

if (!isWholeSqlInExMsg || !isSqlSameAsUserSql)
msg += "| Export's SQL: $sql\n"

if (isSqlSameAsUserSql)
msg += "| This is likely an error in your SQL. Please refer to HSQLDB SQL syntax: https://www.hsqldb.org/doc/guide/dataaccess-chapt.html#dac_sql_select_statement"
else
msg += """
| This may be your SQL error or caused by alteration by CsvCruncher.
| Check HSQLDB SQL syntax: https://www.hsqldb.org/doc/guide/dataaccess-chapt.html#dac_sql_select_statement
| See https://github.com/OndraZizka/csv-cruncher/issues for known bugs.
| """

throw SqlSyntaxCruncherException(msg.trimMargin())
}
if (ex.message!!.contains("object not found:")) {
throw throwHintForObjectNotFound(ex, this, sql)
Expand Down
34 changes: 30 additions & 4 deletions src/main/kotlin/cz/dynawest/csvcruncher/app/App.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cz.dynawest.csvcruncher.app

import ch.qos.logback.classic.Level
import cz.dynawest.csvcruncher.Cruncher
import cz.dynawest.csvcruncher.CsvCruncherException
import cz.dynawest.csvcruncher.SqlSyntaxCruncherException
import cz.dynawest.csvcruncher.util.logger
import org.slf4j.Logger
import org.slf4j.LoggerFactory
Expand Down Expand Up @@ -31,9 +33,25 @@ object App {
try {
mainNoExit(args)
}
catch (ex: IllegalArgumentException) {
println("" + ex.message)
System.exit(1)
catch (ex: Exception) {
val exitCode = when (ex) {
// Known app errors, or user errors -> no stacktrace, exit with error code.
is IllegalArgumentException, -> 1
is SqlSyntaxCruncherException -> 11
is CsvCruncherException -> 20
// Unknown -> Print with stacktrace.
else -> -1;
}
if (exitCode == -1) {
System.err.println("" + ex.message)
printBugReportingGuide(System.err, null)
System.exit(exitCode)
}
else {
//log.error("CSV Cruncher failed: " + ex.message, ex)
System.err.println("CSV Cruncher failed: " + ex.message + "\n" + ex)
System.exit(127)
}
}
catch (ex: Throwable) {
log.error("CSV Cruncher failed: " + ex.message, ex)
Expand Down Expand Up @@ -62,14 +80,22 @@ object App {
outputStream.println(" For more, read the README.md distributed with CsvCruncher, or at: https://github.com/OndraZizka/csv-cruncher#readme")
}

internal fun printBugReportingGuide(outputStream: PrintStream, options: Options?) {
if (options?.logLevel == LogLevel.OFF) return;
outputStream.println(" If you believe this is a bug, feel free to report it at https://github.com/OndraZizka/csv-cruncher/issues")
outputStream.println(" but please check the open tickets first.")
if (options?.logLevel !in setOf(null, LogLevel.DEBUG, LogLevel.TRACE))
outputStream.println(" For more info about what CsvCruncher did before the error, try --logLevel=${LogLevel.DEBUG.name}")
}

private fun setLogLevel(options: Options) {
if (options.logLevel == null) return

val configuredLevel: Level? = Level.toLevel(options.logLevel!!.name)

val rootLogger: Logger = LoggerFactory.getLogger("root")
val logBackLogger = rootLogger as ch.qos.logback.classic.Logger
rootLogger.debug("Changing all loggers' level to ${configuredLevel} as per option --logLevel=${options.logLevel!!.name}. Possible levels: " + LogLevel.values().joinToString(", "))
rootLogger.debug("Changing all loggers' level to ${configuredLevel} as per option --logLevel=${options.logLevel!!.name}. Possible levels: " + LogLevel.entries.joinToString(", "))
logBackLogger.level = configuredLevel

// For now, this needs to be synchronized with logback.xml, to override all specifically set there.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import cz.dynawest.csvcruncher.CsvCruncherTestUtils
import cz.dynawest.csvcruncher.CsvCruncherTestUtils.extractArgumentsFromTestCommandFormat
import cz.dynawest.csvcruncher.CsvCruncherTestUtils.testDataDir
import cz.dynawest.csvcruncher.CsvCruncherTestUtils.testOutputDir
import cz.dynawest.csvcruncher.SqlSyntaxCruncherException
import cz.dynawest.csvcruncher.app.ExportArgument.TargetType
import cz.dynawest.csvcruncher.app.OptionsParser
import cz.dynawest.csvcruncher.util.FilesUtils.parseColumnsFromFirstCsvLine
import org.assertj.core.api.Assertions
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertIterableEquals
Expand Down Expand Up @@ -129,6 +131,26 @@ class OptionsCombinationsTest {
assertIterableEquals("Op,recording_group_id,status,storage_base_url,owner_id,room_id,session_id,requested_ts,created_ts,deleted_ts,updated_ts,is_deleted,acc_consumer_id".split(","), columnNames)
}

@Test
fun sqlSyntaxError_noStackTrace(testInfo: TestInfo) {
val command =
" | -in | $testDataDir/eapBuilds.csv" +
" | -sql | SELECT * LIMIT 1" + // SQL syntax error.
" | -out | $testOutputDir/sqlSyntaxError_noStackTrace.csv" +
" | --logLevel=ERROR"

Assertions.assertThatThrownBy {
CsvCruncherTestUtils.runCruncherWithArguments(command)
}
.hasNoSuppressedExceptions()
.hasMessageContaining("SELECT * LIMIT 1")
.isInstanceOf(SqlSyntaxCruncherException::class.java)

val resultCsv = Paths.get("$testOutputDir/sqlSyntaxError_noStackTrace.csv").toFile()
assertTrue( ! resultCsv.exists() )
}


/**
* Fails because of:
* $table placeholder not replaced when dir input with non-canonical JSONs is used. #149
Expand Down

0 comments on commit fd4b06e

Please sign in to comment.