Skip to content

Commit b56d9b2

Browse files
LulalabyCopilot
andcommitted
fix: handle uncached role updates and report library faults
Handle GUILD_ROLE_UPDATE when the role is missing from cache by refreshing the cache entry from the payload instead of dereferencing null. Also allow library-origin exceptions through telemetry filtering so bugs like dispatch-time NullReferenceExceptions still reach Sentry even when they are not part of the explicit tracked-exception allowlist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d6f3641 commit b56d9b2

File tree

6 files changed

+160
-47
lines changed

6 files changed

+160
-47
lines changed

DisCatSharp.Tests/DisCatSharp.CopilotTests/Store/StoreGatewayDispatchFollowupRegressionTests.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,49 @@ public async Task HandleDispatchSafelyAsync_DispatchFailure_CapturesDiagnosticsS
354354
Assert.Equal(42, captured.Context["sequence"]);
355355
}
356356

357+
[Fact]
358+
public async Task GuildRoleUpdate_MissingRoleCache_DoesNotThrowAndRefreshesGuildRoleCache()
359+
{
360+
var client = CreateClient();
361+
const ulong guildId = 804032421678153819;
362+
const ulong roleId = 9001;
363+
364+
var guild = new DiscordGuild
365+
{
366+
Id = guildId,
367+
Discord = client
368+
};
369+
client.GuildsInternal[guildId] = guild;
370+
371+
GuildRoleUpdateEventArgs? captured = null;
372+
client.GuildRoleUpdated += (_, args) =>
373+
{
374+
captured = args;
375+
return Task.CompletedTask;
376+
};
377+
378+
var role = new DiscordRole
379+
{
380+
Id = roleId,
381+
Name = "mods",
382+
Discord = client,
383+
Permissions = Permissions.Administrator,
384+
Position = 7,
385+
IsMentionable = true
386+
};
387+
388+
var exception = await Record.ExceptionAsync(() => client.OnGuildRoleUpdateEventAsync(role, guild));
389+
390+
Assert.Null(exception);
391+
Assert.NotNull(captured);
392+
Assert.Equal(guildId, captured!.Guild.Id);
393+
Assert.Equal(roleId, captured.RoleAfter.Id);
394+
Assert.Equal("mods", captured.RoleAfter.Name);
395+
Assert.Equal(roleId, captured.RoleBefore.Id);
396+
Assert.Equal("mods", captured.RoleBefore.Name);
397+
Assert.Same(captured.RoleAfter, guild.GetRole(roleId));
398+
}
399+
357400
[Fact]
358401
public async Task HandleSocketMessageAsync_HeartbeatWithNullData_UsesLastKnownSequence()
359402
{

DisCatSharp.Tests/DisCatSharp.CopilotTests/Telemetry/SentryDiagnosticsSinkTests.cs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,35 @@ public void ExceptionFilter_AllowsWrappedTrackedException()
4848
Assert.False(filter.Filter(exception));
4949
}
5050

51+
[Fact]
52+
public void ExceptionFilter_AllowsLibraryOriginExceptionOutsideTrackedList()
53+
{
54+
var config = new DiscordConfiguration
55+
{
56+
EnableLibraryDeveloperMode = true
57+
};
58+
config.TrackExceptions = [typeof(DiscordJsonException)];
59+
var filter = new DisCatSharpExceptionFilter(config);
60+
var exception = new NullReferenceException("library bug");
61+
exception.Data[DiagnosticTags.ErrorOrigin] = DiagnosticTags.OriginLibrary;
62+
63+
Assert.False(filter.Filter(exception));
64+
}
65+
66+
[Fact]
67+
public void ShouldSendEvent_AllowsLibraryOriginExceptionOutsideTrackedList()
68+
{
69+
var config = new DiscordConfiguration
70+
{
71+
EnableLibraryDeveloperMode = true
72+
};
73+
config.TrackExceptions = [typeof(DiscordJsonException)];
74+
var evt = new SentryEvent(new NullReferenceException("library bug"));
75+
evt.SetTag(DiagnosticTags.ErrorOrigin, DiagnosticTags.OriginLibrary);
76+
77+
Assert.True(TelemetryBootstrap.ShouldSendEvent(evt, config));
78+
}
79+
5180
[Fact]
5281
public void ApplyContext_AttachesStructuredContextToEvent()
5382
{

DisCatSharp/Clients/DiscordClient.Dispatch.cs

Lines changed: 53 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,34 +2769,37 @@ internal async Task OnGuildRoleCreateEventAsync(DiscordRole role, DiscordGuild g
27692769
internal async Task OnGuildRoleUpdateEventAsync(DiscordRole role, DiscordGuild guild)
27702770
{
27712771
var newRole = guild.GetRole(role.Id);
2772-
var oldRole = new DiscordRole
2773-
{
2774-
GuildId = guild.Id,
2775-
ColorInternal = newRole.ColorInternal,
2776-
Discord = this,
2777-
IsHoisted = newRole.IsHoisted,
2778-
Id = newRole.Id,
2779-
IsManaged = newRole.IsManaged,
2780-
IsMentionable = newRole.IsMentionable,
2781-
Name = newRole.Name,
2782-
Permissions = newRole.Permissions,
2783-
Position = newRole.Position,
2784-
IconHash = newRole.IconHash,
2785-
Tags = newRole.Tags ?? null,
2786-
UnicodeEmojiString = newRole.UnicodeEmojiString
2787-
};
2772+
var oldRole = CreateRoleSnapshot(newRole ?? role, guild);
27882773

2789-
newRole.GuildId = guild.Id;
2790-
newRole.ColorInternal = role.ColorInternal;
2791-
newRole.IsHoisted = role.IsHoisted;
2792-
newRole.IsManaged = role.IsManaged;
2793-
newRole.IsMentionable = role.IsMentionable;
2794-
newRole.Name = role.Name;
2795-
newRole.Permissions = role.Permissions;
2796-
newRole.Position = role.Position;
2797-
newRole.IconHash = role.IconHash;
2798-
newRole.Tags = role.Tags ?? null;
2799-
newRole.UnicodeEmojiString = role.UnicodeEmojiString;
2774+
if (newRole is null)
2775+
{
2776+
this.Logger.LogWarning(LoggerEvents.WebSocketReceive, "Received guild_role_update for uncached role {RoleId} in guild {GuildId}; refreshing cache from payload.", role.Id, guild.Id);
2777+
role.Discord = this;
2778+
role.GuildId = guild.Id;
2779+
role.GuildInternal = guild;
2780+
guild.RolesInternal[role.Id] = role;
2781+
newRole = role;
2782+
}
2783+
else
2784+
{
2785+
newRole.GuildId = guild.Id;
2786+
newRole.ColorInternal = role.ColorInternal;
2787+
newRole.Discord = this;
2788+
newRole.GuildInternal = guild;
2789+
newRole.IsHoisted = role.IsHoisted;
2790+
newRole.IsManaged = role.IsManaged;
2791+
newRole.IsMentionable = role.IsMentionable;
2792+
newRole.Name = role.Name;
2793+
newRole.Version = role.Version;
2794+
newRole.Description = role.Description;
2795+
newRole.Permissions = role.Permissions;
2796+
newRole.Position = role.Position;
2797+
newRole.IconHash = role.IconHash;
2798+
newRole.Tags = role.Tags;
2799+
newRole.UnicodeEmojiString = role.UnicodeEmojiString;
2800+
newRole.Flags = role.Flags;
2801+
newRole.Colors = role.Colors;
2802+
}
28002803

28012804
var ea = new GuildRoleUpdateEventArgs(this.ServiceProvider)
28022805
{
@@ -2807,6 +2810,29 @@ internal async Task OnGuildRoleUpdateEventAsync(DiscordRole role, DiscordGuild g
28072810
await this.RaiseEventAsync(this._guildRoleUpdated, ea).ConfigureAwait(false);
28082811
}
28092812

2813+
private DiscordRole CreateRoleSnapshot(DiscordRole source, DiscordGuild guild)
2814+
=> new()
2815+
{
2816+
Id = source.Id,
2817+
Discord = this,
2818+
GuildId = guild.Id,
2819+
GuildInternal = guild,
2820+
Name = source.Name,
2821+
Version = source.Version,
2822+
Description = source.Description,
2823+
ColorInternal = source.ColorInternal,
2824+
IsHoisted = source.IsHoisted,
2825+
Position = source.Position,
2826+
Permissions = source.Permissions,
2827+
IsManaged = source.IsManaged,
2828+
IsMentionable = source.IsMentionable,
2829+
Tags = source.Tags,
2830+
IconHash = source.IconHash,
2831+
UnicodeEmojiString = source.UnicodeEmojiString,
2832+
Flags = source.Flags,
2833+
Colors = source.Colors
2834+
};
2835+
28102836
/// <summary>
28112837
/// Handles the guild role delete event.
28122838
/// </summary>

DisCatSharp/Exceptions/ExceptionFilter.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ internal DisCatSharpExceptionFilter(DiscordConfiguration configuration)
3131
/// <param name="ex">The exception to check.</param>
3232
public bool Filter(Exception ex)
3333
{
34+
if (ex.Data[DiagnosticTags.ErrorOrigin] as string == DiagnosticTags.OriginLibrary)
35+
return false;
36+
3437
var trackedException = ex is SentryCapturableException { InnerException: not null } wrapper
3538
? wrapper.InnerException
3639
: ex;

DisCatSharp/Telemetry/SentryDiagnosticsSink.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ public SentryDiagnosticsSink(SentryOptions options, DiscordConfiguration config)
4040
/// <inheritdoc />
4141
public void CaptureException(string source, Exception exception, IDictionary<string, object>? context = null, IDictionary<string, string>? tags = null)
4242
{
43+
exception.Data[DiagnosticTags.Source] = source;
44+
if (tags?.TryGetValue(DiagnosticTags.ErrorOrigin, out var errorOrigin) is true)
45+
exception.Data[DiagnosticTags.ErrorOrigin] = errorOrigin;
46+
4347
if (context is not null && exception is SentryCapturableException sce)
4448
sce.AddSentryContext("Request", context);
4549

DisCatSharp/Telemetry/TelemetryBootstrap.cs

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -123,26 +123,8 @@ private static void ConfigureBeforeSend(SentryOptions options, DiscordConfigurat
123123
{
124124
options.SetBeforeSend((e, _) =>
125125
{
126-
if (!config.Telemetry.DisableExceptionFilter)
127-
{
128-
if (e.Exception is not null)
129-
{
130-
var trackedException = e.Exception is SentryCapturableException { InnerException: not null } wrapper
131-
? wrapper.InnerException
132-
: e.Exception;
133-
134-
if (!config.Telemetry.TrackExceptions.Contains(trackedException.GetType()))
135-
return null;
136-
}
137-
else if (e.Extra.ContainsKey("Found Fields"))
138-
{
139-
// Missing-field reports are allowed through the non-exception path.
140-
}
141-
else if (!e.Tags.ContainsKey(DiagnosticTags.Source))
142-
{
143-
return null;
144-
}
145-
}
126+
if (!ShouldSendEvent(e, config))
127+
return null;
146128

147129
if (!e.Extra.ContainsKey("Found Fields") && (e.Fingerprint is null || e.Fingerprint.Count is 0))
148130
e.SetFingerprint(SentryDiagnosticsSink.GenerateFingerprint(e));
@@ -153,6 +135,32 @@ private static void ConfigureBeforeSend(SentryOptions options, DiscordConfigurat
153135
});
154136
}
155137

138+
internal static bool ShouldSendEvent(SentryEvent e, DiscordConfiguration config)
139+
{
140+
if (config.Telemetry.DisableExceptionFilter)
141+
return true;
142+
143+
var isLibraryOrigin = e.Tags.TryGetValue(DiagnosticTags.ErrorOrigin, out var errorOrigin)
144+
&& errorOrigin == DiagnosticTags.OriginLibrary;
145+
146+
if (e.Exception is not null)
147+
{
148+
if (isLibraryOrigin)
149+
return true;
150+
151+
var trackedException = e.Exception is SentryCapturableException { InnerException: not null } wrapper
152+
? wrapper.InnerException
153+
: e.Exception;
154+
155+
return config.Telemetry.TrackExceptions.Contains(trackedException.GetType());
156+
}
157+
158+
if (e.Extra.ContainsKey("Found Fields"))
159+
return true;
160+
161+
return e.Tags.ContainsKey(DiagnosticTags.Source);
162+
}
163+
156164
/// <summary>
157165
/// Rewrites stack frame paths to stable package-root-relative paths so Sentry code mappings can resolve them.
158166
/// </summary>

0 commit comments

Comments
 (0)