WithClientConfig overrides default ClientConfig values with zero values.

[iv-anas]

While using the Go Typesense client (v3.2.0), I noticed a potential configuration issue related to the functional options pattern and default configuration handling.

In NewClient, a default ClientConfig is initialized with sensible default values:

c := &Client{apiConfig: &ClientConfig{
    RetryInterval:             defaultRetryInterval,
    HealthcheckInterval:       defaultHealthcheckInterval,
    ConnectionTimeout:         defaultConnectionTimeout,
    CircuitBreakerName:        defaultCircuitBreakerName,
    CircuitBreakerMaxRequests: circuit.DefaultGoBreakerMaxRequests,
    CircuitBreakerInterval:    circuit.DefaultGoBreakerInterval,
    CircuitBreakerTimeout:     circuit.DefaultGoBreakerTimeout,
    CircuitBreakerReadyToTrip: circuit.DefaultReadyToTrip,
}}

However, when using the WithClientConfig option:

client := typesense.NewClient(
    typesense.WithClientConfig(&typesense.ClientConfig{
        ServerURL: cfg.ServerURL,
        APIKey:    cfg.APIKey,
    }),
)

The implementation of WithClientConfig assigns all fields from the provided config directly:

func WithClientConfig(config *ClientConfig) ClientOption {
    return func(c *Client) {
        c.apiConfig.ServerURL = config.ServerURL
        c.apiConfig.NearestNode = config.NearestNode
        c.apiConfig.Nodes = config.Nodes
        c.apiConfig.NumRetries = config.NumRetries
        c.apiConfig.RetryInterval = config.RetryInterval
        c.apiConfig.HealthcheckInterval = config.HealthcheckInterval
        c.apiConfig.APIKey = config.APIKey
        c.apiConfig.ConnectionTimeout = config.ConnectionTimeout
        c.apiConfig.CircuitBreakerName = config.CircuitBreakerName
        c.apiConfig.CircuitBreakerMaxRequests = config.CircuitBreakerMaxRequests
        c.apiConfig.CircuitBreakerInterval = config.CircuitBreakerInterval
        c.apiConfig.CircuitBreakerTimeout = config.CircuitBreakerTimeout
        c.apiConfig.CircuitBreakerReadyToTrip = config.CircuitBreakerReadyToTrip
        c.apiConfig.CircuitBreakerOnStateChange = config.CircuitBreakerOnStateChange
    }
}

If only a subset of fields is provided (e.g., only ServerURL and APIKey), the remaining fields are Go zero values and overwrite the defaults set in NewClient.

// implement option pattern
	for _, opt := range opts {
		opt(c)
	}

Question / Clarification

1. Is it the intended behavior that WithClientConfig fully overrides all defaults?
2. Specifically, should NumRetries (and other retry-related fields) have a non-zero default if not explicitly provided?

[iv-anas]

@tharropoulos what about the default value for NumRetries? Currently it is zero. But I think it is the standard practice there would be default non-zero value for retries.

[tharropoulos]

The default in every client library is the number of nodes + the nearest node (max is 4, default is 1 on single-node environments). The only fallback missing is another check for 3. But the behavior is in sync with the rest of the clients.