feat: option-to-pass-optional-resty-or-http-client

[Zaimwa9]

Thanks for submitting a PR! Please check the boxes below:

• I have added information to docs/ if required so people know about the feature!
• I have filled in the "Changes" section below?
• I have filled in the "How did you test this code" section below?
• I have used a Conventional Commit title for this Pull Request

Changes

This PR aims to allow usage of custom client (HTTP or Resty) with priority over Resty client if both are provided.

• 2 new option funcs WithRestyClient and WithHttpClient
• Initialize the flagsmith client with priority to these 2 options
• Order of priority CustomResty => CustomHTTP => default resty
• Logging is composable — adding a custom logger to a client won’t override Flagsmith’s by default and will not be overriden

How did you test this code?

• Added tests

Manual tests:
https://github.com/Zaimwa9/flagsmith-go-sandbox
feat/custom-clients-with-loggers

• Create custom clients
• Link the local go client by uncommenting go.mod replace directive
• Test assignment of clients

e.g Test request logging with following round tripper configuration for custom HTTP Client

	rt := CustomRoundTripper{}

	customHttpClient := &http.Client{
		Transport: rt,
	}


func (ct CustomRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
	resp, err := http.DefaultTransport.RoundTrip(req)
	if err != nil {
		return nil, err
	}

	log.Printf("url: %s | method: %s", resp.Request.URL.String(), resp.Request.Method)

	data, err := io.ReadAll(resp.Body)
	if err != nil {
		return nil, err
	}
	_ = resp.Body.Close()

	log.Println("number of bytes in tripper:", len(data))
	log.Printf("Logging from custom http client, value in tripper: %s\n", string(data))
	resp.Body = io.NopCloser(bytes.NewReader(data))

	return resp, nil
}

2025/04/14 13:40:08 url: https://edge.api.flagsmith.com/api/v1/flags/ | method: GET
2025/04/14 13:40:08 number of bytes in tripper: 117
2025/04/14 13:40:08 Logging from custom http client, value in tripper: [{"feature": {"id": 132944, "name": "my_go_flag", "type": "STANDARD"}, "enabled": true, "feature_state_value": null}]

e.g Test request logging with following round tripper configuration for custom Resty Client

	restyClient := resty.New()
	logger := slog.New(slog.NewTextHandler(os.Stdout, nil))
	restyClient.OnBeforeRequest(func(c *resty.Client, req *resty.Request) error {
		logger.Info("Custom Resty: HTTP request",
			slog.String("method", req.Method),
			slog.String("url", req.URL),
			slog.Any("headers", req.Header),
		)
		return nil
	})

	restyClient.OnAfterResponse(func(c *resty.Client, resp *resty.Response) error {
		logger.Info("Custom Resty: HTTP response",
			slog.String("url", resp.Request.URL),
			slog.Int("status", resp.StatusCode()),
			slog.Int("bytes", len(resp.Body())),
			slog.Duration("elapsed", resp.Time()),
		)
		return nil
	})

time=2025-04-14T13:43:55.983+02:00 level=INFO msg="Custom Client Resty"
time=2025-04-14T13:43:55.983+02:00 level=INFO msg="Custom Resty: HTTP request" method=GET url=https://edge.api.flagsmith.com/api/v1/flags/ headers=map[]
time=2025-04-14T13:43:56.150+02:00 level=INFO msg="Custom Client Resty"
time=2025-04-14T13:43:56.151+02:00 level=INFO msg="Custom Resty: HTTP response" url=https://edge.api.flagsmith.com/api/v1/flags/ status=200 bytes=117 elapsed=167.24975ms

[Zaimwa9]

Basically here I detect client related options based on names. They need to be ran first because some options requires the client
e.g

func WithCustomHeaders(headers map[string]string) Option {
	return func(c *Client) {
		c.client.SetHeaders(headers)
	}
}

A clean alternative would be to change the option type like

type Option {
  apply: func(c *Client)
  isInit: bool
} 

It's quite a big refacto so let me know if it's worth going into this

[gagantrivedi]

What do we think about throwing an error if someone sets a custom client with options like timeout, etc

[Zaimwa9]

Good point. IMO timeout could be part of their custom client options that some users might like to keep.

I added a guard if no timeout specified here
but it could make sense to have a upper limit too (15sec) ?
Wdyt ?

[gagantrivedi]

Sorry, I meant the user should not be able to set conflicting options. For example, if they are setting an HTTP client, they should not be able to set a Resty client and vice versa. Additionally, if they are passing a custom client, they should not be able to use any options that modify the client.

[Zaimwa9]

Done, so now:

• Resty + Http client causes panic instead of prioritizing the resty client
• WithRequestTimeout / WithRetries / WithCustomHeaders / WithProxy will throw a panic if provided with custom clients

[Zaimwa9]

https://github.com/Flagsmith-Support/Axis/issues/11#issuecomment-2012242615

Also from this comment. When a custom client is provided, do we want to disable the following ?

c.client.SetTimeout(c.config.timeout)
SetLogger(newSlogToRestyAdapter(c.log)).
OnBeforeRequest(newRestyLogRequestMiddleware(c.log)).
OnAfterResponse(newRestyLogResponseMiddleware(c.log))

[matthewelwell]

Flagsmith-Support/Axis#11 (comment)

Also from this comment. When a custom client is provided, do we want to disable the following ?

c.client.SetTimeout(c.config.timeout)
SetLogger(newSlogToRestyAdapter(c.log)).
OnBeforeRequest(newRestyLogRequestMiddleware(c.log)).
OnAfterResponse(newRestyLogResponseMiddleware(c.log))

Yes, I think that makes sense.

[gagantrivedi]

What do we think about throwing an error if someone sets a custom client with options like timeout, etc

[Zaimwa9]

Added this. Specific test cases in which I re-expose the resty client to test the values

[gagantrivedi]

Nit: We follow the Given-When-Then comment pattern, which you are already using. However, it would be great if we explicitly mark those sections using comments.