Context not respected for Ping command

[dominicbarnes]

Issue description

If you try to ping a non-existent Snowflake account, you just get an HTTP timeout after about 60s. This is pretty long, and there is Config.LoginTimeout which cancels the HTTP request after another duration.

However, if you use a Context that expires, this does not get respected by PingContext and the LoginTimeout (if specified) or the raw HTTP timeout gets triggered instead.

Example code

config := gosnowflake.Config{
  // include other auth params
  // use an "account" that does not exist to demonstrate this problem
  LoginTimeout: 30 * time.Second,
}

url, err := gosnowflake.DSN(&config)
if err != nil {
  return err
}

db, err := sql.Open("snowflake", url)
if err != nil {
  return err
}

ctx, cancel := context.WithCancel(context.Background())
cancel() // cancel immediately rather than using timeout to prove that context is not respected

if err := db.PingContext(ctx); err != nil {
  // this will fail after ~30s, but it should fail immediately
  return err
}

Configuration

Driver version (or git SHA): 1.1.2

Go version: go version go1.9.2 linux/amd64

[smtakeda]

Thanks for filing an issue. I'll look into this tomorrow morning or earlier.

[smtakeda]

It appeared the driver method Ping is called only after the connection is success. Hence the context is not honored if the connection fails. In that sense, the functionality of Ping is not really to check the connectivity rather making sure the connected database is functional.

Anyway I believe the goal here is when an invalid account name is specified, how quickly connection failure can be detected and raises an error to the user, right?

I see the issue is LoginTimeout is not honored if it is less than 60 seconds and the connection hangs. But I get 403 back as soon as it founds the endpoint doesn't exist, e.g., https://foobar-somewhere.snowflakecomputing.com/, so LoginTimeout works.

Do you experience something different to me? such as hanging and waiting for 60 seconds on every connection failure?

[dominicbarnes]

@smtakeda as described in my code sample, the LoginTimeout works just fine. The context not being respected is the reason for this bug report, as I tried this method before I discovered the LoginTimeout config.

As far as what to do for an invalid account, I agree with you that it would be much preferred that we identify that condition much more quickly as it's currently indistinguishable from a network error.

[smtakeda]

Thanks for confirmation. At the moment, I cannot do much for the behavior of Ping with context due to what I discovered. But the driver could raise an error as soon as HTTP 403 is returned in authentication. The only reason why the driver retries for 403 is because AWS S3 endpoints sometimes return 403 and retry often works, but since the authentication won't involve AWS S3 endpoints, I believe it should be fine. Working on the fix.

[smtakeda]

@dominicbarnes can you please try the latest code? e1f17ec

[dominicbarnes]

The changes worked great! Thank you very much, I don't think this fix is necessary anymore.