Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ rather than deprecated, so consuming code must be updated.
- **`ColorTemperature.Kelvin` range widened to `1000–20000 K`** to match the docs' forward-
compatible guidance, with a new `ColorTemperature.KelvinUnchecked(int)` escape hatch for
values outside that range.
- **`WLedClient(string)` now refreshes DNS.** The convenience constructor's self-owned
`HttpClient` uses a `SocketsHttpHandler` with a bounded `PooledConnectionLifetime` (2 minutes)
on modern runtimes, so a long-lived client picks up DNS/IP changes instead of pinning a stale
connection (fixes [#8](https://github.com/kevbite/WLED.NET/issues/8)). The `netstandard2.0`
build falls back to `HttpClientHandler`; use `IHttpClientFactory`/DI there.

### Added

Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ services.AddWledClient("http://office-computer-wled/");
services.AddWledClient(client => client.BaseAddress = new Uri("http://office-computer-wled/"));
```

The `WLedClient(string)` constructor owns its `HttpClient` and uses a `SocketsHttpHandler`
with a bounded `PooledConnectionLifetime`, so a long-lived client still picks up DNS changes
(e.g. a WLED device that gets a new IP). For applications, registering via
`IHttpClientFactory`/DI as above remains the recommended approach.

### Quick commands

Common operations have first-class "intent" methods:
Expand Down
111 changes: 111 additions & 0 deletions plans/14-httpclient-connection-lifetime.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# Plan 14 — HttpClient connection lifetime & DNS staleness

**Theme:** Correctness · Reliability · Cross-cutting

Tracks [issue #8](https://github.com/kevbite/WLED.NET/issues/8).

## Why

A long-lived `HttpClient` over a default `HttpClientHandler` keeps its TCP connections
open indefinitely and therefore **never honours DNS changes** — a documented .NET pitfall
(see [HttpClient guidelines: DNS behavior](https://learn.microsoft.com/en-us/dotnet/fundamentals/networking/http/httpclient-guidelines#dns-behavior)).

WLED devices are a realistic trigger for this:

- they are frequently addressed by mDNS/hostname or a DHCP-assigned IP that can change
(device reboot, lease renewal, network change), and
- a typical consumer creates **one** `WLedClient("http://wled.local/")` for the lifetime of
the app and reuses it.

The DI path (`services.AddWledClient(...)`) is already safe: it goes through
`IHttpClientFactory`, which rotates handlers on a default 2-minute lifetime. The problem is
isolated to the convenience constructors that own a self-created `HttpClient`:

- `WLedClient(string baseUri)` → `new HttpClientHandler()` (see `WLedClient.cs`).
- `WLedClient(HttpMessageHandler, string)` → caller-supplied handler (out of scope; the
caller owns its lifetime).

## Goal

When the library creates the handler itself, give pooled connections a finite lifetime so
DNS is periodically refreshed, **without** regressing throughput or the netstandard2.0
target.

## Approach

In `WLedClient`, replace the default `new HttpClientHandler()` used by
`WLedClient(string baseUri)` with a `SocketsHttpHandler` configured with a bounded
`PooledConnectionLifetime`, on runtimes where `SocketsHttpHandler` exists.

`SocketsHttpHandler` is available on .NET Core 2.1+/.NET 5+ but **not** in the
netstandard2.0 reference assemblies, so this must be guarded by a target-framework
conditional rather than a runtime check:

```csharp
private static HttpMessageHandler CreateDefaultHandler()
{
#if NETSTANDARD2_0
// .NET Framework / legacy: SocketsHttpHandler is unavailable. Fall back to the
// platform handler; consumers on these runtimes should prefer IHttpClientFactory.
return new HttpClientHandler();
#else
return new SocketsHttpHandler
{
// Refresh pooled connections (and therefore DNS) periodically.
PooledConnectionLifetime = TimeSpan.FromMinutes(2)
};
#endif
}
```

Wire it into the existing constructor:

```csharp
public WLedClient(string baseUri) : this(CreateDefaultHandler(), baseUri)
{
}
```

Notes / decisions:

- **Two minutes** mirrors `IHttpClientFactory`'s default handler lifetime, keeping the two
paths consistent. It is a reasonable default for LAN devices and can be revisited if a
configurable overload is later requested.
- Keep the existing `Connection: keep-alive` default request header; `PooledConnectionLifetime`
bounds connection reuse without disabling keep-alive.
- **Do not** change the `WLedClient(HttpMessageHandler, string)` or `WLedClient(HttpClient)`
constructors — those hand ownership of the handler/client to the caller (including DI),
and overriding their lifetime would be surprising.
- netstandard2.0 constraint: `SocketsHttpHandler` is not referenced under that TFM, so the
`#if` keeps the netstandard2.0 build green (it currently builds clean and must stay so).

## Tests

- A test asserting `new WLedClient("http://wled.local/")` constructs successfully and can
issue a request (the existing `MockHttpMessageHandler` path already covers request
behaviour; this guards the default-handler wiring on net8/9/10).
- Because handler internals aren't observable through `HttpClient`, prefer a small,
internal, testable seam if exact `PooledConnectionLifetime` verification is wanted:
e.g. an `internal static HttpMessageHandler CreateDefaultHandler()` exposed to the test
project via `InternalsVisibleTo`, asserting it returns a `SocketsHttpHandler` with the
expected lifetime on modern TFMs. Keep this minimal and conditional-compiled.
- Confirm the full multi-target build (incl. netstandard2.0) and the existing 205 tests
remain green.

## Definition of Done (per `plans/README.md`)

1. **README.md** — add a short note under connecting/DI guidance explaining that the
convenience constructor now refreshes DNS via a bounded connection lifetime, and that
`IHttpClientFactory`/DI remains the recommended approach for apps.
2. **samples/BasicConsole** — no behavioural change required; the existing
`new WLedClient(...)` usage now benefits automatically. Update a comment if helpful.
3. **CHANGELOG.md** — record the fix under the unreleased section
(e.g. *"The `WLedClient(string)` constructor now uses a `SocketsHttpHandler` with a
bounded `PooledConnectionLifetime` so DNS changes are picked up (fixes #8)."*).

## Out of scope

- Making the connection lifetime configurable via a new constructor/options overload — can
be a fast follow if requested.
- Any change to the DI package, which already delegates lifetime management to
`IHttpClientFactory`.
1 change: 1 addition & 0 deletions plans/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ These plans cross-reference two mature community libraries (linked by the WLED d
| 11 | [Client ergonomics & cross-cutting concerns](11-client-ergonomics-and-cross-cutting.md) | Quality |
| 12 | [Usability & correctness improvements (post-review)](12-usability-and-correctness-improvements.md) | Correctness |
| 13 | [Device ergonomics, catalogs & agent guidance](13-device-ergonomics-and-agent-guidance.md) | Usability |
| 14 | [HttpClient connection lifetime & DNS staleness](14-httpclient-connection-lifetime.md) | Correctness |

Plan 0 modernises the toolchain (multi-targeting `netstandard2.0;net8.0;net9.0;net10.0`)
and should land first. Plans 1–2 are the ergonomic foundation and unblock everything else. Plans 3–10 add
Expand Down
4 changes: 4 additions & 0 deletions src/Kevsoft.WLED/Kevsoft.WLED.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
<None Include="../../icon.png" Pack="true" Visible="false" PackagePath="" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Kevsoft.WLED.Tests" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="System.Net.Http.Json" Version="6.0.0" />
<PackageReference Include="System.Text.Json" Version="8.0.5" />
Expand Down
21 changes: 20 additions & 1 deletion src/Kevsoft.WLED/WLedClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public WLedClient(HttpMessageHandler httpMessageHandler, string baseUri)
{
}

public WLedClient(string baseUri) : this(new HttpClientHandler(), baseUri)
public WLedClient(string baseUri) : this(CreateDefaultHandler(), baseUri)
{
}

Expand Down Expand Up @@ -38,6 +38,25 @@ private static HttpClient CreateClient(HttpMessageHandler httpMessageHandler, st
return client;
}

/// <summary>
/// Creates the default <see cref="HttpMessageHandler"/> used when the client owns its own
/// <see cref="HttpClient"/>. On modern runtimes this is a <c>SocketsHttpHandler</c> with a bounded
/// <c>PooledConnectionLifetime</c> so pooled connections — and therefore DNS — are refreshed
/// periodically. See
/// https://learn.microsoft.com/dotnet/fundamentals/networking/http/httpclient-guidelines#dns-behavior.
/// </summary>
internal static HttpMessageHandler CreateDefaultHandler()
{
#if NETSTANDARD2_0
return new HttpClientHandler();
#else
return new SocketsHttpHandler
{
PooledConnectionLifetime = TimeSpan.FromMinutes(2)
};
#endif
}

public Task<WLedRootResponse> Get(CancellationToken cancellationToken = default)
=> GetJson<WLedRootResponse>("json", cancellationToken);

Expand Down
25 changes: 25 additions & 0 deletions test/Kevsoft.WLED.Tests/WLedClientConstructionTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
using System.Net.Http;

namespace Kevsoft.WLED.Tests;

public class WLedClientConstructionTests
{
[Fact]
public void StringConstructorSucceeds()
{
var act = () => new WLedClient("http://wled.local/");

act.Should().NotThrow();
}

#if !NETSTANDARD2_0
[Fact]
public void DefaultHandlerRefreshesPooledConnectionsToAvoidStaleDns()
{
var handler = WLedClient.CreateDefaultHandler();

handler.Should().BeOfType<SocketsHttpHandler>()
.Which.PooledConnectionLifetime.Should().Be(TimeSpan.FromMinutes(2));
}
#endif
}
Loading