Unexpected response on connect tear

[DifferentialOrange]

Behavior for this connector is similar as in tarantool/go-tarantool, see here: tarantool/go-tarantool#129

Tarantool setup

box.cfg{ listen = 3401 }
box.schema.user.grant('guest', 'read,write,execute,create,drop,alter', 'universe', nil, { if_not_exists = true })

Go code

package main

import (
    "github.com/FZambia/tarantool"
    "log"
    "time"
)

func main() {
    var uri string = "127.0.0.1:3401"
    var user string = "guest"

    opts := tarantool.Opts{User: user, MaxReconnects: 5, ReconnectDelay: 3 * time.Second}
    conn, err := tarantool.Connect(uri, opts)

    if err != nil {
        log.Fatalf("Connection refused:", err)
    }

    eval := tarantool.Eval("require('fiber').sleep(10); return true", []interface{}{})

    resp1, err1 := conn.Exec(eval)
    log.Println(resp1)
    log.Println(err1)
}

If I stop running Tarantool with Ctrl+C golang prints

2021/12/27 20:22:05 <nil>
2021/12/27 20:22:05 EOF

Response resp do not have Code and Data fields as usual, attempt to extract them results in segmentation fault; error seems useless.

[FZambia]

@DifferentialOrange what you expect to see? EOF seems fine to me. Segmentation fault? Do you call Go panic a segmentation fault?

[DifferentialOrange]

@DifferentialOrange what you expect to see? EOF seems fine to me. Segmentation fault? Do you call Go panic a segmentation fault?

In case of ordinary error, there is a usual structure:

eval := tarantool.Eval("error('err')", []interface{}{})

resp1, err1 := conn.Exec(eval)
log.Println(resp1.Code)
log.Println(resp1.Data)
log.Println(err1)

logs out

2021/12/28 09:20:44 32
2021/12/28 09:20:44 []
2021/12/28 09:20:44 eval:1: err (0x20)

So in case of request error response structure is still returned. Or is it not in convention to always return this structure?

I don't know if it's hard to handle, but seeing more readable error like client connection is not ready (0x4000) would be more convenient than EOF.

[FZambia]

I see. Well, looking at error handling in general – it's a bit confusing here at the moment. I think Response should not contain Code and Error fields at all. Moreover, Response struct does not make a lot of sense – we can simply return []interface{}, error from Exec. Where error could be ClientError or Error, sth like this:

func (conn *Connection) Exec(req *Request) ([]interface{}, error) {...}

This may remove ambiguity in error handling. Especially since methods like:

func (conn *Connection) ExecTyped(req *Request, result interface{}) error {...}

– already only have error returned.

In this case even if io.EOF returned without wrapping it into ClientError it will be expected that data should not be processed.

Need some more research whether this makes sense though. What do you think?

[DifferentialOrange]

The issue here is based on tarantool/go-tarantool#129 : all connector examples in README have this

log.Println(resp.Code)
log.Println(resp.Data)
log.Println(err)

output print and returning nil breaks this sequence. Since your connector have similar return conventions (while README examples process nil, err case carefully), I thought it may be an unexpected return format for you too (at least it surprised me at first) and decided to file this issue.

I agree that the question needs research (including verifying return conventions and examples in tarantool/go-tarantool). I don't have any answers for this question for now and we need to think what to do (or does there anything we really need to do).

[FZambia]

Since your connector have similar return conventions (while README examples process nil, err case carefully), I thought it may be an unexpected return format for you too (at least it surprised me at first) and decided to file this issue.

In Go if an error returned from a function call in most cases response is unusable. So this does not surprise me.

But I am not fully understand why we want to stick to Response struct at all – the only useful thing it contains is Data []interface{}. Everything else can be returned as an error. We can still change it here since this driver is experimental and some API changes are expected. Next step here is to look how ExecTyped returns errors currently - if all possible errors (from Tarantool, ClientErrors or io.EOF) all returned as an error – then no real reason to keep Response in Exec.

[DifferentialOrange]

But I am not fully understand why we want to stick to Response struct at all – the only useful thing it contains is Data []interface{}.

There are also response codes, but I don't know if they are any use cases.

[FZambia]

Merged #8 -error handling should be more intuitive. I have not changed the type of error returned on connection close - it's still io.EOF, but I don't think this is a problem in general. Let's see how it goes – we can adjust sth later if required.