Return correct error from Rows::next

[haaawk]

Revert the change introduced by 77c8b71. It caused a wrong error being returned.
For example column index out of range was returned when database is locked was expected.

[haaawk]

@aqrln I had to revert part of your #296 change.
That makes me wonder if the change to Statement::execute is correct

[haaawk]

Same with prepare. What was the the motivation for #296? Is there a particular example that led to it?

[aqrln]

@haaawk The motivation was to fix the failing tests in http://github.com/prisma/prisma-engines when compiling with libSQL instead of SQLite+rusqlite, and make the errors work similar to rusqlite. We match on extended result codes and parse the error messages, so not exposing that information would be a problem and will break our error handling. It will also make the error messages quite cryptic for libSQL users in general.

As for extended codes, it's possible to use https://www.sqlite.org/c3ref/extended_result_codes.html once after connecting to get them directly in the result code, but as for the error messages, I'm afraid there's no other way to get them except how it was done in #296.

errors::error_from_code (i.e., sqlite3_errstr) here returns a very generic textual description of the error code, not the actual error message, the latter is what errors::error_from_handle (i.e., sqlite3_errmsg) does.

To illustrate the difference using a random test from our codebase:

• errors::error_from_code returns constraint failed
• errors::error_from_handle returns UNIQUE constraint failed: Fruit.name

As for why it's possible for the errors to get mixed up in the Go tests, is there any chance they are running concurrently without synchronization? The Rust API contract for libsql::Connection currently forbids using it concurrently from multiple threads without putting it behind a mutex, it's a compiler error, so this problem does not occur, but since this restriction is not be enforced anymore in C bindings, I can imagine running into issues because of it.

For what it's worth, SQLite documentation suggests wrapping the operation that may error and reading the error itself with sqlite3_mutex_enter(sqlite3_db_mutex(db)) and sqlite3_mutex_leave(sqlite3_db_mutex(db)) when using the same connection from multiple threads to avoid the errors from being mixed up: https://www.sqlite.org/c3ref/errcode.html

cc @penberg @LucioFranco

[aqrln]

Looking at the code though, I'm not sure if concurrency is the problem here, and the expected error message doesn't look right to me:

error code = 1: Error fetching next row: Failed to fetch row: `database is locked`

The error code and description mismatch here, errors::error_from_code(1) would return SQL logic error, database is locked is the description of error code 5.

[haaawk]

I can clearly see how extended error messages are better. The question is why are they returning a wrong description. The failing test has absolutely nothing to do with column index out of range and is actually locking the db so database is locked is the expected result.

[haaawk]

We should try the mutex thing and see if it helps