nakagami / firebirdsql

Firebird RDBMS sql driver for Go (golang)
MIT License
227 stars 60 forks source link

error handling/messages Problem #39

Closed michael88 closed 7 years ago

michael88 commented 7 years ago

Hello, I tried the following (should return something like : Artithmetic Overflow or Division / 0)

package main import ( "database/sql" "fmt" "log" _ "github.com/nakagami/firebirdsql" ) func main() { var n int conn, err := sql.Open("firebirdsql", "sysdba:xxxxx@192.168.1.3/wetten") if err != nil { log.Fatal(err) } defer conn.Close() err = conn.QueryRow("SELECT 5 / 0 FROM rdb$database").Scan(&n) if err != nil { log.Fatal(err) } fmt.Println("Result=", n) }

And I get: 2017/06/15 23:05:51 sql: no rows in result set This is correct, but why I'm not getting the real error message?

On Server Side (log from firebirdsql) I get Thu Jun 15 23:05:09 2017 INET/Inet_error: read errno = 104

if I change the query to err = conn.QueryRow("SELECT 5 / 2 FROM rdb$database").Scan(&n) (this produces no error, of course) anything is ok and on server side there is not INET/Inet_error anymore

How can I get (possible) runtime Errors?

If i change from QueryRow to Query and loop over row.next then I never get an error If i use Transactions then on commit (or rollback) I get opcode=0

What I'm doing wrong?

michael88 commented 7 years ago

Oh, I forgot: Go Version is 1.8.3 and I load your firebird driver with go get at 15.06.2017 pm

nakagami commented 7 years ago

It means 'socket reset by peer'. I think it is not start connect negotiation. Can you connect with isql or another language client

michael88 commented 7 years ago

Yes, I can connect with isql. I can also connect with your client.

if the query contains no runtime errors there is no Problem. if the query contains a syntax error: there is no Problem and I get a correct error message. But, If the query fails at runtime (like my example with 5 / 0) then it fails

My Firebird version is 2.5

michael88 commented 7 years ago

I think my Problem relates to https://github.com/nakagami/firebirdsql/issues/38 because I also get the "opcode 0" if I try this with transactions and I try to commit or rollback.

michael88 commented 7 years ago

I don't know how to request a diff/pull But can you try/test this:

func TestIssue39(t *testing.T) { conn, err := sql.Open("firebirdsql_createdb", "sysdba:masterkey@localhost:3050/tmp /go_test_issue39.fdb") if err != nil { t.Fatalf("Error connecting: %v", err) } conn.Close() conn, err = sql.Open("firebirdsql", "sysdba:masterkey@localhost:3050/tmp/go_test_issue38.fdb") defer conn.Close() tx, err := conn.Begin()

if err != nil {
    t.Fatalf("Error Begin: %v", err)
}
var rowId = sql.NullInt64{}
err = tx.QueryRow(
    "select 5 / 0 from rdb$database).Scan(&rowId)
if err == nil {
    t.Fatalf("'Dynamic SQL Error' is not occured.")
}
err = tx.Rollback()
if err != nil {
    t.Fatalf("Error Rollback: %v", err)
}

}

This will fail with: transaction_test.go:210: Error Rollback: Error op_response:0

nakagami commented 7 years ago

thanks. I have add a test code. And I will fix it.

michael88 commented 7 years ago

Hello again,

Maybe the following helps you: Code:

func main() {
    var n int
    conn, err := sql.Open("firebirdsql", "sysdba:xxxx@localhost/wetten?auth_plugin=Legacy_Auth&wire_crypt=false")
    if err != nil {
        log.Fatal("connection Error", err)
    }
    defer conn.Close()
    err = conn.Ping()
    if err != nil {
        log.Fatal("Pingerror ", err)
    }
    stmt, err := conn.Prepare("select 5 / 0 from rdb$database")
//stmt, err := conn.Prepare("select 5 / 2 from rdb$database")   
        if err != nil {
        log.Fatal("Prepare Error", err)
    }
    fmt.Println("Prepare Ok")
    rows, err1 := stmt.Query()
    if err1 != nil {
        log.Fatal(err1)
    }
    fmt.Println("rows Ok", rows)

    defer rows.Close()
    defer stmt.Close()
    err = rows.Err()
    if err != nil {
        log.Fatal("rowserr", err)
    }
    fmt.Println("vor rowsnext")
    ok := rows.Next()
    return
    fmt.Println("Relations count=", n)
}

Attachement fetch_with_errors (select 5 / 0 from rdb$database) Attachement fetch_without_errors (select 5 / 2 from rdb$database)

first Case -> rows.next op_fetch_response = 0 0 0 9 seconde Case -> rows.next op_fetch_response = 0 0 0 66 (and 4 more lines)

fetch_with_errors.txt fetch_without_errors.txt

michael88 commented 7 years ago

The real Problem is not fixed:

Maybe it is in rows.go line 837 if bytes_to_bint32(b) != op_fetch_response { return nil, false, errors.New("opFetchResponse:Internal Error") } Hier you get,in case of failure ,Opcode 9 (op response) and not op_fetch_response

And, still on serverside I get: INET/inet_error: read errno = 104 Should I open a new issue about it?

nakagami commented 7 years ago

Should I open a new issue about it?

No.

err = tx.QueryRow("select 5 / 0 from rdb$database").Scan(&rowId)

I think, this statement return error("sql: no rows in result set"), So Opcode 0 is no so important. If query has error, we can't avoid error on commit/rollback I think.

Do you have good idea ?

michael88 commented 7 years ago

The error is important. I'll give one example. Say we have a stored procedure, doing something with customers. Then I would write something like this:

procedure do_something(in_id_customer bigint)
returns(some_text varchar(40)
as
begin
if (not exists(select id from customer where customer.id = :in_id_customer)) then begin
    exception e_standard 'Customer ' || in_id_customer ' not found ';
end
--....... do something with customer and fill some_text.
suspend;
end procedure

and call this with select some_text from some_procedure(3);

Using your library I will get opcode 0. If I use node.js and the library https://github.com/hgourvest/node-firebird then I would get the correct error "exception e_standard Customer 3 not found"

This is essential for production use. Imagine, for example, a customer calls me and say, "hey your program has errors, the message is: opcode 0"! In this case, I get no info, what to do and what happened.

In Contrast to: Hey I get the error "Customer 3 not found". In this case it is clear, that the procedure is called with a non existent customer

Second: there must be a reason for the servermessage INET/inet_error: read errno = 104 because this happens only on failure of the query. I dont know the firebird wire protocol conventions, but there must be something wrong in the handling from fetching a record when the returning opcode is op_response (instead of op_fetch_response) Maybe you can take a look intto https://github.com/hgourvest/node-firebird and see something interesting.

michael88 commented 7 years ago

Maybe the First Step to success!

if I change: (rows.go line 837)

if bytes_to_bint32(b) != op_fetch_response {
   return nil, false, errors.New("opFetchResponse:Internal Error")
}

to

if bytes_to_bint32(b) != op_fetch_response {
   if bytes_to_bint32(b) == op_response {
      p._parse_op_response()
   }
   return nil, false, errors.New("opFetchResponse:Internal Error")
}

then the error in server log INET/inet_error: read errno = 104 no longer exists

michael88 commented 7 years ago

,,_,err := p_parse_op_response() even returns the correct error:

"arithmetic exception, numeric overflow, or string truncation Integer divide by zero. The code attempted to divide an integer value by an integer divisor of zero".

The only question left is, where to store the error. I know the function rows.Next must return io.EOF on error. Maybe you can add a field "lastError" to struct firebirdSqlRows and return this error-field on rows.Close() or rows.Err()

nakagami commented 7 years ago

Thanks, but still I don't know how to fix exactly. I want someone's pull request...

michael88 commented 7 years ago

I don't know how to pull a request

Change in writeprotocol.go Beginning at line 837

if bytes_to_bint32(b) != op_fetch_response {
   return nil, false, errors.New("opFetchResponse:Internal Error")
}

to

if bytes_to_bint32(b) != op_fetch_response {
   if bytes_to_bint32(b) == op_response {
      _, _, _, err := p._parse_op_response()
      if err != nil {
         return nil, false, err
     }
   }
   return nil, false, errors.New("opFetchResponse:Internal Error")
}

and in rows.go line 88 from

            if err == nil {
        rows.currentChunkRow = chunk.Front()
    }

to

    if err == nil {
        rows.currentChunkRow = chunk.Front()
    } else {
               return
        }

I think, you (and I also) thought, you!! must return io.EOF even on errors in rows.next. I looked at it and find out, that this is not true. The /database/sql Library Function does this. But before this library function returns io.EOF, it stores the error in Variable lastErr This is in standard library: database/sql line 2142 function next_locked

michael88 commented 7 years ago

Wait!!!! Do not change the Code with my Assumptions. I found some Failures when i run the Test!!!! Sorry

michael88 commented 7 years ago

Ok, now I have it.

First: if your commit (transactions) was only because issue 39 then you can undo it, we don't need id anymore (undo your commit https://github.com/nakagami/firebirdsql/commit/f4b78c6ddfc964369de281213a7ca2e39efd488b) Second: Do the Updates https://github.com/nakagami/firebirdsql/issues/39#issuecomment-309457304 Third: Test Issue 39 is wrong Instead of:

if err == nil {
        t.Fatalf("broken transaction, but error is not occured.")
    }

it should be

if err != nil {
        t.Fatalf("broken transaction, but error is not occured.")
    }

With this Changes anything runs fine.

michael88 commented 7 years ago

I have made a pull request regarding this issue

nakagami commented 7 years ago

continue to issue #41