diff --git a/CHANGELOG.md b/CHANGELOG.md index b80ca567f..09f7c5816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,12 @@ # CHANGELOG ## 0.7.5dev -* [Feature] Using native DuckDB `.df()` method when using `autopandas` - +* [Feature] Using native DuckDB `.df()` method when using `autopandas` * [Doc] documenting `%sqlcmd tables`/`%sqlcmd columns` * [Feature] Better error messages when function used in plotting API unsupported by DB driver (#159) * [Fix] Fix the default value of %config SqlMagic.displaylimit to 10 (#462) +* [Feature] Detailed error messages when syntax error in SQL query, postgres connection password missing or inaccessible, invalid DuckDB connection string (#229) + ## 0.7.4 (2023-04-28) No changes @@ -299,4 +300,4 @@ Converted from an IPython Plugin to an Extension for 1.0 compatibility *Release date: 21-Mar-2013* -* Initial release +* Initial release \ No newline at end of file diff --git a/doc/community/developer-guide.md b/doc/community/developer-guide.md index acf37803e..88b1a5130 100644 --- a/doc/community/developer-guide.md +++ b/doc/community/developer-guide.md @@ -61,6 +61,16 @@ The internal implementation of `sql.exceptions` is a workaround due to some IPyt +++ +### Handling common errors + +Currently, these common errors are handled by providing more meaningful error messages: + +* Syntax error in SQL query - The SQL query is parsed using `sqlglot` and possible syntax issues or query suggestions are provided to the user. This raises a `UsageError` with the message. +* Missing password when connecting to PostgreSQL - Displays guide on DB connections +* Invalid connection strings when connecting to DuckDB. + ++++ + ## Unit testing ### Running tests diff --git a/src/sql/__init__.py b/src/sql/__init__.py index 0377bd8d8..104190f47 100644 --- a/src/sql/__init__.py +++ b/src/sql/__init__.py @@ -1,4 +1,6 @@ from .magic import RenderMagic, SqlMagic, load_ipython_extension +from .error_message import SYNTAX_ERROR +from .connection import PLOOMBER_DOCS_LINK_STR __version__ = "0.7.5dev" @@ -7,4 +9,6 @@ "RenderMagic", "SqlMagic", "load_ipython_extension", + "SYNTAX_ERROR", + "PLOOMBER_DOCS_LINK_STR", ] diff --git a/src/sql/connection.py b/src/sql/connection.py index 70a7b5b3b..486e1a48a 100644 --- a/src/sql/connection.py +++ b/src/sql/connection.py @@ -3,7 +3,7 @@ import sqlalchemy from sqlalchemy.engine import Engine -from sqlalchemy.exc import NoSuchModuleError +from sqlalchemy.exc import NoSuchModuleError, OperationalError from IPython.core.error import UsageError import difflib import sqlglot @@ -11,10 +11,11 @@ from sql.store import store from sql.telemetry import telemetry from sql import exceptions +from sql.error_message import detail +from ploomber_core.exceptions import modify_exceptions -PLOOMBER_SUPPORT_LINK_STR = ( - "For technical support: https://ploomber.io/community" - "\nDocumentation: https://jupysql.ploomber.io/en/latest/connecting.html" +PLOOMBER_DOCS_LINK_STR = ( + "Documentation: https://jupysql.ploomber.io/en/latest/connecting.html" ) IS_SQLALCHEMY_ONE = int(sqlalchemy.__version__.split(".")[0]) == 1 @@ -129,19 +130,19 @@ def __init__(self, engine, alias=None): self.name = self.assign_name(engine) self.dialect = self.url.get_dialect() self.engine = engine - self.session = engine.connect() if IS_SQLALCHEMY_ONE: self.metadata = sqlalchemy.MetaData(bind=engine) - self.connections[ - alias - or ( - repr(sqlalchemy.MetaData(bind=engine).bind.url) - if IS_SQLALCHEMY_ONE - else repr(engine.url) - ) - ] = self + url = ( + repr(sqlalchemy.MetaData(bind=engine).bind.url) + if IS_SQLALCHEMY_ONE + else repr(engine.url) + ) + + self.session = self._create_session(engine, url) + + self.connections[alias or url] = self self.connect_args = None self.alias = alias @@ -157,6 +158,21 @@ def _suggest_fix_no_module_found(module_name): options = [f"{prefix}{suffix}", suggest_str] return "\n\n".join(options) + @classmethod + @modify_exceptions + def _create_session(cls, engine, connect_str): + try: + session = engine.connect() + return session + except OperationalError as e: + detailed_msg = detail(e) + if detailed_msg is not None: + raise exceptions.UsageError(detailed_msg) + else: + print(e) + except Exception as e: + raise cls._error_invalid_connection_info(e, connect_str) from e + @classmethod def _suggest_fix(cls, env_var, connect_str=None): """ @@ -203,21 +219,25 @@ def _suggest_fix(cls, env_var, connect_str=None): if len(options) >= 3: options.insert(-1, "OR") - options.append(PLOOMBER_SUPPORT_LINK_STR) + options.append(PLOOMBER_DOCS_LINK_STR) return "\n\n".join(options) @classmethod def _error_no_connection(cls): """Error when there isn't any connection""" - return UsageError("No active connection." + cls._suggest_fix(env_var=True)) + err = UsageError("No active connection." + cls._suggest_fix(env_var=True)) + err.modify_exception = True + return err @classmethod def _error_invalid_connection_info(cls, e, connect_str): - return UsageError( + err = UsageError( "An error happened while creating the connection: " f"{e}.{cls._suggest_fix(env_var=False, connect_str=connect_str)}" ) + err.modify_exception = True + return err @classmethod def from_connect_str( @@ -245,7 +265,7 @@ def from_connect_str( [ str(e), suggestion_str, - PLOOMBER_SUPPORT_LINK_STR, + PLOOMBER_DOCS_LINK_STR, ] ) ) from e diff --git a/src/sql/error_message.py b/src/sql/error_message.py new file mode 100644 index 000000000..b07c2cb79 --- /dev/null +++ b/src/sql/error_message.py @@ -0,0 +1,53 @@ +import sqlglot +import sqlparse + +SYNTAX_ERROR = "\nLooks like there is a syntax error in your query." +ORIGINAL_ERROR = "\nOriginal error message from DB driver:\n" + + +def detail(original_error, query=None): + original_error = str(original_error) + return_msg = SYNTAX_ERROR + if "syntax error" in original_error: + query_list = sqlparse.split(query) + for q in query_list: + try: + q = q.strip() + q = q[:-1] if q.endswith(";") else q + parse = sqlglot.transpile(q) + suggestions = "" + if q.upper() not in [suggestion.upper() for suggestion in parse]: + suggestions += f"Did you mean : {parse}\n" + return_msg = ( + return_msg + "Possible reason: \n" + suggestions + if suggestions + else return_msg + ) + + except sqlglot.errors.ParseError as e: + err = e.errors + position = "" + for item in err: + position += ( + f"Syntax Error in {q}: {item['description']} at " + f"Line {item['line']}, Column {item['col']}\n" + ) + return_msg = ( + return_msg + "Possible reason: \n" + position + if position + else return_msg + ) + + return return_msg + "\n" + ORIGINAL_ERROR + original_error + "\n" + + if "fe_sendauth: no password supplied" in original_error: + return ( + "\nLooks like you have run into some issues. " + "Review our DB connection via URL strings guide: " + "https://jupysql.ploomber.io/en/latest/connecting.html ." + " Using Ubuntu? Check out this guide: " + "https://help.ubuntu.com/community/PostgreSQL#fe_sendauth:_" + "no_password_supplied\n" + ORIGINAL_ERROR + original_error + "\n" + ) + + return None diff --git a/src/sql/magic.py b/src/sql/magic.py index 3a072098c..510a94b78 100644 --- a/src/sql/magic.py +++ b/src/sql/magic.py @@ -29,6 +29,7 @@ from sql._patch import patch_ipython_usage_error from ploomber_core.dependencies import check_installed +from sql.error_message import detail from traitlets.config.configurable import Configurable from traitlets import Bool, Int, TraitError, Unicode, Dict, observe, validate @@ -281,6 +282,7 @@ def execute(self, line="", cell="", local_ns=None): ) @telemetry.log_call("execute", payload=True) + @modify_exceptions def _execute(self, payload, line, cell, local_ns, is_interactive_mode=False): def interactive_execute_wrapper(**kwargs): for key, value in kwargs.items(): @@ -429,11 +431,18 @@ def interactive_execute_wrapper(**kwargs): # JA: added DatabaseError for MySQL except (ProgrammingError, OperationalError, DatabaseError) as e: # Sqlite apparently return all errors as OperationalError :/ - + detailed_msg = detail(e, command.sql) if self.short_errors: - print(e) + if detailed_msg is not None: + err = exceptions.UsageError(detailed_msg) + raise err + else: + print(e) else: - raise + if detailed_msg is not None: + print(detailed_msg) + e.modify_exception = True + raise e legal_sql_identifier = re.compile(r"^[A-Za-z0-9#_$]+") diff --git a/src/tests/conftest.py b/src/tests/conftest.py index 09def6113..3225b6a65 100644 --- a/src/tests/conftest.py +++ b/src/tests/conftest.py @@ -69,7 +69,7 @@ def ip(ip_empty): "CREATE TABLE test (n INT, name TEXT)", "INSERT INTO test VALUES (1, 'foo')", "INSERT INTO test VALUES (2, 'bar')", - 'CREATE TABLE "table with spaces" (first INT, second TEXT)', + "CREATE TABLE [table with spaces] (first INT, second TEXT)", "CREATE TABLE author (first_name, last_name, year_of_death)", "INSERT INTO author VALUES ('William', 'Shakespeare', 1616)", "INSERT INTO author VALUES ('Bertold', 'Brecht', 1956)", diff --git a/src/tests/integration/conftest.py b/src/tests/integration/conftest.py index 19f233445..70e154052 100644 --- a/src/tests/integration/conftest.py +++ b/src/tests/integration/conftest.py @@ -131,6 +131,15 @@ def ip_with_postgreSQL(ip_empty, setup_postgreSQL): ip_empty.run_cell("%sql -x " + alias) +@pytest.fixture +def postgreSQL_config_incorrect_pwd(ip_empty, setup_postgreSQL): + configKey = "postgreSQL" + alias = _testing.DatabaseConfigHelper.get_database_config(configKey)["alias"] + url = _testing.DatabaseConfigHelper.get_database_url(configKey) + url = url.replace(":ploomber_app_password", "") + return alias, url + + @pytest.fixture(scope="session") def setup_mySQL(test_table_name_dict, skip_on_live_mode): with _testing.mysql(): diff --git a/src/tests/integration/test_postgreSQL.py b/src/tests/integration/test_postgreSQL.py index 70917c32c..b00ecb417 100644 --- a/src/tests/integration/test_postgreSQL.py +++ b/src/tests/integration/test_postgreSQL.py @@ -17,3 +17,16 @@ def test_auto_commit_mode_on(ip_with_postgreSQL, capsys): assert out_after_creating.error_in_exec is None assert any(row[0] == "new_db" for row in out_all_dbs) assert "CREATE DATABASE cannot run inside a transaction block" not in out + + +def test_postgres_error(ip_empty, postgreSQL_config_incorrect_pwd): + alias, url = postgreSQL_config_incorrect_pwd + + # Select database engine + out = ip_empty.run_cell("%sql " + url + " --alias " + alias) + assert "Review our DB connection via URL strings guide" in str(out.error_in_exec) + assert "Original error message from DB driver" in str(out.error_in_exec) + assert ( + "If you need help solving this issue, " + "send us a message: https://ploomber.io/community" in str(out.error_in_exec) + ) diff --git a/src/tests/test_magic.py b/src/tests/test_magic.py index c1a21a070..f98f31562 100644 --- a/src/tests/test_magic.py +++ b/src/tests/test_magic.py @@ -19,6 +19,10 @@ from sql import magic from conftest import runsql +from sql.connection import PLOOMBER_DOCS_LINK_STR +from ploomber_core.exceptions import COMMUNITY + +COMMUNITY = COMMUNITY.strip() def test_memory_db(ip): @@ -722,7 +726,7 @@ def test_autolimit(ip): assert len(result) == 1 -invalid_connection_string = """ +invalid_connection_string = f""" No active connection. To fix it: @@ -734,8 +738,8 @@ def test_autolimit(ip): Set the environment variable $DATABASE_URL -For technical support: https://ploomber.io/community -Documentation: https://jupysql.ploomber.io/en/latest/connecting.html +{PLOOMBER_DOCS_LINK_STR} +{COMMUNITY} """ @@ -746,14 +750,14 @@ def test_error_on_invalid_connection_string(ip_empty, clean_conns): assert isinstance(result.error_in_exec, UsageError) -invalid_connection_string_format = """\ +invalid_connection_string_format = f"""\ Can't load plugin: sqlalchemy.dialects:something To fix it, make sure you are using correct driver name: Ref: https://docs.sqlalchemy.org/en/20/core/engines.html#database-urls -For technical support: https://ploomber.io/community -Documentation: https://jupysql.ploomber.io/en/latest/connecting.html +{PLOOMBER_DOCS_LINK_STR} +{COMMUNITY} """ # noqa @@ -764,32 +768,21 @@ def test_error_on_invalid_connection_string_format(ip_empty, clean_conns): assert isinstance(result.error_in_exec, UsageError) -invalid_connection_string_existing_conns = """ -Can't load plugin: sqlalchemy.dialects:something - -To fix it, make sure you are using correct driver name: -Ref: https://docs.sqlalchemy.org/en/20/core/engines.html#database-urls - -For technical support: https://ploomber.io/community -Documentation: https://jupysql.ploomber.io/en/latest/connecting.html -""" # noqa - - def test_error_on_invalid_connection_string_with_existing_conns(ip_empty, clean_conns): ip_empty.run_cell("%sql sqlite://") result = ip_empty.run_cell("%sql something://") - assert invalid_connection_string_existing_conns.strip() == str(result.error_in_exec) + assert invalid_connection_string_format.strip() == str(result.error_in_exec) assert isinstance(result.error_in_exec, UsageError) -invalid_connection_string_with_possible_typo = """ +invalid_connection_string_with_possible_typo = f""" Can't load plugin: sqlalchemy.dialects:sqlit Perhaps you meant to use driver the dialect: "sqlite" -For technical support: https://ploomber.io/community -Documentation: https://jupysql.ploomber.io/en/latest/connecting.html +{PLOOMBER_DOCS_LINK_STR} +{COMMUNITY} """ # noqa @@ -803,6 +796,29 @@ def test_error_on_invalid_connection_string_with_possible_typo(ip_empty, clean_c assert isinstance(result.error_in_exec, UsageError) +invalid_connection_string_duckdb = f""" +An error happened while creating the connection: connect(): incompatible function arguments. The following argument types are supported: + 1. (database: str = ':memory:', read_only: bool = False, config: dict = None) -> duckdb.DuckDBPyConnection + +Invoked with: kwargs: host='invalid_db'. + +To fix it: + +Pass a valid connection string: + Example: %sql postgresql://username:password@hostname/dbname + +{PLOOMBER_DOCS_LINK_STR} +{COMMUNITY} +""" # noqa + + +def test_error_on_invalid_connection_string_duckdb(ip_empty, clean_conns): + result = ip_empty.run_cell("%sql duckdb://invalid_db") + + assert invalid_connection_string_duckdb.strip() == str(result.error_in_exec) + assert isinstance(result.error_in_exec, UsageError) + + def test_jupysql_alias(): assert SqlMagic.magics == { "line": {"jupysql": "execute", "sql": "execute"}, @@ -916,8 +932,8 @@ def test_save_with_non_existing_table(ip, capsys): def test_save_with_bad_query_save(ip, capsys): ip.run_cell("%sql --save my_query SELECT * non_existing_table") ip.run_cell("%sql --with my_query SELECT * FROM my_query") - out, _ = capsys.readouterr() - assert '(sqlite3.OperationalError) near "non_existing_table": syntax error' in out + out, err = capsys.readouterr() + assert '(sqlite3.OperationalError) near "non_existing_table": syntax error' in err def test_interact_basic_data_types(ip, capsys): diff --git a/src/tests/test_syntax_errors.py b/src/tests/test_syntax_errors.py new file mode 100644 index 000000000..e919515f2 --- /dev/null +++ b/src/tests/test_syntax_errors.py @@ -0,0 +1,172 @@ +import pytest +import sqlalchemy.exc + +from sqlalchemy.exc import OperationalError +from IPython.core.error import UsageError + +from sql.error_message import SYNTAX_ERROR, ORIGINAL_ERROR +from ploomber_core.exceptions import COMMUNITY + + +COMMUNITY = COMMUNITY.strip() + + +@pytest.fixture +def query_no_suggestion(): + return ( + "sql", + "", + """ + sqlite:// + SELECT FROM author; + """, + ) + + +@pytest.fixture +def query_incorrect_column_name(): + return ( + "sql", + "", + """ + sqlite:// + SELECT first_(name FROM author; + """, + ) + + +@pytest.fixture +def query_suggestion(): + return ( + "sql", + "", + """ + sqlite:// + ALTER TABLE author RENAME new_author; + """, + ) + + +@pytest.fixture +def query_multiple(): + return ( + "sql", + "", + """ + sqlite:// + INSERT INTO author VALUES ('Charles', 'Dickens', 1812); + ALTER TABLE author RENAME another_name; + """, + ) + + +def _common_strings_check(err): + assert SYNTAX_ERROR.strip() in str(err.value) + assert ORIGINAL_ERROR.strip() in str(err.value) + assert COMMUNITY in str(err.value) + assert isinstance(err.value, UsageError) + + +def test_syntax_error_no_suggestion(ip, query_no_suggestion): + with pytest.raises(UsageError) as err: + ip.run_cell_magic(*query_no_suggestion) + _common_strings_check(err) + + +message_no_suggestion_long = f"""\ +(sqlite3.OperationalError) near "FROM": syntax error +{COMMUNITY} +[SQL: SELECT FROM author;] +""" # noqa + + +def test_syntax_error_no_suggestion_long(ip, capsys, query_no_suggestion): + ip.run_line_magic("config", "SqlMagic.short_errors = False") + with pytest.raises(OperationalError) as err: + ip.run_cell_magic(*query_no_suggestion) + out, _ = capsys.readouterr() + assert message_no_suggestion_long.strip() in str(err.value).strip() + assert SYNTAX_ERROR.strip() in out + assert isinstance(err.value, sqlalchemy.exc.OperationalError) + + +def test_syntax_error_incorrect_column_name(ip, query_incorrect_column_name): + with pytest.raises(UsageError) as err: + ip.run_cell_magic(*query_incorrect_column_name) + assert ( + "Syntax Error in SELECT first_(name FROM author: " + "Expecting ) at Line 1, Column 24" in str(err.value) + ) + _common_strings_check(err) + + +message_incorrect_column_name_long = f"""\ +(sqlite3.OperationalError) near "FROM": syntax error +{COMMUNITY} +[SQL: SELECT first_(name FROM author;] +""" # noqa + + +def test_syntax_error_incorrect_column_name_long( + ip, capsys, query_incorrect_column_name +): + ip.run_line_magic("config", "SqlMagic.short_errors = False") + with pytest.raises(OperationalError) as err: + ip.run_cell_magic(*query_incorrect_column_name) + out, _ = capsys.readouterr() + assert message_incorrect_column_name_long.strip() in str(err.value).strip() + assert SYNTAX_ERROR.strip() in out + assert isinstance(err.value, sqlalchemy.exc.OperationalError) + + +def test_syntax_error_suggestion(ip, query_suggestion): + with pytest.raises(UsageError) as err: + ip.run_cell_magic(*query_suggestion) + assert "Did you mean : ['ALTER TABLE author RENAME TO new_author']" in str( + err.value + ) + _common_strings_check(err) + + +message_error_suggestion_long = f"""\ +(sqlite3.OperationalError) near ";": syntax error +{COMMUNITY} +[SQL: ALTER TABLE author RENAME new_author;] +""" # noqa + + +def test_syntax_error_suggestion_long(ip, capsys, query_suggestion): + ip.run_line_magic("config", "SqlMagic.short_errors = False") + with pytest.raises(OperationalError) as err: + ip.run_cell_magic(*query_suggestion) + out, _ = capsys.readouterr() + assert message_error_suggestion_long.strip() in str(err.value).strip() + assert SYNTAX_ERROR.strip() in out + assert isinstance(err.value, sqlalchemy.exc.OperationalError) + + +def test_syntax_error_multiple_statements(ip, query_multiple): + with pytest.raises(UsageError) as err: + ip.run_cell_magic(*query_multiple) + assert ( + "Did you mean : ['ALTER TABLE author RENAME TO another_name']" + in str(err.value).strip() + ) + _common_strings_check(err) + + +message_multiple_statements_long = f"""\ +(sqlite3.OperationalError) near ";": syntax error +{COMMUNITY} +[SQL: ALTER TABLE author RENAME another_name;] +""" # noqa + + +def test_syntax_error_multiple_statements_long(ip, capsys, query_multiple): + ip.run_line_magic("config", "SqlMagic.short_errors = False") + with pytest.raises(OperationalError) as err: + ip.run_cell_magic(*query_multiple) + out, _ = capsys.readouterr() + assert message_multiple_statements_long.strip() in str(err.value).strip() + assert SYNTAX_ERROR.strip() in out + assert isinstance(err.value, sqlalchemy.exc.OperationalError)