Skip to content

Commit f1540af

Browse files
ImmutableJeffreyclaude
ImmutableJeffrey
and
claude
committed
refactor(audience-sdk): centralise log message text in AudienceLogs (SDK-272)
Adds Runtime/Utility/AudienceLogs.cs as the single source of truth for runtime log message strings. Runtime callers in ImmutableAudience.cs, Session.cs, and HttpTransport.cs now read from AudienceLogs constants and methods instead of inline literals, and the test assertions read from the same place so wording changes touch one file. - 23 call sites migrated across ImmutableAudience, Session, and HttpTransport. - Test assertions in ImmutableAudienceTests, PublishableKeyPrefixTests, and SessionTests now reference AudienceLogs constants directly. - HttpTransport.NotifyError and ImmutableAudience.NotifyErrorCallback now share the OnErrorThrew template; HttpTransport's prior wording carried an incidental underscore (_onError) from the field name and is dropped during the migration. Both situations are "the consumer's error handler threw inside our notify path" so they read from one constant. - Three negative-path tests in PublishableKeyPrefixTests previously asserted that no key-mismatch warning fired by checking lines for the substring "BaseUrl". The substring is incidental to the message wording; an unrelated future log line that mentions "BaseUrl" would silently trip the test. Switched to assert against the actual AudienceLogs.TestKeyAgainstProduction and AudienceLogs.NonTestKeyAgainstSandbox constants. No code behaviour changes; wire format and OnError contract are untouched. dotnet test reports 278 passed, 0 failed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1fef585 commit f1540af

8 files changed

Lines changed: 166 additions & 49 deletions

File tree

‎src/Packages/Audience/Runtime/Core/Session.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ internal void Pause()
112112
// when End fires while paused), over-crediting engagement.
113113
if (_pausedAt.HasValue)
114114
{
115-
Log.Debug("Session: Pause while already paused. Ignoring.");
115+
Log.Debug(AudienceLogs.SessionPauseAlreadyPaused);
116116
return;
117117
}
118118
_pausedAt = _getUtcNow();
@@ -259,7 +259,7 @@ private void SafeTrack(string eventName, Dictionary<string, object> properties)
259259
}
260260
catch (Exception ex)
261261
{
262-
Log.Warn($"Session: {eventName} track callback threw {ex.GetType().Name}. Event dropped.");
262+
Log.Warn(AudienceLogs.SessionTrackCallbackThrew(eventName, ex));
263263
}
264264
}
265265

@@ -277,8 +277,7 @@ private void DrainHeartbeatTimer()
277277

278278
if (!TimerDisposal.DisposeAndWait(timer, TimeSpan.FromSeconds(1)))
279279
{
280-
Log.Warn("Session: heartbeat callback did not complete within 1s on timer stop. " +
281-
"A trailing session_heartbeat may race with the next session lifecycle event.");
280+
Log.Warn(AudienceLogs.SessionHeartbeatTimeout);
282281
}
283282
}
284283

‎src/Packages/Audience/Runtime/ImmutableAudience.cs‎

Lines changed: 18 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,7 @@ public static void Init(AudienceConfig config)
159159
{
160160
if (_initialized)
161161
{
162-
Log.Warn("Init called more than once. Ignoring; original config retained. " +
163-
"Call Shutdown() first if reconfiguring is intended.");
162+
Log.Warn(AudienceLogs.InitCalledTwice);
164163
return;
165164
}
166165

@@ -237,7 +236,7 @@ public static void Track(IEvent evt)
237236
if (!_initialized || !state.Level.CanTrack()) return;
238237
if (evt == null)
239238
{
240-
Log.Warn("Track(IEvent) called with null event. Dropping.");
239+
Log.Warn(AudienceLogs.TrackIEventNull);
241240
return;
242241
}
243242

@@ -254,13 +253,13 @@ public static void Track(IEvent evt)
254253
}
255254
catch (Exception ex)
256255
{
257-
Log.Warn($"Track(IEvent): {evt.GetType().Name}.ToProperties()/EventName threw {ex.GetType().Name}: {ex.Message}. Dropping.");
256+
Log.Warn(AudienceLogs.TrackIEventThrew(evt.GetType().Name, ex));
258257
return;
259258
}
260259

261260
if (string.IsNullOrEmpty(eventName))
262261
{
263-
Log.Warn($"Track(IEvent): {evt.GetType().Name}.EventName returned null or empty. Dropping.");
262+
Log.Warn(AudienceLogs.TrackIEventEmptyName(evt.GetType().Name));
264263
return;
265264
}
266265

@@ -284,7 +283,7 @@ public static void Track(string eventName, Dictionary<string, object>? propertie
284283
if (!_initialized || !state.Level.CanTrack()) return;
285284
if (string.IsNullOrEmpty(eventName))
286285
{
287-
Log.Warn("Track(string) called with null or empty event name. Dropping.");
286+
Log.Warn(AudienceLogs.TrackStringEmptyName);
288287
return;
289288
}
290289

@@ -315,7 +314,7 @@ public static void Identify(string userId, IdentityType identityType, Dictionary
315314
// Validate inputs before consent so null-arg callers get the right warning.
316315
if (string.IsNullOrEmpty(userId))
317316
{
318-
Log.Warn("Identify called with null or empty userId. Dropping.");
317+
Log.Warn(AudienceLogs.IdentifyEmptyUserId);
319318
return;
320319
}
321320

@@ -330,7 +329,7 @@ public static void Identify(string userId, IdentityType identityType, Dictionary
330329
level = current.Level;
331330
if (!level.CanIdentify())
332331
{
333-
Log.Warn($"Identify discarded. Requires Full consent, current is {level}.");
332+
Log.Warn(AudienceLogs.IdentifyDiscarded(level));
334333
return;
335334
}
336335
config = _config;
@@ -357,13 +356,13 @@ public static void Alias(string fromId, IdentityType fromType, string toId, Iden
357356

358357
if (string.IsNullOrEmpty(fromId) || string.IsNullOrEmpty(toId))
359358
{
360-
Log.Warn("Alias called with null or empty fromId/toId. Dropping.");
359+
Log.Warn(AudienceLogs.AliasEmptyIds);
361360
return;
362361
}
363362
var state = _state;
364363
if (!state.Level.CanIdentify())
365364
{
366-
Log.Warn($"Alias discarded. Requires Full consent, current is {state.Level}.");
365+
Log.Warn(AudienceLogs.AliasDiscarded(state.Level));
367366
return;
368367
}
369368

@@ -486,7 +485,7 @@ private static void NotifyErrorCallback(Action<AudienceError>? onError, Audience
486485
}
487486
catch (Exception ex)
488487
{
489-
Log.Warn($"onError threw {ex.GetType().Name}: {ex.Message}");
488+
Log.Warn(AudienceLogs.OnErrorThrew(ex));
490489
}
491490
}
492491

@@ -579,8 +578,7 @@ public static void SetConsent(ConsentLevel level)
579578
}
580579
catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException)
581580
{
582-
Log.Warn($"SetConsent: failed to persist consent level. {ex.GetType().Name}: {ex.Message}. " +
583-
"In-memory level is updated but will revert on next launch.");
581+
Log.Warn(AudienceLogs.ConsentPersistFailed(ex));
584582
NotifyErrorCallback(config.OnError, AudienceErrorCode.ConsentPersistFailed,
585583
$"Consent persist failed: {ex.Message}");
586584
}
@@ -802,13 +800,12 @@ public static void Shutdown()
802800
var send = transport.SendBatchAsync();
803801
if (!send.Wait(timeoutMs))
804802
{
805-
Log.Warn($"Shutdown flush exceeded {timeoutMs}ms. Abandoning. " +
806-
"Queued events remain on disk and will retry on next startup.");
803+
Log.Warn(AudienceLogs.ShutdownFlushExceeded(timeoutMs));
807804
}
808805
}
809806
catch (Exception ex)
810807
{
811-
Log.Warn($"Shutdown flush threw: {ex.GetType().Name}: {ex.Message}");
808+
Log.Warn(AudienceLogs.ShutdownFlushThrew(ex));
812809
}
813810
}
814811

@@ -870,17 +867,11 @@ private static void WarnIfKeyEnvironmentMismatch(string publishableKey, string?
870867

871868
if (isTestKey && trimmed == Constants.ProductionBaseUrl)
872869
{
873-
Log.Warn(
874-
$"Publishable key has the test prefix ({Constants.TestKeyPrefix}) but BaseUrl points to production. " +
875-
"The backend will reject events with 401. Either remove the BaseUrl override (test keys " +
876-
"default to sandbox) or use a non-test publishable key.");
870+
Log.Warn(AudienceLogs.TestKeyAgainstProduction);
877871
}
878872
else if (!isTestKey && trimmed == Constants.SandboxBaseUrl)
879873
{
880-
Log.Warn(
881-
"Publishable key is not a test key but BaseUrl points to sandbox. " +
882-
"The backend will reject events with 401. Either remove the BaseUrl override (non-test " +
883-
$"keys default to production) or use a test publishable key ({Constants.TestKeyPrefix}).");
874+
Log.Warn(AudienceLogs.NonTestKeyAgainstSandbox);
884875
}
885876
}
886877

@@ -922,8 +913,7 @@ private static void MergeUnityContext(Dictionary<string, object>? msg)
922913
}
923914
catch (Exception ex)
924915
{
925-
Log.Warn($"ContextProvider threw {ex.GetType().Name}: {ex.Message}. " +
926-
"Event ships with base context only.");
916+
Log.Warn(AudienceLogs.ContextProviderThrew(ex));
927917
return;
928918
}
929919
if (extra == null) return;
@@ -959,7 +949,7 @@ private static void SendBatch()
959949
catch (Exception ex)
960950
{
961951
// Timer-thread callback; no caller above to catch.
962-
Log.Warn($"SendBatch unexpected exception: {ex.GetType().Name}: {ex.Message}");
952+
Log.Warn(AudienceLogs.SendBatchUnexpected(ex));
963953
}
964954
}
965955

@@ -1007,8 +997,7 @@ private static void FireGameLaunch(AudienceConfig config, ConsentLevel consentAt
1007997
try { unityContext = provider(); }
1008998
catch (Exception ex)
1009999
{
1010-
Log.Warn($"LaunchContextProvider threw {ex.GetType().Name}: {ex.Message}. " +
1011-
"game_launch will ship without auto-detected Unity context.");
1000+
Log.Warn(AudienceLogs.LaunchContextProviderThrew(ex));
10121001
}
10131002

10141003
if (unityContext != null)

‎src/Packages/Audience/Runtime/Transport/HttpTransport.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ private static async Task<int> ParseRejectedCount(HttpResponseMessage response,
288288
}
289289
catch (Exception ex)
290290
{
291-
Log.Warn($"ParseRejectedCount threw {ex.GetType().Name}: {ex.Message}");
291+
Log.Warn(AudienceLogs.ParseRejectedCountThrew(ex));
292292
return 0;
293293
}
294294
if (string.IsNullOrEmpty(body)) return 0;
@@ -339,7 +339,7 @@ private void NotifyError(AudienceErrorCode code, string message)
339339
}
340340
catch (Exception ex)
341341
{
342-
Log.Warn($"_onError threw {ex.GetType().Name}: {ex.Message}");
342+
Log.Warn(AudienceLogs.OnErrorThrew(ex));
343343
}
344344
}
345345
}

‎src/Packages/Audience/Runtime/Utility/AudienceLogs.cs‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
#nullable enable
2+
3+
using System;
4+
5+
namespace Immutable.Audience
6+
{
7+
internal static class AudienceLogs
8+
{
9+
// ---- Init / config validation ----
10+
11+
internal const string InitCalledTwice =
12+
"Init called more than once. Ignoring; original config retained. " +
13+
"Call Shutdown() first if reconfiguring is intended.";
14+
15+
internal static readonly string TestKeyAgainstProduction =
16+
$"Publishable key has the test prefix ({Constants.TestKeyPrefix}) but BaseUrl points to production. " +
17+
"The backend will reject events with 401. Either remove the BaseUrl override (test keys " +
18+
"default to sandbox) or use a non-test publishable key.";
19+
20+
internal static readonly string NonTestKeyAgainstSandbox =
21+
"Publishable key is not a test key but BaseUrl points to sandbox. " +
22+
"The backend will reject events with 401. Either remove the BaseUrl override (non-test " +
23+
$"keys default to production) or use a test publishable key ({Constants.TestKeyPrefix}).";
24+
25+
// ---- Track ----
26+
27+
internal const string TrackIEventNull =
28+
"Track(IEvent) called with null event. Dropping.";
29+
30+
internal const string TrackStringEmptyName =
31+
"Track(string) called with null or empty event name. Dropping.";
32+
33+
internal static string TrackIEventThrew(string evtTypeName, Exception ex) =>
34+
$"Track(IEvent): {evtTypeName}.ToProperties()/EventName threw {ex.GetType().Name}: {ex.Message}. Dropping.";
35+
36+
internal static string TrackIEventEmptyName(string evtTypeName) =>
37+
$"Track(IEvent): {evtTypeName}.EventName returned null or empty. Dropping.";
38+
39+
// ---- Identify / Alias ----
40+
41+
internal const string IdentifyEmptyUserId =
42+
"Identify called with null or empty userId. Dropping.";
43+
44+
internal const string AliasEmptyIds =
45+
"Alias called with null or empty fromId/toId. Dropping.";
46+
47+
internal static string IdentifyDiscarded(ConsentLevel current) =>
48+
$"Identify discarded. Requires Full consent, current is {current}.";
49+
50+
internal static string AliasDiscarded(ConsentLevel current) =>
51+
$"Alias discarded. Requires Full consent, current is {current}.";
52+
53+
// ---- Consent / Shutdown ----
54+
55+
internal static string ConsentPersistFailed(Exception ex) =>
56+
$"SetConsent: failed to persist consent level. {ex.GetType().Name}: {ex.Message}. " +
57+
"In-memory level is updated but will revert on next launch.";
58+
59+
internal static string ShutdownFlushExceeded(int timeoutMs) =>
60+
$"Shutdown flush exceeded {timeoutMs}ms. Abandoning. " +
61+
"Queued events remain on disk and will retry on next startup.";
62+
63+
internal static string ShutdownFlushThrew(Exception ex) =>
64+
$"Shutdown flush threw: {ex.GetType().Name}: {ex.Message}";
65+
66+
// ---- onError handler swallow ----
67+
68+
internal static string OnErrorThrew(Exception ex) =>
69+
$"onError threw {ex.GetType().Name}: {ex.Message}";
70+
71+
// ---- Send loop ----
72+
73+
internal static string SendBatchUnexpected(Exception ex) =>
74+
$"SendBatch unexpected exception: {ex.GetType().Name}: {ex.Message}";
75+
76+
internal static string ParseRejectedCountThrew(Exception ex) =>
77+
$"ParseRejectedCount threw {ex.GetType().Name}: {ex.Message}";
78+
79+
// ---- Session ----
80+
81+
internal const string SessionPauseAlreadyPaused =
82+
"Session: Pause while already paused. Ignoring.";
83+
84+
internal const string SessionHeartbeatTimeout =
85+
"Session: heartbeat callback did not complete within 1s on timer stop. " +
86+
"A trailing session_heartbeat may race with the next session lifecycle event.";
87+
88+
internal static string SessionTrackCallbackThrew(string eventName, Exception ex) =>
89+
$"Session: {eventName} track callback threw {ex.GetType().Name}. Event dropped.";
90+
91+
// ---- Context providers ----
92+
93+
internal static string ContextProviderThrew(Exception ex) =>
94+
$"ContextProvider threw {ex.GetType().Name}: {ex.Message}. " +
95+
"Event ships with base context only.";
96+
97+
internal static string LaunchContextProviderThrew(Exception ex) =>
98+
$"LaunchContextProvider threw {ex.GetType().Name}: {ex.Message}. " +
99+
"game_launch will ship without auto-detected Unity context.";
100+
}
101+
}

‎src/Packages/Audience/Runtime/Utility/AudienceLogs.cs.meta‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Packages/Audience/Tests/Runtime/Core/SessionTests.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ void Track(string name, Dictionary<string, object> props)
499499

500500
lock (warnings)
501501
{
502-
Assert.IsTrue(warnings.Any(w => w.Contains("heartbeat callback did not complete")),
502+
Assert.IsTrue(warnings.Any(w => w.Contains(AudienceLogs.SessionHeartbeatTimeout)),
503503
"End must log the drain-timeout warning when an in-flight heartbeat exceeds 1 s");
504504
}
505505
}
@@ -557,7 +557,8 @@ void ThrowingTrack(string name, Dictionary<string, object> props)
557557

558558
lock (warnings)
559559
{
560-
Assert.IsTrue(warnings.Any(w => w.Contains("session_heartbeat track callback threw")),
560+
var expected = AudienceLogs.SessionTrackCallbackThrew("session_heartbeat", new InvalidOperationException());
561+
Assert.IsTrue(warnings.Any(w => w.Contains(expected)),
561562
"SafeTrack must log a warning when the callback throws");
562563
}
563564
}
@@ -590,7 +591,8 @@ void ThrowingTrack(string name, Dictionary<string, object> props)
590591

591592
lock (warnings)
592593
{
593-
Assert.IsTrue(warnings.Any(w => w.Contains("session_start track callback threw")),
594+
var expected = AudienceLogs.SessionTrackCallbackThrew("session_start", new InvalidOperationException());
595+
Assert.IsTrue(warnings.Any(w => w.Contains(expected)),
594596
"SafeTrack must log a warning when the Start callback throws");
595597
}
596598
}
@@ -625,7 +627,8 @@ void ThrowingTrack(string name, Dictionary<string, object> props)
625627

626628
lock (warnings)
627629
{
628-
Assert.IsTrue(warnings.Any(w => w.Contains("session_end track callback threw")),
630+
var expected = AudienceLogs.SessionTrackCallbackThrew("session_end", new InvalidOperationException());
631+
Assert.IsTrue(warnings.Any(w => w.Contains(expected)),
629632
"SafeTrack must log a warning when the End callback throws");
630633
}
631634
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ public void Track_NullEvent_DoesNotThrow_AndLogsWarning()
303303
try
304304
{
305305
Assert.DoesNotThrow(() => ImmutableAudience.Track((IEvent)null));
306-
Assert.That(lines, Has.Some.Contains("null event"));
306+
Assert.That(lines, Has.Some.Contains(AudienceLogs.TrackIEventNull));
307307
}
308308
finally { Log.Writer = null; }
309309
}
@@ -320,7 +320,9 @@ public void Track_IEventMissingRequiredField_DropsWithWarn()
320320
// Purchase with no Value set: ToProperties throws; Track must
321321
// catch, warn, and drop rather than ship an incomplete event.
322322
Assert.DoesNotThrow(() => ImmutableAudience.Track(new Purchase { Currency = "USD" }));
323-
Assert.That(lines, Has.Some.Contains("Purchase"));
323+
// Assert the stable parts (event-type name and trailing "Dropping")
324+
// so the test survives any change to the exception type or message.
325+
Assert.That(lines, Has.Some.Contains(nameof(Purchase)));
324326
Assert.That(lines, Has.Some.Contains("Dropping"));
325327
}
326328
finally { Log.Writer = null; }
@@ -430,7 +432,7 @@ public void Init_CalledTwice_LogsWarning()
430432
ImmutableAudience.Init(MakeConfig());
431433
ImmutableAudience.Init(MakeConfig());
432434

433-
Assert.That(lines, Has.Some.Contains("Init called more than once"),
435+
Assert.That(lines, Has.Some.Contains(AudienceLogs.InitCalledTwice),
434436
"second Init must surface a warning so a developer notices the silent no-op");
435437
}
436438
finally
@@ -467,7 +469,7 @@ public void Init_ConcurrentCalls_OnlyOneSucceeds_OthersWarn()
467469

468470
foreach (var t in threads) t.Join(TimeSpan.FromSeconds(5));
469471

470-
var warningCount = lines.Count(l => l.Contains("Init called more than once"));
472+
var warningCount = lines.Count(l => l.Contains(AudienceLogs.InitCalledTwice));
471473
Assert.AreEqual(threadCount - 1, warningCount,
472474
"exactly one thread should initialise; the other (threadCount - 1) should hit the duplicate-call warning branch");
473475
}

0 commit comments

Comments
 (0)