Skip to content

Commit 08cc1b1

Browse files
test(audience): cancellation exits FlushAsync and SendBatchAsync cleanly
Two regression guards for PR #701 review from @nattb8. SendBatchAsync_CallerCancelled_Throws: pre-cancel the token, confirm the method throws OperationCanceledException, confirm the batch stays on disk, confirm no backoff and no onError. Sabotage: re-add the empty catch body and this fails because SendBatchAsync returns true silently. FlushAsync_CancelledToken_Terminates_DoesNotHotLoop: pre-cancel the token, start FlushAsync, race against a 2s timeout. With the fix the task faults quickly; without it the task never completes. Also flips the handler to 200 and runs a follow-up FlushAsync to prove _sendInFlight was released (the finally block didn't get stranded). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b07fd40 commit 08cc1b1

2 files changed

Lines changed: 84 additions & 0 deletions

File tree

src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,63 @@ public void FlushAsync_ConcurrentCallers_OnlyOneReachesTransport()
927927
"both FlushAsync calls should complete after release");
928928
}
929929

930+
[Test]
931+
public async Task FlushAsync_CancelledToken_Terminates_DoesNotHotLoop()
932+
{
933+
// Regression for PR #701 review (@nattb8): if SendBatchAsync
934+
// silently swallowed caller cancellation, the inner while-loop
935+
// here would re-enter on the same cancelled token and spin
936+
// because the batch is never deleted on that code path. The
937+
// task below would never complete. After the fix, cancellation
938+
// propagates and the task faults quickly.
939+
var handler = new CancellingHandler();
940+
var config = MakeConfig();
941+
config.HttpHandler = handler;
942+
943+
ImmutableAudience.Init(config);
944+
ImmutableAudience.Track("event_to_send");
945+
ImmutableAudience.FlushQueueToDiskForTesting();
946+
947+
using var cts = new CancellationTokenSource();
948+
cts.Cancel();
949+
950+
var flush = ImmutableAudience.FlushAsync(cts.Token);
951+
var finishedFirst = await Task.WhenAny(flush, Task.Delay(TimeSpan.FromSeconds(2)));
952+
953+
Assert.AreSame(flush, finishedFirst,
954+
"FlushAsync must terminate (not hot-loop) when the token is cancelled");
955+
Assert.IsTrue(flush.IsCanceled || flush.IsFaulted,
956+
"FlushAsync must propagate the cancellation, not return normally");
957+
Assert.LessOrEqual(handler.CallCount, 1,
958+
"a cancelled token must not drive repeated SendAsync attempts");
959+
960+
// Gate must be released by the finally block — a follow-up flush
961+
// on an uncancelled token should proceed, proving _sendInFlight
962+
// is not stranded at 1.
963+
handler.AcceptNextAsSuccess = true;
964+
var followUp = ImmutableAudience.FlushAsync();
965+
Assert.IsTrue(followUp.Wait(TimeSpan.FromSeconds(2)),
966+
"_sendInFlight must be released after a cancelled flush");
967+
}
968+
969+
private class CancellingHandler : HttpMessageHandler
970+
{
971+
public int CallCount;
972+
public bool AcceptNextAsSuccess;
973+
974+
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken ct)
975+
{
976+
Interlocked.Increment(ref CallCount);
977+
ct.ThrowIfCancellationRequested();
978+
var status = AcceptNextAsSuccess ? HttpStatusCode.OK : HttpStatusCode.ServiceUnavailable;
979+
var body = AcceptNextAsSuccess ? "{\"accepted\":1,\"rejected\":0}" : "";
980+
return Task.FromResult(new HttpResponseMessage(status)
981+
{
982+
Content = new StringContent(body)
983+
});
984+
}
985+
}
986+
930987
private class BlockingHandler : HttpMessageHandler
931988
{
932989
public readonly ManualResetEventSlim EnteredSendAsync = new ManualResetEventSlim(false);

src/Packages/Audience/Tests/Runtime/Transport/HttpTransportTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,33 @@ public async Task SendBatchAsync_HttpClientTimeout_TreatedAsNetworkError()
396396
Assert.AreEqual(AudienceErrorCode.NetworkError, reportedError.Code);
397397
}
398398

399+
[Test]
400+
public void SendBatchAsync_CallerCancelled_Throws_DoesNotDeleteOrRecordFailure()
401+
{
402+
// Regression guard for PR #701 review: caller cancellation must
403+
// propagate. If the `when (ct.IsCancellationRequested)` branch
404+
// swallowed the exception, SendBatchAsync would return `true`
405+
// with the batch still on disk, and a FlushAsync loop watching
406+
// that return value would re-enter on the same cancelled token
407+
// forever — nothing ever drains, nothing ever throws.
408+
_store.Write("{\"type\":\"track\"}");
409+
410+
var handler = new MockHandler(() => throw new OperationCanceledException("simulated"));
411+
AudienceError reportedError = null;
412+
using var transport = new HttpTransport(_store, "pk_imapik-test-key1",
413+
onError: e => reportedError = e, handler: handler);
414+
415+
using var cts = new CancellationTokenSource();
416+
cts.Cancel();
417+
418+
Assert.ThrowsAsync<OperationCanceledException>(
419+
async () => await transport.SendBatchAsync(cts.Token));
420+
421+
Assert.AreEqual(1, _store.Count(), "cancelled send must not delete the batch");
422+
Assert.IsFalse(transport.IsInBackoffWindow, "cancel is not a failure — no backoff engaged");
423+
Assert.IsNull(reportedError, "cancel is caller-initiated — no onError fires");
424+
}
425+
399426
[Test]
400427
public async Task IsInBackoffWindow_ClearsAfterNextAttemptAtElapses()
401428
{

0 commit comments

Comments
 (0)