From f40a58fcfc19b45acbf113a52a413a0e466bee77 Mon Sep 17 00:00:00 2001 From: David Gregory Date: Tue, 7 Nov 2017 18:17:25 +0000 Subject: [PATCH] Optionally return more detailed exception messages when SQL statements fail --- .../main/scala/sqlest/executor/Database.scala | 55 +++++++++++++- .../sqlest/executor/SqlestException.scala | 6 ++ .../scala/sqlest/executor/ExecutorSpec.scala | 76 +++++++++++++++++++ .../scala/sqlest/executor/TestDatabase.scala | 31 ++++---- 4 files changed, 152 insertions(+), 16 deletions(-) create mode 100644 sqlest/src/main/scala/sqlest/executor/SqlestException.scala diff --git a/sqlest/src/main/scala/sqlest/executor/Database.scala b/sqlest/src/main/scala/sqlest/executor/Database.scala index af1feff..6ef05d5 100644 --- a/sqlest/src/main/scala/sqlest/executor/Database.scala +++ b/sqlest/src/main/scala/sqlest/executor/Database.scala @@ -30,12 +30,29 @@ import scala.util.control.NonFatal import sqlest.extractor.IndexedExtractor object Database { - def withDataSource(dataSource: DataSource, builder: StatementBuilder): Database = new Database { + def withDataSource( + dataSource: DataSource, + builder: StatementBuilder + ): Database = new Database { def getConnection: Connection = dataSource.getConnection val statementBuilder = builder } - def withDataSource(dataSource: DataSource, builder: StatementBuilder, connectionDescription: Connection => String): Database = { + def withDataSource( + dataSource: DataSource, + builder: StatementBuilder, + verboseExceptionMessages: Boolean + ): Database = new Database { + def getConnection: Connection = dataSource.getConnection + val statementBuilder = builder + override val verboseExceptions = verboseExceptionMessages + } + + def withDataSource( + dataSource: DataSource, + builder: StatementBuilder, + connectionDescription: Connection => String + ): Database = { val inConnectionDescription = connectionDescription new Database { def getConnection: Connection = dataSource.getConnection @@ -43,12 +60,28 @@ object Database { override val connectionDescription = Some(inConnectionDescription) } } + + def withDataSource( + dataSource: DataSource, + builder: StatementBuilder, + connectionDescription: Connection => String, + verboseExceptionMessages: Boolean + ): Database = { + val inConnectionDescription = connectionDescription + new Database { + def getConnection: Connection = dataSource.getConnection + val statementBuilder = builder + override val verboseExceptions = verboseExceptionMessages + override val connectionDescription = Some(inConnectionDescription) + } + } } trait Database { private[sqlest] def getConnection: Connection private[sqlest] def statementBuilder: StatementBuilder private[sqlest] def connectionDescription: Option[Connection => String] = None + private[sqlest] def verboseExceptions: Boolean = false def withConnection[A](f: Connection => A): A = Session(this).withConnection(f) @@ -98,6 +131,12 @@ class Session(database: Database) extends Logging { if (resultSet != null) resultSet.close } catch { case e: SQLException => } } + } catch { + case NonFatal(e) => + if (!database.verboseExceptions) + throw e + else + throw new SqlestException(s"Exception running sql: ${logDetails(connection, sql, argumentLists)}", e) } finally { try { if (preparedStatement != null) preparedStatement.close @@ -267,6 +306,12 @@ case class Transaction(database: Database) extends Session(database) { val endTime = new DateTime logger.info(s"Ran sql in ${endTime.getMillis - startTime.getMillis}ms: ${logDetails(connection, sql, argumentLists)}") result + } catch { + case NonFatal(e) => + if (!database.verboseExceptions) + throw e + else + throw new SqlestException(s"Exception running sql: ${logDetails(connection, sql, argumentLists)}", e) } finally { try { if (preparedStatement != null) preparedStatement.close @@ -292,6 +337,12 @@ case class Transaction(database: Database) extends Session(database) { val endTime = new DateTime logger.info(s"Ran sql in ${endTime.getMillis - startTime.getMillis}ms: ${logDetails(connection, sql, argumentLists)}") keys + } catch { + case NonFatal(e) => + if (!database.verboseExceptions) + throw e + else + throw new SqlestException(s"Exception running sql: ${logDetails(connection, sql, argumentLists)}", e) } finally { try { if (preparedStatement != null) preparedStatement.close diff --git a/sqlest/src/main/scala/sqlest/executor/SqlestException.scala b/sqlest/src/main/scala/sqlest/executor/SqlestException.scala new file mode 100644 index 0000000..c73705d --- /dev/null +++ b/sqlest/src/main/scala/sqlest/executor/SqlestException.scala @@ -0,0 +1,6 @@ +package sqlest.executor + +case class SqlestException( + message: String, + cause: Throwable +) extends Exception(message, cause) diff --git a/sqlest/src/test/scala/sqlest/executor/ExecutorSpec.scala b/sqlest/src/test/scala/sqlest/executor/ExecutorSpec.scala index 311103b..67bbb12 100644 --- a/sqlest/src/test/scala/sqlest/executor/ExecutorSpec.scala +++ b/sqlest/src/test/scala/sqlest/executor/ExecutorSpec.scala @@ -365,4 +365,80 @@ class ExecutorSpec extends FlatSpec with Matchers { insertStatement.execute } } + + it should "return verbose exception messages when configured to do so" in { + val database = TestDatabase(testResultSet, Some(keyResultSet), shouldThrow = true, verboseExceptionMessages = true) + + val selectException = intercept[SqlestException] { + database.withSession { implicit session => + selectStatement.fetchAll + } + } + + assert(selectException.message.startsWith("Exception running sql")) + selectException.cause shouldBe database.anException + + val insertException = intercept[SqlestException] { + database.withTransaction { implicit transaction => + insertStatement.execute + } + } + + assert(insertException.message.startsWith("Exception running sql")) + insertException.cause shouldBe database.anException + + val insertReturningKeysException = intercept[SqlestException] { + database.withTransaction { implicit transaction => + insertStatement.executeReturningKeys[String] + } + } + + assert(insertReturningKeysException.message.startsWith("Exception running sql")) + insertReturningKeysException.cause shouldBe database.anException + + val updateException = intercept[SqlestException] { + database.withTransaction { implicit transaction => + updateStatement.execute + } + } + + assert(updateException.message.startsWith("Exception running sql")) + updateException.cause shouldBe database.anException + } + + it should "return the underlying exception otherwise" in { + val database = TestDatabase(testResultSet, Some(keyResultSet), shouldThrow = true, verboseExceptionMessages = false) + + val selectException = intercept[Exception] { + database.withSession { implicit session => + selectStatement.fetchAll + } + } + + selectException shouldBe database.anException + + val insertException = intercept[Exception] { + database.withTransaction { implicit transaction => + insertStatement.execute + } + } + + insertException shouldBe database.anException + + val insertReturningKeysException = intercept[Exception] { + database.withTransaction { implicit transaction => + insertStatement.executeReturningKeys[String] + } + } + + insertReturningKeysException shouldBe database.anException + + val updateException = intercept[Exception] { + database.withTransaction { implicit transaction => + updateStatement.execute + } + } + + updateException shouldBe database.anException + } } diff --git a/sqlest/src/test/scala/sqlest/executor/TestDatabase.scala b/sqlest/src/test/scala/sqlest/executor/TestDatabase.scala index a735844..51a5cef 100644 --- a/sqlest/src/test/scala/sqlest/executor/TestDatabase.scala +++ b/sqlest/src/test/scala/sqlest/executor/TestDatabase.scala @@ -19,9 +19,12 @@ package sqlest.executor import java.sql.ResultSet import sqlest._ -case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = None) extends Database { +case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = None, shouldThrow: Boolean = false, verboseExceptionMessages: Boolean = false) extends Database { var preparedStatement: Option[AbstractPreparedStatement] = None var lastConnection: Option[AbstractConnection] = None + override val verboseExceptions = verboseExceptionMessages + + val anException = new Exception("Oh noes!") def statementBuilder: StatementBuilder = sqlest.sql.H2StatementBuilder @@ -40,14 +43,14 @@ case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = override def setAutoCommit(autoCommit: Boolean) = {} override def createStatement() = new AbstractPreparedStatement { val sql = null - override def execute(sql: String) = true + override def execute(sql: String) = if (shouldThrow) throw anException else true } override def prepareStatement(inSql: String, columnIndices: Array[Int]) = { val statement = new AbstractPreparedStatement { val sql = inSql - override def executeQuery() = resultSet - override def executeUpdate() = 1 - override def executeBatch() = Array() + override def executeQuery() = if (shouldThrow) throw anException else resultSet + override def executeUpdate() = if (shouldThrow) throw anException else 1 + override def executeBatch() = if (shouldThrow) throw anException else Array() override def getGeneratedKeys(): java.sql.ResultSet = { keyResultSet.getOrElse(null) } @@ -58,9 +61,9 @@ case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = override def prepareStatement(inSql: String, columnNames: Array[String]) = { val statement = new AbstractPreparedStatement { val sql = inSql - override def executeQuery() = resultSet - override def executeUpdate() = 1 - override def executeBatch() = Array() + override def executeQuery() = if (shouldThrow) throw anException else resultSet + override def executeUpdate() = if (shouldThrow) throw anException else 1 + override def executeBatch() = if (shouldThrow) throw anException else Array() override def getGeneratedKeys(): java.sql.ResultSet = { keyResultSet.getOrElse(null) } @@ -71,9 +74,9 @@ case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = override def prepareStatement(inSql: String, returnGeneratedKeys: Int) = { val statement = new AbstractPreparedStatement { val sql = inSql - override def executeQuery() = resultSet - override def executeUpdate() = 1 - override def executeBatch() = Array() + override def executeQuery() = if (shouldThrow) throw anException else resultSet + override def executeUpdate() = if (shouldThrow) throw anException else 1 + override def executeBatch() = if (shouldThrow) throw anException else Array() override def getGeneratedKeys(): java.sql.ResultSet = { if (returnGeneratedKeys == java.sql.Statement.RETURN_GENERATED_KEYS) keyResultSet.getOrElse(null) @@ -86,9 +89,9 @@ case class TestDatabase(resultSet: ResultSet, keyResultSet: Option[ResultSet] = override def prepareStatement(inSql: String) = { val statement = new AbstractPreparedStatement { val sql = inSql - override def executeQuery() = resultSet - override def executeUpdate() = 1 - override def executeBatch() = Array() + override def executeQuery() = if (shouldThrow) throw anException else resultSet + override def executeUpdate() = if (shouldThrow) throw anException else 1 + override def executeBatch() = if (shouldThrow) throw anException else Array() override def getGeneratedKeys(): java.sql.ResultSet = null } preparedStatement = Some(statement)