Provide the SQL statement context (#554)

* Provide the SQL statement context for errors that occur when preparing a SQL statement * Extract status2class function * Use status2class in CHECK_MSG / rb_sqlite3_raise_msg * Prefer rb_sprintf to asprintf+strnew2+free * Add two failing tests for SQL with newlines * Move sql error message formatting into Ruby to handle newlines and handle versions of sqlite3 without sqlite3_error_offset() * doc: update CHANGELOG --------- Co-authored-by: Mike Dalessio <[email protected]>
sparklemotion · Oct 23, 2024 · 2d0139d · 2d0139d
1 parent 557ce1b
commit 2d0139d
Show file tree

Hide file tree

Showing 7 changed files with 174 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # sqlite3-ruby Changelog
 
+## next / unreleased
+
+### Improved
+
+- SQL Syntax errors during `Database#prepare` will raise a verbose exception with a multiline message indicating with a "^" exactly where in the statement the error occurred. [#554] @fractaledmind @flavorjones
+
+
 ## 2.1.1 / 2024-10-22
 
 ### Dependencies

diff --git a/ext/sqlite3/exception.c b/ext/sqlite3/exception.c
@@ -1,101 +1,82 @@
 #include <sqlite3_ruby.h>
 
-void
-rb_sqlite3_raise(sqlite3 *db, int status)
+static VALUE
+status2klass(int status)
 {
-    VALUE klass = Qnil;
-
     /* Consider only lower 8 bits, to work correctly when
        extended result codes are enabled. */
     switch (status & 0xff) {
         case SQLITE_OK:
-            return;
-            break;
+            return Qnil;
         case SQLITE_ERROR:
-            klass = rb_path2class("SQLite3::SQLException");
-            break;
+            return rb_path2class("SQLite3::SQLException");
         case SQLITE_INTERNAL:
-            klass = rb_path2class("SQLite3::InternalException");
-            break;
+            return rb_path2class("SQLite3::InternalException");
         case SQLITE_PERM:
-            klass = rb_path2class("SQLite3::PermissionException");
-            break;
+            return rb_path2class("SQLite3::PermissionException");
         case SQLITE_ABORT:
-            klass = rb_path2class("SQLite3::AbortException");
-            break;
+            return rb_path2class("SQLite3::AbortException");
         case SQLITE_BUSY:
-            klass = rb_path2class("SQLite3::BusyException");
-            break;
+            return rb_path2class("SQLite3::BusyException");
         case SQLITE_LOCKED:
-            klass = rb_path2class("SQLite3::LockedException");
-            break;
+            return rb_path2class("SQLite3::LockedException");
         case SQLITE_NOMEM:
-            klass = rb_path2class("SQLite3::MemoryException");
-            break;
+            return rb_path2class("SQLite3::MemoryException");
         case SQLITE_READONLY:
-            klass = rb_path2class("SQLite3::ReadOnlyException");
-            break;
+            return rb_path2class("SQLite3::ReadOnlyException");
         case SQLITE_INTERRUPT:
-            klass = rb_path2class("SQLite3::InterruptException");
-            break;
+            return rb_path2class("SQLite3::InterruptException");
         case SQLITE_IOERR:
-            klass = rb_path2class("SQLite3::IOException");
-            break;
+            return rb_path2class("SQLite3::IOException");
         case SQLITE_CORRUPT:
-            klass = rb_path2class("SQLite3::CorruptException");
-            break;
+            return rb_path2class("SQLite3::CorruptException");
         case SQLITE_NOTFOUND:
-            klass = rb_path2class("SQLite3::NotFoundException");
-            break;
+            return rb_path2class("SQLite3::NotFoundException");
         case SQLITE_FULL:
-            klass = rb_path2class("SQLite3::FullException");
-            break;
+            return rb_path2class("SQLite3::FullException");
         case SQLITE_CANTOPEN:
-            klass = rb_path2class("SQLite3::CantOpenException");
-            break;
+            return rb_path2class("SQLite3::CantOpenException");
         case SQLITE_PROTOCOL:
-            klass = rb_path2class("SQLite3::ProtocolException");
-            break;
+            return rb_path2class("SQLite3::ProtocolException");
         case SQLITE_EMPTY:
-            klass = rb_path2class("SQLite3::EmptyException");
-            break;
+            return rb_path2class("SQLite3::EmptyException");
         case SQLITE_SCHEMA:
-            klass = rb_path2class("SQLite3::SchemaChangedException");
-            break;
+            return rb_path2class("SQLite3::SchemaChangedException");
         case SQLITE_TOOBIG:
-            klass = rb_path2class("SQLite3::TooBigException");
-            break;
+            return rb_path2class("SQLite3::TooBigException");
         case SQLITE_CONSTRAINT:
-            klass = rb_path2class("SQLite3::ConstraintException");
-            break;
+            return rb_path2class("SQLite3::ConstraintException");
         case SQLITE_MISMATCH:
-            klass = rb_path2class("SQLite3::MismatchException");
-            break;
+            return rb_path2class("SQLite3::MismatchException");
         case SQLITE_MISUSE:
-            klass = rb_path2class("SQLite3::MisuseException");
-            break;
+            return rb_path2class("SQLite3::MisuseException");
         case SQLITE_NOLFS:
-            klass = rb_path2class("SQLite3::UnsupportedException");
-            break;
+            return rb_path2class("SQLite3::UnsupportedException");
         case SQLITE_AUTH:
-            klass = rb_path2class("SQLite3::AuthorizationException");
-            break;
+            return rb_path2class("SQLite3::AuthorizationException");
         case SQLITE_FORMAT:
-            klass = rb_path2class("SQLite3::FormatException");
-            break;
+            return rb_path2class("SQLite3::FormatException");
         case SQLITE_RANGE:
-            klass = rb_path2class("SQLite3::RangeException");
-            break;
+            return rb_path2class("SQLite3::RangeException");
         case SQLITE_NOTADB:
-            klass = rb_path2class("SQLite3::NotADatabaseException");
-            break;
+            return rb_path2class("SQLite3::NotADatabaseException");
         default:
-            klass = rb_path2class("SQLite3::Exception");
+            return rb_path2class("SQLite3::Exception");
+    }
+}
+
+void
+rb_sqlite3_raise(sqlite3 *db, int status)
+{
+    VALUE klass = status2klass(status);
+    if (NIL_P(klass)) {
+        return;
     }
 
-    klass = rb_exc_new2(klass, sqlite3_errmsg(db));
-    rb_iv_set(klass, "@code", INT2FIX(status));
-    rb_exc_raise(klass);
+    VALUE exception = rb_exc_new2(klass, sqlite3_errmsg(db));
+    rb_iv_set(exception, "@code", INT2FIX(status));
+
+    rb_exc_raise(exception);
 }
 
 /*
@@ -104,14 +85,38 @@ rb_sqlite3_raise(sqlite3 *db, int status)
 void
 rb_sqlite3_raise_msg(sqlite3 *db, int status, const char *msg)
 {
-    VALUE exception;
-
-    if (status == SQLITE_OK) {
+    VALUE klass = status2klass(status);
+    if (NIL_P(klass)) {
         return;
     }
 
-    exception = rb_exc_new2(rb_path2class("SQLite3::Exception"), msg);
+    VALUE exception = rb_exc_new2(klass, msg);
+    rb_iv_set(exception, "@code", INT2FIX(status));
     sqlite3_free((void *)msg);
+
+    rb_exc_raise(exception);
+}
+
+void
+rb_sqlite3_raise_with_sql(sqlite3 *db, int status, const char *sql)
+{
+    VALUE klass = status2klass(status);
+    if (NIL_P(klass)) {
+        return;
+    }
+
+    const char *error_msg = sqlite3_errmsg(db);
+    int error_offset = -1;
+#ifdef HAVE_SQLITE3_ERROR_OFFSET
+    error_offset = sqlite3_error_offset(db);
+#endif
+
+    VALUE exception = rb_exc_new2(klass, error_msg);
     rb_iv_set(exception, "@code", INT2FIX(status));
+    if (sql) {
+        rb_iv_set(exception, "@sql", rb_str_new2(sql));
+        rb_iv_set(exception, "@sql_offset", INT2FIX(error_offset));
+    }
+
     rb_exc_raise(exception);
 }
diff --git a/ext/sqlite3/exception.h b/ext/sqlite3/exception.h
@@ -3,8 +3,10 @@
 
 #define CHECK(_db, _status) rb_sqlite3_raise(_db, _status);
 #define CHECK_MSG(_db, _status, _msg) rb_sqlite3_raise_msg(_db, _status, _msg);
+#define CHECK_PREPARE(_db, _status, _sql) rb_sqlite3_raise_with_sql(_db, _status, _sql)
 
 void rb_sqlite3_raise(sqlite3 *db, int status);
 void rb_sqlite3_raise_msg(sqlite3 *db, int status, const char *msg);
+void rb_sqlite3_raise_with_sql(sqlite3 *db, int status, const char *sql);
 
 #endif
diff --git a/ext/sqlite3/extconf.rb b/ext/sqlite3/extconf.rb
@@ -132,6 +132,7 @@ def configure_extension
 
         have_func("sqlite3_prepare_v2")
         have_func("sqlite3_db_name", "sqlite3.h") # v3.39.0
+        have_func("sqlite3_error_offset", "sqlite3.h") # v3.38.0
 
         have_type("sqlite3_int64", "sqlite3.h")
         have_type("sqlite3_uint64", "sqlite3.h")

diff --git a/ext/sqlite3/statement.c b/ext/sqlite3/statement.c
@@ -78,7 +78,7 @@ prepare(VALUE self, VALUE db, VALUE sql)
                  &tail
              );
 
-    CHECK(db_ctx->db, status);
+    CHECK_PREPARE(db_ctx->db, status, StringValuePtr(sql));
     timespecclear(&db_ctx->stmt_deadline);
 
     return rb_utf8_str_new_cstr(tail);

diff --git a/lib/sqlite3/errors.rb b/lib/sqlite3/errors.rb
@@ -4,6 +4,34 @@ module SQLite3
   class Exception < ::StandardError
     # A convenience for accessing the error code for this exception.
     attr_reader :code
+
+    # If the error is associated with a SQL query, this is the query
+    attr_reader :sql
+
+    # If the error is associated with a particular offset in a SQL query, this is the non-negative
+    # offset. If the offset is not available, this will be -1.
+    attr_reader :sql_offset
+
+    def message
+      [super, sql_error].compact.join(":\n")
+    end
+
+    private def sql_error
+      return nil unless @sql
+      return @sql.chomp unless @sql_offset >= 0
+
+      offset = @sql_offset
+      sql.lines.flat_map do |line|
+        if offset >= 0 && line.length > offset
+          blanks = " " * offset
+          offset = -1
+          [line.chomp, blanks + "^"]
+        else
+          offset -= line.length if offset
+          line.chomp
+        end
+      end.join("\n")
+    end
   end
 
   class SQLException < Exception; end

diff --git a/test/test_integration.rb b/test/test_integration.rb
@@ -107,6 +107,70 @@ def test_prepare_invalid_syntax
     end
   end
 
+  def test_prepare_exception_shows_error_position
+    exception = assert_raise(SQLite3::SQLException) do
+      @db.prepare "select from foo"
+    end
+    if exception.sql_offset >= 0 # HAVE_SQLITE_ERROR_OFFSET
+      assert_equal(<<~MSG.chomp, exception.message)
+        near "from": syntax error:
+        select from foo
+               ^
+      MSG
+    else
+      assert_equal(<<~MSG.chomp, exception.message)
+        near "from": syntax error:
+        select from foo
+      MSG
+    end
+  end
+
+  def test_prepare_exception_shows_error_position_newline1
+    exception = assert_raise(SQLite3::SQLException) do
+      @db.prepare(<<~SQL)
+        select
+        from foo
+      SQL
+    end
+    if exception.sql_offset >= 0 # HAVE_SQLITE_ERROR_OFFSET
+      assert_equal(<<~MSG.chomp, exception.message)
+        near "from": syntax error:
+        select
+        from foo
+        ^
+      MSG
+    else
+      assert_equal(<<~MSG.chomp, exception.message)
+        near "from": syntax error:
+        select
+        from foo
+      MSG
+    end
+  end
+
+  def test_prepare_exception_shows_error_position_newline2
+    exception = assert_raise(SQLite3::SQLException) do
+      @db.prepare(<<~SQL)
+        select asdf
+        from foo
+      SQL
+    end
+    if exception.sql_offset >= 0 # HAVE_SQLITE_ERROR_OFFSET
+      assert_equal(<<~MSG.chomp, exception.message)
+        no such column: asdf:
+        select asdf
+               ^
+        from foo
+      MSG
+    else
+      assert_equal(<<~MSG.chomp, exception.message)
+        no such column: asdf:
+        select asdf
+        from foo
+      MSG
+    end
+  end
+
   def test_prepare_invalid_column
     assert_raise(SQLite3::SQLException) do
       @db.prepare "select k from foo"