From 6f40eb37cbdcdae032d73c3537df1e01d2e9e67c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 17 Oct 2014 19:37:45 -0400 Subject: [PATCH] - Exception messages have been spiffed up a bit. The SQL statement and parameters are not displayed if None, reducing confusion for error messages that weren't related to a statement. The full module and classname for the DBAPI-level exception is displayed, making it clear that this is a wrapped DBAPI exception. The statement and parameters themselves are bounded within a bracketed sections to better isolate them from the error message and from each other. fixes #3172 --- doc/build/changelog/changelog_10.rst | 13 +++++++++++ lib/sqlalchemy/exc.py | 21 +++++++++-------- test/base/test_except.py | 34 +++++++++++++++++----------- test/engine/test_execute.py | 6 ++--- test/engine/test_logging.py | 8 ++++--- test/sql/test_query.py | 7 +++--- 6 files changed, 57 insertions(+), 32 deletions(-) diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index ec812a0916..5aed3bdddd 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -21,6 +21,19 @@ series as well. For changes that are specific to 1.0 with an emphasis on compatibility concerns, see :doc:`/changelog/migration_10`. + .. change:: + :tags: feature, sql + :tickets: 3172 + + Exception messages have been spiffed up a bit. The SQL statement + and parameters are not displayed if None, reducing confusion for + error messages that weren't related to a statement. The full + module and classname for the DBAPI-level exception is displayed, + making it clear that this is a wrapped DBAPI exception. The + statement and parameters themselves are bounded within a bracketed + sections to better isolate them from the error message and from + each other. + .. change:: :tags: bug, orm :tickets: 3228 diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py index 5d35dc2e7a..3271d09d44 100644 --- a/lib/sqlalchemy/exc.py +++ b/lib/sqlalchemy/exc.py @@ -238,14 +238,16 @@ class StatementError(SQLAlchemyError): def __str__(self): from sqlalchemy.sql import util - params_repr = util._repr_params(self.params, 10) + details = [SQLAlchemyError.__str__(self)] + if self.statement: + details.append("[SQL: %r]" % self.statement) + if self.params: + params_repr = util._repr_params(self.params, 10) + details.append("[parameters: %r]" % params_repr) return ' '.join([ "(%s)" % det for det in self.detail - ] + [ - SQLAlchemyError.__str__(self), - repr(self.statement), repr(params_repr) - ]) + ] + details) def __unicode__(self): return self.__str__() @@ -289,10 +291,10 @@ class DBAPIError(StatementError): # not a DBAPI error, statement is present. # raise a StatementError if not isinstance(orig, dbapi_base_err) and statement: - msg = traceback.format_exception_only( - orig.__class__, orig)[-1].strip() return StatementError( - "%s (original cause: %s)" % (str(orig), msg), + "(%s.%s) %s" % + (orig.__class__.__module__, orig.__class__.__name__, + orig), statement, params, orig ) @@ -316,7 +318,8 @@ class DBAPIError(StatementError): text = 'Error in str() of DB-API-generated exception: ' + str(e) StatementError.__init__( self, - '(%s) %s' % (orig.__class__.__name__, text), + '(%s.%s) %s' % ( + orig.__class__.__module__, orig.__class__.__name__, text, ), statement, params, orig diff --git a/test/base/test_except.py b/test/base/test_except.py index a438e26d9e..918e7a0423 100644 --- a/test/base/test_except.py +++ b/test/base/test_except.py @@ -44,8 +44,10 @@ class WrapTest(fixtures.TestBase): 'this is a message', None, OperationalError(), DatabaseError) except sa_exceptions.DBAPIError as exc: - assert str(exc) \ - == "(OperationalError) 'this is a message' None" + eq_( + str(exc), + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message']") def test_tostring_large_dict(self): try: @@ -58,7 +60,8 @@ class WrapTest(fixtures.TestBase): OperationalError(), DatabaseError) except sa_exceptions.DBAPIError as exc: assert str(exc).startswith( - "(OperationalError) 'this is a message' {") + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: {") def test_tostring_large_list(self): try: @@ -68,8 +71,9 @@ class WrapTest(fixtures.TestBase): OperationalError(), DatabaseError) except sa_exceptions.DBAPIError as exc: assert str(exc).startswith( - "(OperationalError) 'this is a " - "message' [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]") + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: " + "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]]") def test_tostring_large_executemany(self): try: @@ -81,9 +85,10 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(OperationalError) 'this is a message' [{1: 1}, " + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: [{1: 1}, " "{1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: " - "1}, {1: 1}, {1: 1}]" + "1}, {1: 1}, {1: 1}]]" ) try: raise sa_exceptions.DBAPIError.instance('this is a message', [ @@ -92,10 +97,11 @@ class WrapTest(fixtures.TestBase): ], OperationalError(), DatabaseError) except sa_exceptions.DBAPIError as exc: eq_(str(exc), - "(OperationalError) 'this is a message' [{1: 1}, " + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: [{1: 1}, " "{1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: 1}, " "{1: 1}, {1: 1} ... displaying 10 of 11 total " - "bound parameter sets ... {1: 1}, {1: 1}]" + "bound parameter sets ... {1: 1}, {1: 1}]]" ) try: raise sa_exceptions.DBAPIError.instance( @@ -108,8 +114,9 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(OperationalError) 'this is a message' [(1,), " - "(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,)]") + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: [(1,), " + "(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,)]]") try: raise sa_exceptions.DBAPIError.instance('this is a message', [ (1, ), (1, ), (1, ), (1, ), (1, ), (1, ), (1, ), (1, ), (1, ), @@ -117,10 +124,11 @@ class WrapTest(fixtures.TestBase): ], OperationalError(), DatabaseError) except sa_exceptions.DBAPIError as exc: eq_(str(exc), - "(OperationalError) 'this is a message' [(1,), " + "(test.base.test_except.OperationalError) " + "[SQL: 'this is a message'] [parameters: [(1,), " "(1,), (1,), (1,), (1,), (1,), (1,), (1,) " "... displaying 10 of 11 total bound " - "parameter sets ... (1,), (1,)]" + "parameter sets ... (1,), (1,)]]" ) def test_db_error_busted_dbapi(self): diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index e0bba0afa2..00b4ba7f36 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -285,7 +285,7 @@ class ExecuteTest(fixtures.TestBase): def _go(conn): assert_raises_message( tsa.exc.StatementError, - r"nope \(original cause: Exception: nope\) u?'SELECT 1 ", + r"\(exceptions.Exception\) nope \[SQL\: u?'SELECT 1 ", conn.execute, select([1]). where( @@ -1608,7 +1608,7 @@ class HandleErrorTest(fixtures.TestBase): with engine.connect() as conn: assert_raises_message( tsa.exc.StatementError, - r"nope \(original cause: Exception: nope\) u?'SELECT 1 ", + r"\(exceptions.Exception\) nope \[SQL\: u?'SELECT 1 ", conn.execute, select([1]).where( column('foo') == literal('bar', MyType())) @@ -1799,7 +1799,7 @@ class HandleErrorTest(fixtures.TestBase): with engine.connect() as conn: assert_raises_message( tsa.exc.StatementError, - r"nope \(original cause: Exception: nope\) u?'SELECT 1 ", + r"\(exceptions.Exception\) nope \[SQL\: u?'SELECT 1 ", conn.execute, select([1]).where( column('foo') == literal('bar', MyType())) diff --git a/test/engine/test_logging.py b/test/engine/test_logging.py index 1432a0f7bf..180ea93888 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -56,7 +56,8 @@ class LogParamsTest(fixtures.TestBase): def test_error_large_dict(self): assert_raises_message( tsa.exc.DBAPIError, - r".*'INSERT INTO nonexistent \(data\) values \(:data\)' " + r".*'INSERT INTO nonexistent \(data\) values \(:data\)'\] " + "\[parameters: " "\[{'data': '0'}, {'data': '1'}, {'data': '2'}, " "{'data': '3'}, {'data': '4'}, {'data': '5'}, " "{'data': '6'}, {'data': '7'} ... displaying 10 of " @@ -71,8 +72,9 @@ class LogParamsTest(fixtures.TestBase): assert_raises_message( tsa.exc.DBAPIError, r".*INSERT INTO nonexistent \(data\) values " - "\(\?\)' \[\('0',\), \('1',\), \('2',\), \('3',\), " - "\('4',\), \('5',\), \('6',\), \('7',\) ... displaying " + "\(\?\)'\] \[parameters: \[\('0',\), \('1',\), \('2',\), \('3',\), " + "\('4',\), \('5',\), \('6',\), \('7',\) " + "... displaying " "10 of 100 total bound parameter sets ... " "\('98',\), \('99',\)\]", lambda: self.eng.execute( diff --git a/test/sql/test_query.py b/test/sql/test_query.py index fc040dfed6..2f13486eb8 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -81,11 +81,10 @@ class QueryTest(fixtures.TestBase): assert_raises_message( exc.StatementError, - r"A value is required for bind parameter 'user_name', in " + r"\(sqlalchemy.exc.InvalidRequestError\) A value is required for " + "bind parameter 'user_name', in " "parameter group 2 " - "\(original cause: (sqlalchemy.exc.)?InvalidRequestError: A " - "value is required for bind parameter 'user_name', in " - "parameter group 2\) u?'INSERT INTO query_users", + r"\[SQL: u?'INSERT INTO query_users", users.insert().execute, {'user_id': 7, 'user_name': 'jack'}, {'user_id': 8, 'user_name': 'ed'}, -- 2.47.2