fix(battle-node): BattleSession crash safety, fresh-key per push, phase guards
- Wrap HandleMsgEventAsync / HandleAliveEventAsync bodies in try/catch(Exception) logging at Error, eliminating async-void unobserved-exception crash risk (Issue 1). - Replace deterministic seq-based key generator with RandomNumberGenerator.GetInt32 so each EncodeAndSendAsync call uses a fresh random key (Issue 2). - Add `when Phase == …` guards to InitNetwork / Loaded / Swap cases in ComputeResponses; add default arm that logs+drops out-of-order URIs (Issue 3). - Widen SendSioAckAsync arg from int to long; drop (int) cast at call site; boundary cast to int is now checked() for defensive overflow detection (Issue 4). - Update RunAsync doc comment (was stale Task-13 placeholder) (Issue 5). - Add Kill and out-of-order-Swap-before-Loaded tests (Issue 6). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
using System.Net.WebSockets;
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using SVSim.BattleNode.Lifecycle;
|
||||
@@ -31,8 +32,7 @@ public sealed class BattleSession
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Send the EIO3 open handshake then enter the read loop. Returns when the WS closes.
|
||||
/// Dispatch is a no-op in this task; Task 13 fills it in.
|
||||
/// Send the EIO3 open handshake then run the read loop until the WS closes.
|
||||
/// </summary>
|
||||
public async Task RunAsync(CancellationToken cancellation)
|
||||
{
|
||||
@@ -103,57 +103,71 @@ public sealed class BattleSession
|
||||
|
||||
private async Task HandleMsgEventAsync(SocketIoFrame frame)
|
||||
{
|
||||
MsgEnvelope env;
|
||||
try { env = MsgPayloadCodec.Decode(frame.BinaryAttachments[0]); }
|
||||
catch (Exception ex)
|
||||
try
|
||||
{
|
||||
_log.LogWarning(ex, "BattleSession {Bid}: failed to decode msg envelope", BattleId);
|
||||
return;
|
||||
}
|
||||
|
||||
// Ack tracking + dedupe.
|
||||
bool shouldDispatch = true;
|
||||
if (env.PubSeq.HasValue)
|
||||
{
|
||||
shouldDispatch = Inbound.Observe(env.PubSeq.Value);
|
||||
if (frame.AckId.HasValue)
|
||||
MsgEnvelope env;
|
||||
try { env = MsgPayloadCodec.Decode(frame.BinaryAttachments[0]); }
|
||||
catch (Exception ex)
|
||||
{
|
||||
await SendSioAckAsync(frame.AckId.Value, (int)env.PubSeq.Value);
|
||||
_log.LogWarning(ex, "BattleSession {Bid}: failed to decode msg envelope", BattleId);
|
||||
return;
|
||||
}
|
||||
|
||||
// Ack tracking + dedupe.
|
||||
bool shouldDispatch = true;
|
||||
if (env.PubSeq.HasValue)
|
||||
{
|
||||
shouldDispatch = Inbound.Observe(env.PubSeq.Value);
|
||||
if (frame.AckId.HasValue)
|
||||
{
|
||||
await SendSioAckAsync(frame.AckId.Value, env.PubSeq.Value);
|
||||
}
|
||||
}
|
||||
if (!shouldDispatch) return;
|
||||
|
||||
// Run the pure-logic decision and drive sends.
|
||||
var responses = ComputeResponses(env);
|
||||
foreach (var (responseEnv, noStock) in responses)
|
||||
{
|
||||
if (noStock)
|
||||
await PushNoStockAsync(responseEnv);
|
||||
else
|
||||
await PushOrderedAsync(responseEnv);
|
||||
}
|
||||
}
|
||||
if (!shouldDispatch) return;
|
||||
|
||||
// Run the pure-logic decision and drive sends.
|
||||
var responses = ComputeResponses(env);
|
||||
foreach (var (responseEnv, noStock) in responses)
|
||||
catch (Exception ex)
|
||||
{
|
||||
if (noStock)
|
||||
await PushNoStockAsync(responseEnv);
|
||||
else
|
||||
await PushOrderedAsync(responseEnv);
|
||||
_log.LogError(ex, "BattleSession {Bid}: unhandled exception in HandleMsgEventAsync", BattleId);
|
||||
}
|
||||
}
|
||||
|
||||
private async Task HandleAliveEventAsync(SocketIoFrame frame)
|
||||
{
|
||||
// Client emits Gungnir every 5s with an SIO ack callback expecting just liveness confirmation
|
||||
// (payload ignored). We ack immediately, then push our own alive back with scs/ocs ONLINE
|
||||
// placeholders — the only response the client uses to drive its scs/ocs state machine.
|
||||
if (frame.AckId.HasValue)
|
||||
try
|
||||
{
|
||||
await SendSioAckAsync(frame.AckId.Value, 0);
|
||||
// Client emits Gungnir every 5s with an SIO ack callback expecting just liveness confirmation
|
||||
// (payload ignored). We ack immediately, then push our own alive back with scs/ocs ONLINE
|
||||
// placeholders — the only response the client uses to drive its scs/ocs state machine.
|
||||
if (frame.AckId.HasValue)
|
||||
{
|
||||
await SendSioAckAsync(frame.AckId.Value, 0);
|
||||
}
|
||||
var aliveEnv = new MsgEnvelope(
|
||||
Uri: NetworkBattleUri.Gungnir,
|
||||
ViewerId: ScriptedLifecycle.FakeOpponentViewerId,
|
||||
Uuid: "node-stub",
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
Cat: EmitCategory.General,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
Body: Gungnir.BuildAlivePushBody());
|
||||
await PushNoStockAsync(aliveEnv, eventName: "alive");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_log.LogError(ex, "BattleSession {Bid}: unhandled exception in HandleAliveEventAsync", BattleId);
|
||||
}
|
||||
var aliveEnv = new MsgEnvelope(
|
||||
Uri: NetworkBattleUri.Gungnir,
|
||||
ViewerId: ScriptedLifecycle.FakeOpponentViewerId,
|
||||
Uuid: "node-stub",
|
||||
Bid: null,
|
||||
Try: 0,
|
||||
Cat: EmitCategory.General,
|
||||
PubSeq: null,
|
||||
PlaySeq: null,
|
||||
Body: Gungnir.BuildAlivePushBody());
|
||||
await PushNoStockAsync(aliveEnv, eventName: "alive");
|
||||
}
|
||||
|
||||
internal IReadOnlyList<(MsgEnvelope Envelope, bool NoStock)> ComputeResponses(MsgEnvelope env)
|
||||
@@ -161,26 +175,31 @@ public sealed class BattleSession
|
||||
var result = new List<(MsgEnvelope Envelope, bool NoStock)>();
|
||||
switch (env.Uri)
|
||||
{
|
||||
case NetworkBattleUri.InitNetwork:
|
||||
case NetworkBattleUri.InitNetwork when Phase == BattleSessionPhase.AwaitingInitNetwork:
|
||||
result.Add((BuildAckedEnvelope(NetworkBattleUri.InitNetwork), NoStock: true));
|
||||
result.Add((ScriptedLifecycle.BuildMatched(ViewerId, ScriptedLifecycle.FakeOpponentViewerId, BattleId), NoStock: false));
|
||||
result.Add((ScriptedLifecycle.BuildBattleStart(ViewerId), NoStock: false));
|
||||
Phase = BattleSessionPhase.AwaitingLoaded;
|
||||
break;
|
||||
case NetworkBattleUri.Loaded:
|
||||
case NetworkBattleUri.Loaded when Phase == BattleSessionPhase.AwaitingLoaded:
|
||||
result.Add((ScriptedLifecycle.BuildDeal(), NoStock: false));
|
||||
Phase = BattleSessionPhase.AwaitingSwap;
|
||||
break;
|
||||
case NetworkBattleUri.Swap:
|
||||
case NetworkBattleUri.Swap when Phase == BattleSessionPhase.AwaitingSwap:
|
||||
result.Add((ScriptedLifecycle.BuildSwapResponse(ExtractIdxList(env)), NoStock: false));
|
||||
result.Add((ScriptedLifecycle.BuildReady(), NoStock: false));
|
||||
Phase = BattleSessionPhase.AfterReady;
|
||||
break;
|
||||
case NetworkBattleUri.Retire:
|
||||
case NetworkBattleUri.Kill:
|
||||
// These always terminate, regardless of phase.
|
||||
result.Add((BuildBattleFinishNoContest(), NoStock: true));
|
||||
Phase = BattleSessionPhase.Terminal;
|
||||
break;
|
||||
default:
|
||||
// Out-of-order or unknown URI: log and drop (no response).
|
||||
_log.LogDebug("BattleSession {Bid}: dropping uri={Uri} in phase={Phase}", BattleId, env.Uri, Phase);
|
||||
break;
|
||||
}
|
||||
return result;
|
||||
}
|
||||
@@ -222,8 +241,7 @@ public sealed class BattleSession
|
||||
|
||||
private async Task EncodeAndSendAsync(MsgEnvelope env, string eventName)
|
||||
{
|
||||
var seq = 0;
|
||||
var key = NodeCrypto.GenerateKey(() => (seq++ * 11) % 16);
|
||||
var key = NodeCrypto.GenerateKey(() => RandomNumberGenerator.GetInt32(0, 16));
|
||||
var bytes = MsgPayloadCodec.Encode(env, key);
|
||||
var sio = SocketIoFrame.BinaryEventWithAttachments(eventName, new[] { bytes });
|
||||
var (text, bins) = sio.Encode();
|
||||
@@ -233,9 +251,10 @@ public sealed class BattleSession
|
||||
await _ws.SendAsync(bin, WebSocketMessageType.Binary, endOfMessage: true, CancellationToken.None);
|
||||
}
|
||||
|
||||
private async Task SendSioAckAsync(int ackId, int arg)
|
||||
private async Task SendSioAckAsync(int ackId, long arg)
|
||||
{
|
||||
var ack = SocketIoFrame.AckResponse(ackId, arg);
|
||||
// checked() ensures we'd notice if pubSeq ever exceeded int.MaxValue (not realistic but defensive).
|
||||
var ack = SocketIoFrame.AckResponse(ackId, checked((int)arg));
|
||||
var (text, _) = ack.Encode();
|
||||
var eioText = $"{(int)EngineIoPacketType.Message}{text}";
|
||||
await SendTextAsync(eioText, CancellationToken.None);
|
||||
|
||||
@@ -61,4 +61,25 @@ public class BattleSessionDispatchTests
|
||||
Assert.That(noStock, Is.True);
|
||||
Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.Terminal));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Kill_PushesBattleFinishNoContest_TransitionsToTerminal()
|
||||
{
|
||||
var s = NewSession();
|
||||
var responses = s.ComputeResponses(NewEnvelope(NetworkBattleUri.Kill));
|
||||
var (env, noStock) = responses.Single();
|
||||
Assert.That(env.Uri, Is.EqualTo(NetworkBattleUri.BattleFinish));
|
||||
Assert.That(noStock, Is.True);
|
||||
Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.Terminal));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void Swap_ArrivingBeforeLoaded_ProducesNoResponseAndDoesNotAdvancePhase()
|
||||
{
|
||||
var s = NewSession();
|
||||
// Skip Loaded — fire Swap straight out of AwaitingInitNetwork.
|
||||
var responses = s.ComputeResponses(NewEnvelope(NetworkBattleUri.Swap));
|
||||
Assert.That(responses, Is.Empty);
|
||||
Assert.That(s.Phase, Is.EqualTo(BattleSessionPhase.AwaitingInitNetwork));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user