Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NewClient doesn't properly report client connection errors due to invalid headers #184

Open
jlewi opened this issue Sep 9, 2023 · 0 comments

Comments

@jlewi
Copy link

jlewi commented Sep 9, 2023

I called NewClient with invalid headers in the configuration. Specifically, I set the OpenAPIKey header but the value was invalid because it had newlines in it. This results in an invalid request and thus NewClient times out waiting for Weaviate to be ready. However, the error isn't properly reported; the only error we get is a timeout waiting for weaviate. As a result, its extremely difficult to root cause the failure.

Here are more details:

NewClient ends up timing out trying to establish a connection. Here

if err := con.WaitForWeaviate(config.StartupTimeout); err != nil {

If we step into code we see the REST request made here is failing with an error

The error is
"net/http: invalid header field value for "X-Openai-Api-Key""

So the error message indicates the root cause. The response is also nil. I think this combination implies that the request never gets made and thus its a permanent error and retrying is pointless.

When the retry eventually times out the only error that's returned is that it timed out

return fmt.Errorf("weaviate did not start up in %s. Either the Weaviate URL %q is wrong or Weaviate did not start up in the interval given in 'startupTimeout'", startupTimeout.String(), con.basePath)

So the root cause isn't reported. This makes it extremely hard to debug.

I suggest two fixes

  1. Errors should be classified as permanent or retryable; permanent errors should not be retried as that just slows things down
  2. In the event of a timeout; return the most recent error to help identify the issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant