Skip to content

Commit 4651797

Browse files
committed
fix(network): tolerate slp ping failures
1 parent fe1f99a commit 4651797

4 files changed

Lines changed: 131 additions & 8 deletions

File tree

MCServerLauncher.Common/Network/SlpClient.cs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,27 @@ public class SlpClient
2626
{
2727
if (port < 0 || port > 65535) throw new ArgumentOutOfRangeException(nameof(port));
2828

29-
var client = new SlpClient();
30-
if (!await client.HandShakeAsync(host, (ushort)port, cancellationToken)) return null;
29+
try
30+
{
31+
var client = new SlpClient();
32+
if (!await client.HandShakeAsync(host, (ushort)port, cancellationToken)) return null;
3133

32-
var payload = await client.GetSlpAsync(cancellationToken);
33-
var latency = await client.GetLatencyAsync(cancellationToken);
34+
var payload = await client.GetSlpAsync(cancellationToken);
35+
if (payload is null) return null;
3436

35-
if (payload != null && latency != null) return new SlpStatus(payload, latency.Value);
36-
return null;
37+
var latency = await client.GetLatencyAsync(cancellationToken);
38+
39+
return latency != null ? new SlpStatus(payload, latency.Value) : null;
40+
}
41+
catch (OperationCanceledException)
42+
{
43+
throw;
44+
}
45+
catch (Exception e) when (e is SocketException or IOException)
46+
{
47+
Log.Debug(e, "[SlpClient] Modern server ping failed for {Host}:{Port}", host, port);
48+
return null;
49+
}
3750
}
3851

3952
/// <summary>

MCServerLauncher.Daemon/Management/Minecraft/MinecraftInstance.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
using System.Net.Sockets;
21
using MCServerLauncher.Common.Minecraft;
32
using MCServerLauncher.Common.Network;
43
using MCServerLauncher.Common.ProtoType.Instance;
4+
using Serilog;
55

66
namespace MCServerLauncher.Daemon.Management.Minecraft;
77

@@ -43,8 +43,13 @@ private async Task<Player[]> GetServerPlayersAsync(CancellationToken ct = defaul
4343
if (status != null)
4444
return status.Payload.Players.Sample.Select(player => new Player(player.Name, player.Id)).ToArray();
4545
}
46-
catch (Exception e)when (e is SocketException or ArgumentOutOfRangeException or ArgumentException)
46+
catch (OperationCanceledException) when (ct.IsCancellationRequested)
4747
{
48+
throw;
49+
}
50+
catch (Exception e)
51+
{
52+
Log.Debug(e, "[MinecraftInstance] Failed to query server player list for instance {InstanceId}", Config.Uuid);
4853
return [];
4954
}
5055

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System.Net;
2+
using System.Net.Sockets;
3+
using MCServerLauncher.Common.Network;
4+
5+
namespace MCServerLauncher.ProtocolTests;
6+
7+
public class SlpClientFailureToleranceTests
8+
{
9+
[Fact]
10+
public async Task GetStatusModern_InvalidStatusPayload_ReturnsNullWithoutLatencyWrite()
11+
{
12+
using var listener = new TcpListener(IPAddress.Loopback, 0);
13+
listener.Start();
14+
15+
var port = ((IPEndPoint)listener.LocalEndpoint).Port;
16+
var serverTask = Task.Run(async () =>
17+
{
18+
using var client = await listener.AcceptTcpClientAsync();
19+
client.LingerState = new LingerOption(true, 0);
20+
21+
using var stream = client.GetStream();
22+
var buffer = new byte[512];
23+
await stream.ReadAtLeastAsync(buffer, 1);
24+
25+
await stream.WriteAsync(new byte[] { 0x02, 0x00, 0x00 });
26+
});
27+
28+
var result = await SlpClient.GetStatusModern(IPAddress.Loopback.ToString(), port);
29+
30+
Assert.Null(result);
31+
await serverTask.WaitAsync(TimeSpan.FromSeconds(3));
32+
}
33+
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# SLP Report Failure Tolerance Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Prevent Minecraft SLP ping/player-list failures from breaking daemon instance report refresh.
6+
7+
**Architecture:** Treat Minecraft server-list ping as best-effort report enrichment. `SlpClient.GetStatusModern` should stop after an invalid status payload instead of reusing a failed connection for latency, and daemon Minecraft report generation should convert non-cancellation SLP failures into an empty player list.
8+
9+
**Tech Stack:** C# 14, .NET 10, System.Net.Sockets, xUnit protocol tests.
10+
11+
---
12+
13+
## File Structure
14+
15+
- Modify: `MCServerLauncher.Common/Network/SlpClient.cs`
16+
- Owns modern Minecraft SLP status and latency probing.
17+
- Modify: `MCServerLauncher.Daemon/Management/Minecraft/MinecraftInstance.cs`
18+
- Owns Minecraft-specific report enrichment for online players.
19+
- Create: `MCServerLauncher.ProtocolTests/SlpClientFailureToleranceTests.cs`
20+
- Verifies invalid SLP status payloads return `null` without a follow-up latency write.
21+
22+
## Task 1: Add SLP Failure Regression Test
23+
24+
- [x] **Step 1: Write the failing test**
25+
26+
Create `MCServerLauncher.ProtocolTests/SlpClientFailureToleranceTests.cs` with a loopback `TcpListener` that accepts the status request, sends an invalid/empty status response, then closes the connection. Assert `SlpClient.GetStatusModern` returns `null` instead of throwing.
27+
28+
- [x] **Step 2: Run test to verify it fails**
29+
30+
Run:
31+
32+
```powershell
33+
dotnet test MCServerLauncher.ProtocolTests/MCServerLauncher.ProtocolTests.csproj -c Release --filter "FullyQualifiedName~SlpClientFailureToleranceTests"
34+
```
35+
36+
Expected before implementation: FAIL due `IOException` when the client tries to write the latency packet after invalid status.
37+
38+
- [x] **Step 3: Implement minimal common-layer fix**
39+
40+
In `SlpClient.GetStatusModern`, return `null` immediately when `GetSlpAsync` returns `null`; only call `GetLatencyAsync` after a valid status payload.
41+
42+
- [x] **Step 4: Add daemon-side defense**
43+
44+
In `MinecraftInstance.GetServerPlayersAsync`, let caller-requested cancellation propagate and catch other SLP/report enrichment exceptions as an empty player list.
45+
46+
- [x] **Step 5: Verify focused test**
47+
48+
Run:
49+
50+
```powershell
51+
dotnet test MCServerLauncher.ProtocolTests/MCServerLauncher.ProtocolTests.csproj -c Release --filter "FullyQualifiedName~SlpClientFailureToleranceTests"
52+
```
53+
54+
Expected after implementation: PASS.
55+
56+
- [x] **Step 6: Verify relevant project and hygiene**
57+
58+
Run:
59+
60+
```powershell
61+
dotnet build MCServerLauncher.Daemon/MCServerLauncher.Daemon.csproj /m:1
62+
dotnet test MCServerLauncher.ProtocolTests/MCServerLauncher.ProtocolTests.csproj -c Release --no-build
63+
git diff --check
64+
```
65+
66+
Expected: commands exit 0.
67+
68+
## Changelog
69+
70+
- Added regression coverage for invalid Minecraft SLP status responses closing before latency ping.
71+
- Updated modern SLP probing to stop after invalid status payloads.
72+
- Hardened Minecraft instance report enrichment so player-list probe failures do not fail `GetAllReports`.

0 commit comments

Comments
 (0)