v2: unexpected response on connect tear

[DifferentialOrange]

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/tarantool/go-tarantool"
    "log"
    "time"
)

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

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

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

   resp1, err1 := conn.Eval("require('fiber').sleep(10); return true", []interface{}{})
    log.Println(resp1)
    log.Println(err1)

If I stop running Tarantool with Ctrl+C golang prints

2021/12/27 17:42:54 <nil>
2021/12/27 17:42:54 EOF

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

[DifferentialOrange]

Seems like the best we should do is to fix examples to not confuse users that every request returns non-nil response with Code and Data, so definition of done for this issue is to rework README examples (and maybe tests).

[rybakit]

so definition of done for this issue is to rework README examples (and maybe tests).

Would it be possible to return a more meaningful error message, say unexpected EOF when reading from socket? EOF is just very obscure and does not give any details about what actually happened.

[DifferentialOrange]

Would it be possible to return a more meaningful error message, say unexpected EOF when reading from socket?

I'm not well acquainted with this part of the connector, so it's hard to say how difficult is to change the error. But we definitely will give it a try

[DifferentialOrange]

FZambia/tarantool#8 may be useful

[oleg-jukovec]

It seems to me that there is no need to remove the Response type from the public API. It is better to make the Date field private and add two public methods:

// data is same as Response.Data at now
Decode() (data []interface{}, err error)
// same as internal Response.decodeBodyTyped at now
DecodeTyped(res interface{}) (err error)

and do not call Response.decodeBody() from Future.Get()

So, usage will be look like:

resp, err := conn.Insert(...)
if err != nil {
	// resp == nil here
} else {
	// resp != nil here

	// so we can decode it
	data, err = resp.Decode()
	if err != nil {
		// data == nil
		// unable to decode the response
	} else {
		// data != nil
	}

	// or decode it in an other way
	err = resp.DecodeTyped(&tup)
	if err != nil {
		// unable to decode the response
	} else {
		// tup != nil
	}
}

old code:

resp, err := conn.Insert(...)
if err != nil {
	if resp != nil {
		// unable to get a response
	} else {
		// unable to decode the response
		// resp.Data == nil
	}
} else {
	// resp != nil && resp.Data != nil
}

resp, err := conn.InsertTyped(..., &tup)
if err != nil {
	if resp != nil {
		// unable to get a response
	} else {
		// unable to decode the response
	}
} else {
	// tup != nil
}

Advantages:

1. All Connection.*Typed() and Future.*Typed() methods can be removed. See FZambia's code.
2. There is no double logic in Future.Get(). It will return an error only if it can't get a response. Error handlind is more obvious.

Disadvantages:

1. It changes public API.
2. Error handling in client code is more complicated.
3. We need to write benchmarks for this first.

[Totktonada]

I like the response object idea and I think we can polish it to make error handling easier. But backward compatibility stops us here, sure. We can consider adding a separate conn.Do*() method after #126, but I doubt about a good name.