Clarify OpenAI client timeout precedence
This commit is contained in:
@@ -20,12 +20,17 @@ fields:
|
|||||||
| `PromptDir` | Prompt-definition directory, required unless a prompt source option is supplied. |
|
| `PromptDir` | Prompt-definition directory, required unless a prompt source option is supplied. |
|
||||||
| `ProfileDir` | Optional custom profile directory over built-ins. |
|
| `ProfileDir` | Optional custom profile directory over built-ins. |
|
||||||
| `SchemaDir` | Schema directory; empty uses `.`. |
|
| `SchemaDir` | Schema directory; empty uses `.`. |
|
||||||
| `Timeout` | Default timeout for the built-in OpenAI-compatible client. |
|
| `Timeout` | Base timeout for the built-in OpenAI-compatible client when `HTTPClient` is absent or has a zero timeout. A non-positive value uses the internal default. |
|
||||||
| `HTTPClient` | Optional HTTP client for that built-in client. |
|
| `HTTPClient` | Optional HTTP client for that built-in client. It is cloned; a non-zero `Timeout` on it takes precedence over `Config.Timeout` as the base timeout. |
|
||||||
|
|
||||||
Nil options are ignored. Invalid construction, including
|
Nil options are ignored. Invalid construction, including
|
||||||
`WithLLMClient(nil)`, returns an error matching `ErrInvalidConfig`.
|
`WithLLMClient(nil)`, returns an error matching `ErrInvalidConfig`.
|
||||||
|
|
||||||
|
An effective positive `timeout_seconds` replaces the base timeout. An explicit
|
||||||
|
request override of zero disables the HTTP-client timeout. The timeout is
|
||||||
|
otherwise inherited from the supplied client, `Config.Timeout`, or the internal
|
||||||
|
default in that order.
|
||||||
|
|
||||||
Source options replace their matching directory source:
|
Source options replace their matching directory source:
|
||||||
|
|
||||||
- prompts: `WithPromptFS(fsys, root)`, `WithPromptFile(path)`;
|
- prompts: `WithPromptFS(fsys, root)`, `WithPromptFile(path)`;
|
||||||
|
|||||||
@@ -77,9 +77,12 @@ resolves the configured non-empty `api_key_env` at request time and sends the
|
|||||||
same header. If neither mechanism supplies a key, it sends no
|
same header. If neither mechanism supplies a key, it sends no
|
||||||
`Authorization` header.
|
`Authorization` header.
|
||||||
|
|
||||||
The configured client timeout applies by default. A positive effective
|
The client base timeout is chosen at construction: a non-zero timeout on a
|
||||||
`timeout_seconds` replaces it. An explicit request override of zero disables
|
supplied `http.Client` takes precedence over `Config.Timeout`; otherwise a
|
||||||
the HTTP-client timeout; negative values are rejected before a request is sent.
|
positive `Config.Timeout` is used, then the internal default. A positive
|
||||||
|
effective `timeout_seconds` replaces that base. An explicit request override
|
||||||
|
of zero disables the HTTP-client timeout; negative values are rejected before a
|
||||||
|
request is sent.
|
||||||
|
|
||||||
## Response Subset And Failures
|
## Response Subset And Failures
|
||||||
|
|
||||||
|
|||||||
@@ -10,13 +10,14 @@ format and protocol behavior.
|
|||||||
## Construction
|
## Construction
|
||||||
|
|
||||||
`NewOpenAICompatibleClient` validates a non-empty configured base URL, records
|
`NewOpenAICompatibleClient` validates a non-empty configured base URL, records
|
||||||
an optional default model, and establishes the default timeout. A non-positive
|
an optional default model, and resolves one base timeout. A supplied client with
|
||||||
configured timeout uses the internal default.
|
a non-zero timeout supplies that base; otherwise a positive configured timeout
|
||||||
|
is used, then the internal default.
|
||||||
|
|
||||||
When callers supply an `http.Client`, construction clones it rather than
|
When callers supply an `http.Client`, construction clones it rather than
|
||||||
mutating the caller's instance. A supplied client with no timeout receives the
|
mutating the caller's instance. A supplied client with no timeout receives the
|
||||||
resolved default in the clone; a supplied non-zero timeout is retained. The
|
resolved base timeout in the clone. The client stores the trimmed base URL,
|
||||||
client stores the trimmed base URL, default model, timeout, and cloned client.
|
default model, and cloned client.
|
||||||
|
|
||||||
## Generate Flow
|
## Generate Flow
|
||||||
|
|
||||||
@@ -28,7 +29,7 @@ client stores the trimmed base URL, default model, timeout, and cloned client.
|
|||||||
request;
|
request;
|
||||||
4. prefer a direct API key, otherwise resolve the configured key environment
|
4. prefer a direct API key, otherwise resolve the configured key environment
|
||||||
variable;
|
variable;
|
||||||
5. derive a request HTTP client when an explicit timeout changes the configured
|
5. derive a request HTTP client only when an explicit timeout changes the base
|
||||||
client;
|
client;
|
||||||
6. execute the request, reject non-success status responses without returning
|
6. execute the request, reject non-success status responses without returning
|
||||||
provider response bodies; and
|
provider response bodies; and
|
||||||
|
|||||||
@@ -36,7 +36,6 @@ type OpenAICompatibleConfig struct {
|
|||||||
type OpenAICompatibleClient struct {
|
type OpenAICompatibleClient struct {
|
||||||
baseURL string
|
baseURL string
|
||||||
defaultModel string
|
defaultModel string
|
||||||
timeout time.Duration
|
|
||||||
httpClient *http.Client
|
httpClient *http.Client
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -67,7 +66,6 @@ func NewOpenAICompatibleClient(cfg OpenAICompatibleConfig) (*OpenAICompatibleCli
|
|||||||
return &OpenAICompatibleClient{
|
return &OpenAICompatibleClient{
|
||||||
baseURL: strings.TrimRight(baseURL, "/"),
|
baseURL: strings.TrimRight(baseURL, "/"),
|
||||||
defaultModel: cfg.Model,
|
defaultModel: cfg.Model,
|
||||||
timeout: timeout,
|
|
||||||
httpClient: client,
|
httpClient: client,
|
||||||
}, nil
|
}, nil
|
||||||
}
|
}
|
||||||
@@ -116,19 +114,20 @@ func (c *OpenAICompatibleClient) Generate(ctx context.Context, req domain.Genera
|
|||||||
httpReq.Header.Set("Authorization", "Bearer "+apiKey)
|
httpReq.Header.Set("Authorization", "Bearer "+apiKey)
|
||||||
}
|
}
|
||||||
|
|
||||||
effectiveTimeout := c.timeout
|
|
||||||
if req.Target.TimeoutSeconds > 0 {
|
|
||||||
effectiveTimeout = time.Duration(req.Target.TimeoutSeconds) * time.Second
|
|
||||||
} else if req.TargetPresence.TimeoutSeconds {
|
|
||||||
effectiveTimeout = 0
|
|
||||||
}
|
|
||||||
|
|
||||||
httpClient := c.httpClient
|
httpClient := c.httpClient
|
||||||
if httpClient == nil {
|
if httpClient == nil {
|
||||||
httpClient = &http.Client{Timeout: effectiveTimeout}
|
httpClient = &http.Client{Timeout: defaults.LLMRequestTimeoutDefault}
|
||||||
} else if httpClient.Timeout != effectiveTimeout {
|
}
|
||||||
|
if req.Target.TimeoutSeconds > 0 {
|
||||||
|
effectiveTimeout := time.Duration(req.Target.TimeoutSeconds) * time.Second
|
||||||
|
if httpClient.Timeout != effectiveTimeout {
|
||||||
|
cloned := *httpClient
|
||||||
|
cloned.Timeout = effectiveTimeout
|
||||||
|
httpClient = &cloned
|
||||||
|
}
|
||||||
|
} else if req.TargetPresence.TimeoutSeconds && httpClient.Timeout != 0 {
|
||||||
cloned := *httpClient
|
cloned := *httpClient
|
||||||
cloned.Timeout = effectiveTimeout
|
cloned.Timeout = 0
|
||||||
httpClient = &cloned
|
httpClient = &cloned
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -31,9 +31,6 @@ func TestNewOpenAICompatibleClientDoesNotMutateSuppliedZeroTimeoutClient(t *test
|
|||||||
if client.httpClient == supplied {
|
if client.httpClient == supplied {
|
||||||
t.Fatal("expected constructed client to use a cloned HTTP client")
|
t.Fatal("expected constructed client to use a cloned HTTP client")
|
||||||
}
|
}
|
||||||
if client.httpClient.Timeout != client.timeout {
|
|
||||||
t.Fatalf("expected cloned client timeout %v, got %v", client.timeout, client.httpClient.Timeout)
|
|
||||||
}
|
|
||||||
if client.httpClient.Timeout <= 0 {
|
if client.httpClient.Timeout <= 0 {
|
||||||
t.Fatalf("expected constructed client to use a positive default timeout, got %v", client.httpClient.Timeout)
|
t.Fatalf("expected constructed client to use a positive default timeout, got %v", client.httpClient.Timeout)
|
||||||
}
|
}
|
||||||
@@ -674,8 +671,9 @@ func TestOpenAICompatibleClientExplicitZeroTimeoutDisablesClientTimeout(t *testi
|
|||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{
|
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{
|
||||||
BaseURL: ts.URL + "/v1",
|
BaseURL: ts.URL + "/v1",
|
||||||
Timeout: time.Nanosecond,
|
Timeout: time.Nanosecond,
|
||||||
|
HTTPClient: &http.Client{Timeout: time.Nanosecond},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
@@ -718,6 +716,34 @@ func TestOpenAICompatibleClientOmittedTimeoutUsesClientTimeout(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestOpenAICompatibleClientSuppliedHTTPClientTimeoutOverridesConfigTimeout(t *testing.T) {
|
||||||
|
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
time.Sleep(25 * time.Millisecond)
|
||||||
|
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
|
||||||
|
}))
|
||||||
|
defer ts.Close()
|
||||||
|
|
||||||
|
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{
|
||||||
|
BaseURL: ts.URL + "/v1",
|
||||||
|
Model: "model",
|
||||||
|
Timeout: 5 * time.Millisecond,
|
||||||
|
HTTPClient: &http.Client{Timeout: 100 * time.Millisecond},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
resp, err := client.Generate(context.Background(), domain.GenerateRequest{
|
||||||
|
Prompt: domain.RenderedPrompt{Messages: []domain.RenderedMessage{{Role: "user", Content: "hi"}}},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected supplied client timeout to allow the request, got %v", err)
|
||||||
|
}
|
||||||
|
if resp.Content != "ok" {
|
||||||
|
t.Fatalf("unexpected response content: %q", resp.Content)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestOpenAICompatibleClientRejectsInvalidExtraParamsBeforeProviderCall(t *testing.T) {
|
func TestOpenAICompatibleClientRejectsInvalidExtraParamsBeforeProviderCall(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -1011,9 +1037,10 @@ func TestOpenAICompatibleClientRequestTimeoutOverride(t *testing.T) {
|
|||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{
|
client, err := NewOpenAICompatibleClient(OpenAICompatibleConfig{
|
||||||
BaseURL: ts.URL + "/v1",
|
BaseURL: ts.URL + "/v1",
|
||||||
Model: "m",
|
Model: "m",
|
||||||
Timeout: 50 * time.Millisecond,
|
Timeout: 5 * time.Millisecond,
|
||||||
|
HTTPClient: &http.Client{Timeout: 50 * time.Millisecond},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
|
|||||||
Reference in New Issue
Block a user