From 1bbcec147f0d692cd4022cecaef93d3c3231b034 Mon Sep 17 00:00:00 2001 From: Nate Clark Date: Sat, 16 Feb 2019 19:17:56 -0600 Subject: [PATCH] Change StatementError formatting (newlines and %s) This changes the error formatting for StatementError in two ways: 1) Break each error detail up over multiple newlines instead of spaced out on a single line. Hopefully, this helps readers scan the error message more easily. 2) Change the SQL representation in the message to use __str__ (%s) instead of the current behavior that uses __repr__ (%r). This should help readers recognize the structure of their SQL, particularly if it is a multiline SQL statement. In the multiline case, the SQL would be printed over multiple lines instead of printing an escaped "\n". --- lib/sqlalchemy/exc.py | 4 +- test/base/test_except.py | 63 +++++++++++++++++--------------- test/engine/test_deprecations.py | 2 +- test/engine/test_execute.py | 6 +-- test/engine/test_logging.py | 10 ++--- test/orm/test_session.py | 2 +- test/sql/test_insert_exec.py | 4 +- 7 files changed, 48 insertions(+), 43 deletions(-) diff --git a/lib/sqlalchemy/exc.py b/lib/sqlalchemy/exc.py index b9e2f93396..1475bf35f0 100644 --- a/lib/sqlalchemy/exc.py +++ b/lib/sqlalchemy/exc.py @@ -346,14 +346,14 @@ class StatementError(SQLAlchemyError): details = [self._message(as_unicode=as_unicode)] if self.statement: - details.append("[SQL: %r]" % self.statement) + details.append("[SQL: %s]" % self.statement) if self.params: params_repr = util._repr_params(self.params, 10) details.append("[parameters: %r]" % params_repr) code_str = self._code_str() if code_str: details.append(code_str) - return " ".join(["(%s)" % det for det in self.detail] + details) + return "\n".join(["(%s)" % det for det in self.detail] + details) class DBAPIError(StatementError): diff --git a/test/base/test_except.py b/test/base/test_except.py index 58943902ce..203ba33ede 100644 --- a/test/base/test_except.py +++ b/test/base/test_except.py @@ -70,9 +70,9 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] (Background on this error at: " - "http://sqlalche.me/e/e3q8)", + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n" + "(Background on this error at: http://sqlalche.me/e/e3q8)", ) def test_statement_error_no_code(self): @@ -86,8 +86,8 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.StatementError as err: eq_( str(err), - "(sqlalchemy.exc.InvalidRequestError) hello " - "[SQL: 'select * from table'] [parameters: [{'x': 1}]]", + "(sqlalchemy.exc.InvalidRequestError) hello\n" + "[SQL: select * from table]\n[parameters: [{'x': 1}]]", ) eq_(err.args, ("(sqlalchemy.exc.InvalidRequestError) hello",)) @@ -102,8 +102,9 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.StatementError as err: eq_( str(err), - "(sqlalchemy.exc.InvalidRequestError) hello " - "[SQL: 'select * from table'] [parameters: [{'x': 1}]] " + "(sqlalchemy.exc.InvalidRequestError) hello\n" + "[SQL: select * from table]\n" + "[parameters: [{'x': 1}]]\n" "(Background on this error at: http://sqlalche.me/e/abcd)", ) eq_(err.args, ("(sqlalchemy.exc.InvalidRequestError) hello",)) @@ -114,7 +115,7 @@ class WrapTest(fixtures.TestBase): orig.args = [2006, "Test raise operational error"] eq_( str(orig), - "(2006, 'Test raise operational error') " + "(2006, 'Test raise operational error')\n" "(Background on this error at: http://sqlalche.me/e/dbapi)", ) @@ -125,7 +126,7 @@ class WrapTest(fixtures.TestBase): eq_( compat.text_type(orig), compat.u( - "méil (Background on this error at: " + "méil\n(Background on this error at: " "http://sqlalche.me/e/dbapi)" ), ) @@ -153,8 +154,9 @@ class WrapTest(fixtures.TestBase): ) except sa_exceptions.DBAPIError as exc: assert str(exc).startswith( - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] [parameters: {" + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n" + "[parameters: {" ) def test_tostring_large_list(self): @@ -165,10 +167,10 @@ class WrapTest(fixtures.TestBase): OperationalError(), DatabaseError, ) - except sa_exceptions.DBAPIError as exc: - assert str(exc).startswith( - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] [parameters: " + except sa_exceptions.DBAPIError as ex: + assert str(ex).startswith( + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n[parameters: " "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]]" ) @@ -194,11 +196,11 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(test.base.test_except.OperationalError) sql error " - "[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}]] (Background on this error at: " - "http://sqlalche.me/e/e3q8)", + "(test.base.test_except.OperationalError) sql error\n" + "[SQL: this is a message]\n" + "[parameters: [{1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: 1}," + " {1: 1}, {1: 1}, {1: 1}, {1: 1}, {1: 1}]]\n" + "(Background on this error at: http://sqlalche.me/e/e3q8)", ) eq_( exc.args, @@ -226,11 +228,12 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] [parameters: [{1: 1}, " + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n" + "[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}]]\n" "(Background on this error at: http://sqlalche.me/e/e3q8)", ) try: @@ -244,9 +247,10 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] [parameters: [(1,), " - "(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,)]] " + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n" + "[parameters: [(1,), " + "(1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,), (1,)]]\n" "(Background on this error at: http://sqlalche.me/e/e3q8)", ) try: @@ -271,11 +275,12 @@ class WrapTest(fixtures.TestBase): except sa_exceptions.DBAPIError as exc: eq_( str(exc), - "(test.base.test_except.OperationalError) " - "[SQL: 'this is a message'] [parameters: [(1,), " + "(test.base.test_except.OperationalError) \n" + "[SQL: this is a message]\n" + "[parameters: [(1,), " "(1,), (1,), (1,), (1,), (1,), (1,), (1,) " "... displaying 10 of 11 total bound " - "parameter sets ... (1,), (1,)]] " + "parameter sets ... (1,), (1,)]]\n" "(Background on this error at: http://sqlalche.me/e/e3q8)", ) diff --git a/test/engine/test_deprecations.py b/test/engine/test_deprecations.py index 35226a0972..29bda0ac20 100644 --- a/test/engine/test_deprecations.py +++ b/test/engine/test_deprecations.py @@ -1114,7 +1114,7 @@ class HandleErrorTest(fixtures.TestBase): with engine.connect() as conn: assert_raises_message( tsa.exc.StatementError, - r"\(.*SomeException\) " r"nope \[SQL\: u?'SELECT 1 ", + r"\(.*SomeException\) " r"nope\n\[SQL\: u?SELECT 1 ", conn.execute, select([1]).where(column("foo") == literal("bar", MyType())), ) diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index ad9144a382..fc152417a2 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -371,7 +371,7 @@ class ExecuteTest(fixtures.TestBase): def _go(conn): assert_raises_message( tsa.exc.StatementError, - r"\(.*.SomeException\) " r"nope \[SQL\: u?'SELECT 1 ", + r"\(.*.SomeException\) " r"nope\n\[SQL\: u?SELECT 1 ", conn.execute, select([1]).where(column("foo") == literal("bar", MyType())), ) @@ -415,7 +415,7 @@ class ExecuteTest(fixtures.TestBase): ) if util.py2k else util.u( - "A value is required for bind parameter 'uname'" + "A value is required for bind parameter 'uname'\n" ".*SELECT users.user_name AS .méil." ), conn.execute, @@ -2184,7 +2184,7 @@ class HandleErrorTest(fixtures.TestBase): with engine.connect() as conn: assert_raises_message( tsa.exc.StatementError, - r"\(.*.SomeException\) " r"nope \[SQL\: u?'SELECT 1 ", + r"\(.*.SomeException\) " r"nope\n\[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 8190a3fcde..bd425c940d 100644 --- a/test/engine/test_logging.py +++ b/test/engine/test_logging.py @@ -103,7 +103,7 @@ class LogParamsTest(fixtures.TestBase): exception = tsa.exc.IntegrityError("foo", {"x": "y"}, None) eq_regex( str(exception), - r"\(.*.NoneType\) None \[SQL: 'foo'\] \[parameters: {'x': 'y'}\]", + r"\(.*.NoneType\) None\n\[SQL: foo\]\n\[parameters: {'x': 'y'}\]", ) def test_exception_format_unexpected_parameter(self): @@ -112,7 +112,7 @@ class LogParamsTest(fixtures.TestBase): exception = tsa.exc.IntegrityError("foo", "bar", "bat") eq_regex( str(exception), - r"\(.*.str\) bat \[SQL: 'foo'\] \[parameters: 'bar'\]", + r"\(.*.str\) bat\n\[SQL: foo\]\n\[parameters: 'bar'\]", ) def test_exception_format_unexpected_member_parameter(self): @@ -121,7 +121,7 @@ class LogParamsTest(fixtures.TestBase): exception = tsa.exc.IntegrityError("foo", ["bar", "bat"], "hoho") eq_regex( str(exception), - r"\(.*.str\) hoho \[SQL: 'foo'\] \[parameters: \['bar', 'bat'\]\]", + r"\(.*.str\) hoho\n\[SQL: foo\]\n\[parameters: \['bar', 'bat'\]\]", ) def test_result_large_param(self): @@ -169,7 +169,7 @@ 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\)\]\n" r"\[parameters: " r"\[{'data': '0'}, {'data': '1'}, {'data': '2'}, " r"{'data': '3'}, {'data': '4'}, {'data': '5'}, " @@ -186,7 +186,7 @@ class LogParamsTest(fixtures.TestBase): assert_raises_message( tsa.exc.DBAPIError, r".*INSERT INTO nonexistent \(data\) values " - r"\(\?\)'\] \[parameters: \[\('0',\), \('1',\), \('2',\), " + r"\(\?\)\]\n\[parameters: \[\('0',\), \('1',\), \('2',\), " r"\('3',\), \('4',\), \('5',\), \('6',\), \('7',\) " r"... displaying " r"10 of 100 total bound parameter sets ... " diff --git a/test/orm/test_session.py b/test/orm/test_session.py index 2bc11398dc..ad7ff2d35a 100644 --- a/test/orm/test_session.py +++ b/test/orm/test_session.py @@ -1037,7 +1037,7 @@ class DeferredRelationshipExpressionTest(_fixtures.FixtureTest): assert_raises_message( sa.exc.StatementError, "Can't resolve value for column users.id on object " - ".User.*.; the object is detached and the value was expired ", + ".User.*.; the object is detached and the value was expired", q.one, ) diff --git a/test/sql/test_insert_exec.py b/test/sql/test_insert_exec.py index fafe7cc9ba..7905dc4bc9 100644 --- a/test/sql/test_insert_exec.py +++ b/test/sql/test_insert_exec.py @@ -60,8 +60,8 @@ class InsertExecTest(fixtures.TablesTest): exc.StatementError, r"\(sqlalchemy.exc.InvalidRequestError\) A value is required for " "bind parameter 'user_name', in " - "parameter group 2 " - r"\[SQL: u?'INSERT INTO users", + "parameter group 2\n" + r"\[SQL: u?INSERT INTO users", users.insert().execute, {"user_id": 7, "user_name": "jack"}, {"user_id": 8, "user_name": "ed"}, -- 2.47.3