search

databricks / databricks-sql-go

Golang database/sql driver for Databricks SQL.
Apache License 2.0
37 stars 41 forks source link

RoundTripper returned a response & error; ignoring response #237

Open mdibaiee opened 4 months ago

mdibaiee commented 4 months ago

We are using this library in our connector: https://github.com/estuary/connectors/tree/main/materialize-databricks

We have been seeing this log from the library:

RoundTripper returned a response & error; ignoring response

I did some investigation and it seems Golang's http.Client expects that only one of resp or err have a non-nil value in the response of a RoundTripper.RoundTrip, see: https://github.com/golang/go/blob/release-branch.go1.22/src/net/http/client.go#L260-L264

Also see the documentation here: https://pkg.go.dev/net/http#RoundTripper

RoundTrip must return err == nil if it obtained a response, regardless of the response's HTTP status code. A non-nil err should be reserved for failure to obtain a response.

A similar issue has been spotted and fixed here: https://github.com/nfx/slrp/commit/5ea1bdd4677e668263df5dce225be6da7bde03f9

I haven't yet found a way to fix this issue in this library, perhaps maintainers know better where to look for possible causes of this error.

kravets-levko commented 3 months ago

Hi @mdibaiee! Thank you for reporting this. I've found a suspicious place that may cause this, but, unfortunately, cannot reproduce this particular issue to check if my assumption is correct. Can you maybe provide more details on what you do to get this log message? Or anything else thay may help. Thanks!

https://github.com/databricks/databricks-sql-go/blob/34b734066dbb9b98936f3c226c28de832dc56aa8/internal/client/client.go#L527-L531

mdibaiee commented 3 months ago

Hi @kravets-levko! Unfortunately this log was not accompanied by much else and the underlying reason is also enigmatic to me.

The line you reference is probably the top of the hierarchy... I'd imagine there is some implementation of http.RoundTripper being used as t.Base which is not standard, and that implementation has this issue.

We don't pass a custom Transport when we use the databricks-sql-go client, that's why I think this is probably happening somewhere in the library.

kravets-levko commented 3 months ago

Thank you @mdibaiee! As I understand, this log output actually doesn't cause any further problems. I will keep an eye on it, maybe I'll reproduce it and/or find what causes it. If by any chance you'll have any updates on this - please let me know