Skip to content

Commit cbb345f

Browse files
eordanoclaude
andauthored
fix: guard null scene name in scene-JS Sentry fingerprint and drop review-referencing test comment
SceneShortInfo.Name is declared non-nullable but a default(SceneShortInfo) (no sceneShortInfo passed to ReportData) yields Name == null, since C# does not enforce non-null defaults on struct fields. AddSceneJsFingerprint forwarded that null straight into Scope.SetFingerprint's array instead of handling the missing-scene case, and the accompanying test locked the null passthrough in as intended behavior rather than exercising a real fallback. Guard with a named UNKNOWN_SCENE_NAME constant so a missing scene never ships a null fingerprint segment, and update the test to assert the fallback value. Also removes an AI-narration comment in SentryReportHandlerShould.cs that referenced this diff's own review document by section number instead of describing the test's behavior, replacing it with a plain rationale for why the scene name is null in that scenario. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018c638dR1vPysCMbYt2qQg5
1 parent 957f65f commit cbb345f

2 files changed

Lines changed: 4 additions & 5 deletions

File tree

Explorer/Assets/DCL/PerformanceAndDiagnostics/Diagnostics/ReportsHandling/Sentry/SentryReportHandler.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ public class SentryReportHandler : ReportHandlerBase
1616
public delegate void ConfigureScope(Scope scope);
1717

1818
private static readonly TimeSpan SESSION_FLUSH_TIMEOUT = TimeSpan.FromSeconds(2);
19+
private const string UNKNOWN_SCENE_NAME = "unknown-scene";
1920

2021
private readonly List<ConfigureScope> scopeConfigurators = new (10);
2122

@@ -184,7 +185,7 @@ internal static void AddSceneJsFingerprint(Scope scope, in ReportData data, stri
184185
if (string.IsNullOrEmpty(exceptionMessage))
185186
return;
186187

187-
scope.SetFingerprint(new[] { "scene-js", data.SceneShortInfo.Name, FirstLine(exceptionMessage) });
188+
scope.SetFingerprint(new[] { "scene-js", data.SceneShortInfo.Name ?? UNKNOWN_SCENE_NAME, FirstLine(exceptionMessage) });
188189
}
189190

190191
private static string FirstLine(string message)

Explorer/Assets/DCL/PerformanceAndDiagnostics/Diagnostics/ReportsHandling/Tests/SentryReportHandlerShould.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,15 @@ public void NotSetFingerprintForNonJavaScriptCategory()
4848
}
4949

5050
[Test]
51-
public void SetFingerprintWithNullSceneNameWhenSceneShortInfoIsMissing()
51+
public void SetFingerprintWithFallbackSceneNameWhenSceneShortInfoIsMissing()
5252
{
5353
// No sceneShortInfo supplied -> default(SceneShortInfo), Name == null.
54-
// The patch does not special-case a missing scene: the null flows straight
55-
// into the fingerprint array (documented residual risk in review.md #4).
5654
Scope scope = NewScope();
5755
var reportData = new ReportData(ReportCategory.JAVASCRIPT);
5856

5957
SentryReportHandler.AddSceneJsFingerprint(scope, reportData, "Error: boom");
6058

61-
CollectionAssert.AreEqual(new[] { "scene-js", null, "Error: boom" }, scope.Fingerprint);
59+
CollectionAssert.AreEqual(new[] { "scene-js", "unknown-scene", "Error: boom" }, scope.Fingerprint);
6260
}
6361
}
6462
}

0 commit comments

Comments
 (0)