Skip to content

Do not use http.DefaultClient and more use of WithTransport#27

Merged
myleshorton merged 4 commits intomainfrom
myles/no-DefaultClient
Dec 19, 2025
Merged

Do not use http.DefaultClient and more use of WithTransport#27
myleshorton merged 4 commits intomainfrom
myles/no-DefaultClient

Conversation

@myleshorton
Copy link
Contributor

This pull request refactors how transports are added and managed in the Kindling library, making the transport system more extensible and easier to use. It introduces a minimal Transport interface, simplifies the addition of new transports, and cleans up related code and documentation. The most important changes are grouped below.

Transport System Refactoring

  • Introduced a minimal Transport interface, defining the required methods for custom transports, and updated the code and documentation to use this interface. (kindling.go, README.md) [1] [2]
  • Added a new WithTransport option to allow users to add any custom transport that implements the Transport interface. (kindling.go)
  • Refactored existing transport-related options (WithDomainFronting, WithDNSTunnel, WithAMPCache, WithProxyless) to leverage the new WithTransport mechanism and improve error handling. (kindling.go) [1] [2]

Code Simplification and Cleanup

  • Removed the cached httpClient field from the kindling struct and updated NewHTTPClient to always return a new http.Client with the appropriate transport, simplifying thread safety and usage. (kindling.go) [1] [2]
  • Added an emptyOption type to handle cases where an option is skipped due to missing dependencies, improving robustness. (kindling.go)

@myleshorton
Copy link
Contributor Author

Gonna pull this in. There is still definitely a race condition if ReplaceTransport were ever actually used, but I don't think it is atm.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request refactors the transport management system in the Kindling library to make it more extensible and user-friendly. It removes the use of http.DefaultClient, introduces a minimal Transport interface, and simplifies how custom transports are added.

Key Changes:

  • Introduced a Transport interface with three methods (NewRoundTripper, MaxLength, Name) for custom transport implementations
  • Removed cached httpClient field from the kindling struct; NewHTTPClient() now always returns a new client
  • Refactored transport options (WithDomainFronting, WithDNSTunnel, WithAMPCache, WithProxyless) to use the new WithTransport function and added emptyOption for better error handling

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
kindling.go Removed cached httpClient, introduced Transport interface, added WithTransport function, refactored transport options to use WithTransport, and added emptyOption type for handling nil dependencies
README.md Added documentation for the new Transport interface with detailed method descriptions and usage example

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

myleshorton and others added 3 commits December 19, 2025 16:11
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@myleshorton myleshorton merged commit 21e0da2 into main Dec 19, 2025
1 check passed
@myleshorton myleshorton deleted the myles/no-DefaultClient branch December 19, 2025 23:14
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

Successfully merging this pull request may close these issues.

2 participants