From 43bc220a3daacb64865637304051ed5da37e5b44 Mon Sep 17 00:00:00 2001 From: rladenson Date: Mon, 10 Feb 2025 19:46:52 -0700 Subject: [PATCH 01/18] fix: check group banner existence when querying group banner --- PluralKit.Bot/Commands/Groups.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index fdf0eee1..c9acc464 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -354,8 +354,8 @@ public class Groups { async Task ClearBannerImage() { - await ctx.ConfirmClear("this group's banner image"); ctx.CheckOwnGroup(target); + await ctx.ConfirmClear("this group's banner image"); await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { BannerImage = null }); await ctx.Reply($"{Emojis.Success} Group banner image cleared."); @@ -391,7 +391,7 @@ public class Groups { ctx.CheckSystemPrivacy(target.System, target.BannerPrivacy); - if ((target.Icon?.Trim() ?? "").Length > 0) + if ((target.BannerImage?.Trim() ?? "").Length > 0) switch (ctx.MatchFormat()) { case ReplyFormat.Raw: From 0572725be1dcc2dffbcfc6dc2123422a94d9cda4 Mon Sep 17 00:00:00 2001 From: rladenson Date: Mon, 10 Feb 2025 19:48:27 -0700 Subject: [PATCH 02/18] fix: check member banner permissions on direct queries --- PluralKit.Bot/Commands/MemberEdit.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index 86320128..713dc48f 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -254,6 +254,8 @@ public class MemberEdit async Task ShowBannerImage() { + ctx.CheckSystemPrivacy(target.System, target.BannerPrivacy); + if ((target.BannerImage?.Trim() ?? "").Length > 0) switch (ctx.MatchFormat()) { From 5f6c8c0d140401e3ca75476a0d194bec8d88247f Mon Sep 17 00:00:00 2001 From: Ruth Harris Date: Fri, 21 Feb 2025 13:53:11 -0500 Subject: [PATCH 03/18] feat(docs): add pk;rp to command shorthands list (#723) --- docs/content/tips-and-tricks.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/content/tips-and-tricks.md b/docs/content/tips-and-tricks.md index 9272229c..24fa9163 100644 --- a/docs/content/tips-and-tricks.md +++ b/docs/content/tips-and-tricks.md @@ -27,6 +27,7 @@ PluralKit has a couple of useful command shorthands to reduce the typing: |pk;switch|pk;sw| |pk;message|pk;msg| |pk;autoproxy|pk;ap| +|pk;reproxy|pk;rp| |pk;edit|pk;e| |pk;edit -regex|pk;x| From d537f05b23ba0e1a39eff7efba01959b2547487a Mon Sep 17 00:00:00 2001 From: alyssa Date: Sun, 23 Feb 2025 09:33:32 +0000 Subject: [PATCH 04/18] feat(bot): use avater service for image verify --- PluralKit.Bot/Commands/Groups.cs | 4 +- PluralKit.Bot/Commands/Member.cs | 2 +- PluralKit.Bot/Commands/MemberAvatar.cs | 2 +- PluralKit.Bot/Commands/MemberEdit.cs | 2 +- PluralKit.Bot/Commands/SystemEdit.cs | 6 +-- .../Services/AvatarHostingService.cs | 38 ++++++++++++++ PluralKit.Bot/Utils/AvatarUtils.cs | 51 ------------------- crates/avatars/src/main.rs | 29 ++++++++++- crates/avatars/src/pull.rs | 23 +++++++-- 9 files changed, 94 insertions(+), 63 deletions(-) diff --git a/PluralKit.Bot/Commands/Groups.cs b/PluralKit.Bot/Commands/Groups.cs index c9acc464..a314b248 100644 --- a/PluralKit.Bot/Commands/Groups.cs +++ b/PluralKit.Bot/Commands/Groups.cs @@ -291,7 +291,7 @@ public class Groups ctx.CheckOwnGroup(target); img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Avatar, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); + await _avatarHosting.VerifyAvatarOrThrow(img.Url); await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { Icon = img.CleanUrl ?? img.Url }); @@ -366,7 +366,7 @@ public class Groups ctx.CheckOwnGroup(target); img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Banner, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); + await _avatarHosting.VerifyAvatarOrThrow(img.Url, true); await ctx.Repository.UpdateGroup(target.Id, new GroupPatch { BannerImage = img.CleanUrl ?? img.Url }); diff --git a/PluralKit.Bot/Commands/Member.cs b/PluralKit.Bot/Commands/Member.cs index 46cbfbf9..65931e13 100644 --- a/PluralKit.Bot/Commands/Member.cs +++ b/PluralKit.Bot/Commands/Member.cs @@ -83,7 +83,7 @@ public class Member img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Avatar, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); + await _avatarHosting.VerifyAvatarOrThrow(img.Url); await ctx.Repository.UpdateMember(member.Id, new MemberPatch { AvatarUrl = img.CleanUrl ?? img.Url }, conn); dispatchData.Add("avatar_url", img.CleanUrl); diff --git a/PluralKit.Bot/Commands/MemberAvatar.cs b/PluralKit.Bot/Commands/MemberAvatar.cs index 9dc5b105..26eb310e 100644 --- a/PluralKit.Bot/Commands/MemberAvatar.cs +++ b/PluralKit.Bot/Commands/MemberAvatar.cs @@ -159,7 +159,7 @@ public class MemberAvatar ctx.CheckSystem().CheckOwnMember(target); avatarArg = await _avatarHosting.TryRehostImage(avatarArg.Value, AvatarHostingService.RehostedImageType.Avatar, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, avatarArg.Value.Url); + await _avatarHosting.VerifyAvatarOrThrow(avatarArg.Value.Url); await UpdateAvatar(location, ctx, target, avatarArg.Value.CleanUrl ?? avatarArg.Value.Url); await PrintResponse(location, ctx, target, avatarArg.Value, guildData); } diff --git a/PluralKit.Bot/Commands/MemberEdit.cs b/PluralKit.Bot/Commands/MemberEdit.cs index 713dc48f..0585abd7 100644 --- a/PluralKit.Bot/Commands/MemberEdit.cs +++ b/PluralKit.Bot/Commands/MemberEdit.cs @@ -231,7 +231,7 @@ public class MemberEdit { ctx.CheckOwnMember(target); img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Banner, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); + await _avatarHosting.VerifyAvatarOrThrow(img.Url, true); await ctx.Repository.UpdateMember(target.Id, new MemberPatch { BannerImage = img.CleanUrl ?? img.Url }); diff --git a/PluralKit.Bot/Commands/SystemEdit.cs b/PluralKit.Bot/Commands/SystemEdit.cs index c5d8820a..7df7efff 100644 --- a/PluralKit.Bot/Commands/SystemEdit.cs +++ b/PluralKit.Bot/Commands/SystemEdit.cs @@ -572,7 +572,7 @@ public class SystemEdit ctx.CheckOwnSystem(target); img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Avatar, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); + await _avatarHosting.VerifyAvatarOrThrow(img.Url); await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { AvatarUrl = img.CleanUrl ?? img.Url }); @@ -659,7 +659,7 @@ public class SystemEdit ctx.CheckOwnSystem(target); img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Avatar, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url); + await _avatarHosting.VerifyAvatarOrThrow(img.Url); await ctx.Repository.UpdateSystemGuild(target.Id, ctx.Guild.Id, new SystemGuildPatch { AvatarUrl = img.CleanUrl ?? img.Url }); @@ -781,7 +781,7 @@ public class SystemEdit else if (await ctx.MatchImage() is { } img) { img = await _avatarHosting.TryRehostImage(img, AvatarHostingService.RehostedImageType.Banner, ctx.Author.Id, ctx.System); - await AvatarUtils.VerifyAvatarOrThrow(_client, img.Url, true); + await _avatarHosting.VerifyAvatarOrThrow(img.Url, true); await ctx.Repository.UpdateSystem(target.Id, new SystemPatch { BannerImage = img.CleanUrl ?? img.Url }); diff --git a/PluralKit.Bot/Services/AvatarHostingService.cs b/PluralKit.Bot/Services/AvatarHostingService.cs index 6e88c4af..e43467c3 100644 --- a/PluralKit.Bot/Services/AvatarHostingService.cs +++ b/PluralKit.Bot/Services/AvatarHostingService.cs @@ -18,6 +18,44 @@ public class AvatarHostingService }; } + public async Task VerifyAvatarOrThrow(string url, bool isBanner = false) + { + if (url.Length > Limits.MaxUriLength) + throw Errors.UrlTooLong(url); + + if (!PluralKit.Core.MiscUtils.TryMatchUri(url, out var uri)) + throw Errors.InvalidUrl; + + if (uri.Host.Contains("toyhou.se")) + throw new PKError("Due to server issues, PluralKit is unable to read images hosted on toyhou.se."); + + if (uri.Host == "cdn.pluralkit.me") return; + + if (_config.AvatarServiceUrl == null) + return; + + var kind = isBanner ? "banner" : "avatar"; + + try + { + var response = await _client.PostAsJsonAsync(_config.AvatarServiceUrl + "/verify", + new { url, kind }); + + if (response.StatusCode != HttpStatusCode.OK) + { + var error = await response.Content.ReadFromJsonAsync(); + throw new PKError($"{error.Error}"); + } + } + catch (TaskCanceledException e) + { + // don't show an internal error to users + if (e.Message.Contains("HttpClient.Timeout")) + throw new PKError("Temporary error setting image, please try again later"); + throw; + } + } + public async Task TryRehostImage(ParsedImage input, RehostedImageType type, ulong userId, PKSystem? system) { try diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index 6d986151..a6e05c70 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -8,57 +8,6 @@ namespace PluralKit.Bot; public static class AvatarUtils { - public static async Task VerifyAvatarOrThrow(HttpClient client, string url, bool isFullSizeImage = false) - { - if (url.Length > Limits.MaxUriLength) - throw Errors.UrlTooLong(url); - - // List of MIME types we consider acceptable - var acceptableMimeTypes = new[] - { - "image/jpeg", "image/gif", "image/png", "image/webp" - }; - - if (!PluralKit.Core.MiscUtils.TryMatchUri(url, out var uri)) - throw Errors.InvalidUrl; - - if (uri.Host.Contains("toyhou.se")) - throw new PKError("Due to server issues, PluralKit is unable to read images hosted on toyhou.se."); - - url = TryRewriteCdnUrl(url); - - var response = await client.GetAsync(url); - if (!response.IsSuccessStatusCode) // Check status code - throw Errors.AvatarServerError(response.StatusCode); - if (response.Content.Headers.ContentLength == null) // Check presence of content length - throw Errors.AvatarNotAnImage(null); - try - { - if (!acceptableMimeTypes.Contains(response.Content.Headers.ContentType.MediaType)) // Check MIME type - throw Errors.AvatarNotAnImage(response.Content.Headers.ContentType.MediaType); - } - catch (NullReferenceException) - { - throw new PKError("Could not verify avatar is an image. This can happen when the server sends a malformed response." - + "\nPlease join the support server for help: "); - } - - if (isFullSizeImage) - // no need to do size checking on banners - return; - - if (response.Content.Headers.ContentLength > Limits.AvatarFileSizeLimit) // Check content length - throw Errors.AvatarFileSizeLimit(response.Content.Headers.ContentLength.Value); - - // Parse the image header in a worker - var stream = await response.Content.ReadAsStreamAsync(); - var image = await Task.Run(() => Image.Identify(stream)); - if (image == null) throw Errors.AvatarInvalid; - if (image.Width > Limits.AvatarDimensionLimit || - image.Height > Limits.AvatarDimensionLimit) // Check image size - throw Errors.AvatarDimensionsTooLarge(image.Width, image.Height); - } - // Rewrite cdn.discordapp.com URLs to media.discordapp.net for jpg/png files // This lets us add resizing parameters to "borrow" their media proxy server to downsize the image // which in turn makes it more likely to be underneath the size limit! diff --git a/crates/avatars/src/main.rs b/crates/avatars/src/main.rs index 52ea05f8..b880e326 100644 --- a/crates/avatars/src/main.rs +++ b/crates/avatars/src/main.rs @@ -16,6 +16,7 @@ use axum::{ use libpk::_config::AvatarsConfig; use libpk::db::repository::avatars as db; use libpk::db::types::avatars::*; +use pull::ParsedUrl; use reqwest::{Client, ClientBuilder}; use serde::{Deserialize, Serialize}; use sqlx::PgPool; @@ -35,9 +36,15 @@ pub enum PKAvatarError { #[error("discord cdn responded with status code: {0}")] BadCdnResponse(reqwest::StatusCode), + #[error("server responded with status code: {0}")] + BadServerResponse(reqwest::StatusCode), + #[error("network error: {0}")] NetworkError(reqwest::Error), + #[error("network error: {0}")] + NetworkErrorString(String), + #[error("response is missing header: {0}")] MissingHeader(&'static str), @@ -86,7 +93,6 @@ async fn pull( ) -> Result, PKAvatarError> { let parsed = pull::parse_url(&req.url) // parsing beforehand to "normalize" .map_err(|_| PKAvatarError::InvalidCdnUrl)?; - if !req.force { if let Some(existing) = db::get_by_attachment_id(&state.pool, parsed.attachment_id).await? { // remove any pending image cleanup @@ -132,6 +138,26 @@ async fn pull( })) } +async fn verify( + State(state): State, + Json(req): Json, +) -> Result<(), PKAvatarError> { + let result = crate::pull::pull( + state.pull_client, + &ParsedUrl { + full_url: req.url.clone(), + channel_id: 0, + attachment_id: 0, + filename: "".to_string(), + }, + ) + .await?; + + let encoded = process::process_async(result.data, req.kind).await?; + + Ok(()) +} + pub async fn stats(State(state): State) -> Result, PKAvatarError> { Ok(Json(db::get_stats(&state.pool).await?)) } @@ -193,6 +219,7 @@ async fn real_main() -> anyhow::Result<()> { // migrate::spawn_migrate_workers(Arc::new(state.clone()), state.config.migrate_worker_count); let app = Router::new() + .route("/verify", post(verify)) .route("/pull", post(pull)) .route("/stats", get(stats)) .with_state(state); diff --git a/crates/avatars/src/pull.rs b/crates/avatars/src/pull.rs index a3c96601..e229443c 100644 --- a/crates/avatars/src/pull.rs +++ b/crates/avatars/src/pull.rs @@ -3,6 +3,8 @@ use std::{str::FromStr, sync::Arc}; use crate::PKAvatarError; use anyhow::Context; use reqwest::{Client, StatusCode, Url}; +use std::error::Error; +use std::fmt::Write; use std::time::Instant; use tracing::{error, instrument}; @@ -28,14 +30,29 @@ pub async fn pull( .expect("set_host should not fail"); } let response = client.get(trimmed_url.clone()).send().await.map_err(|e| { - error!("network error for {}: {}", parsed_url.full_url, e); - PKAvatarError::NetworkError(e) + // terrible + let mut s = format!("{}", e); + if let Some(src) = e.source() { + let _ = write!(s, "\n\nCaused by: {}", src); + let mut err = src; + while let Some(src) = err.source() { + let _ = write!(s, "\n\nCaused by: {}", src); + err = src; + } + } + + error!("network error for {}: {}", parsed_url.full_url, s); + PKAvatarError::NetworkErrorString(s) })?; let time_after_headers = Instant::now(); let status = response.status(); if status != StatusCode::OK { - return Err(PKAvatarError::BadCdnResponse(status)); + if trimmed_url.host_str() == Some("cdn.discordapp.com") { + return Err(PKAvatarError::BadCdnResponse(status)); + } else { + return Err(PKAvatarError::BadServerResponse(status)); + } } let size = match response.content_length() { From 82661001554af02c4ed40579a0353d7f86572084 Mon Sep 17 00:00:00 2001 From: alyssa Date: Sun, 23 Feb 2025 11:02:55 +0000 Subject: [PATCH 05/18] fix(bot): clean up some error messages --- PluralKit.Bot/Commands/Message.cs | 6 +++ PluralKit.Bot/Proxy/ProxyService.cs | 44 +++++++++++-------- .../Services/WebhookExecutorService.cs | 3 ++ 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/PluralKit.Bot/Commands/Message.cs b/PluralKit.Bot/Commands/Message.cs index dbd986cf..692f75b1 100644 --- a/PluralKit.Bot/Commands/Message.cs +++ b/PluralKit.Bot/Commands/Message.cs @@ -241,6 +241,12 @@ public class ProxiedMessage { throw new PKError("Could not edit message."); } + catch (BadRequestException e) + { + if (e.Message == "Voice messages cannot be edited") + throw new PKError($"{e.Message}."); + throw; + } } private async Task<(PKMessage, SystemId)> GetMessageToEdit(Context ctx, Duration timeout, bool isReproxy) diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index 79cced26..ead5f1a6 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -238,25 +238,33 @@ public class ProxyService if (trigger.Flags.HasFlag(Message.MessageFlags.VoiceMessage)) flags |= Message.MessageFlags.VoiceMessage; - var proxyMessage = await _webhookExecutor.ExecuteWebhook(new ProxyRequest + try { - GuildId = trigger.GuildId!.Value, - ChannelId = rootChannel.Id, - ThreadId = threadId, - MessageId = trigger.Id, - Name = await FixSameName(messageChannel.Id, ctx, match.Member), - AvatarUrl = AvatarUtils.TryRewriteCdnUrl(match.Member.ProxyAvatar(ctx)), - Content = content, - Attachments = trigger.Attachments, - FileSizeLimit = guild.FileSizeLimit(), - Embeds = embeds.ToArray(), - Stickers = trigger.StickerItems, - AllowEveryone = allowEveryone, - Flags = flags, - Tts = tts, - Poll = trigger.Poll, - }); - await HandleProxyExecutedActions(ctx, autoproxySettings, trigger, proxyMessage, match); + var proxyMessage = await _webhookExecutor.ExecuteWebhook(new ProxyRequest + { + GuildId = trigger.GuildId!.Value, + ChannelId = rootChannel.Id, + ThreadId = threadId, + MessageId = trigger.Id, + Name = await FixSameName(messageChannel.Id, ctx, match.Member), + AvatarUrl = AvatarUtils.TryRewriteCdnUrl(match.Member.ProxyAvatar(ctx)), + Content = content, + Attachments = trigger.Attachments, + FileSizeLimit = guild.FileSizeLimit(), + Embeds = embeds.ToArray(), + Stickers = trigger.StickerItems, + AllowEveryone = allowEveryone, + Flags = flags, + Tts = tts, + Poll = trigger.Poll, + }); + await HandleProxyExecutedActions(ctx, autoproxySettings, trigger, proxyMessage, match); + } + catch (PKError) + { + if (ctx.ProxyErrorMessageEnabled) + throw; + } } public async Task ExecuteReproxy(Message trigger, PKMessage msg, List members, ProxyMember member, string prefix) diff --git a/PluralKit.Bot/Services/WebhookExecutorService.cs b/PluralKit.Bot/Services/WebhookExecutorService.cs index 999920d6..3bfcb3bf 100644 --- a/PluralKit.Bot/Services/WebhookExecutorService.cs +++ b/PluralKit.Bot/Services/WebhookExecutorService.cs @@ -191,6 +191,9 @@ public class WebhookExecutorService } catch (BadRequestException e) { + if (e.Message == "Cannot use one or more emoji included with this poll") + throw new PKError($"Discord rejected proxy message: {e.Message}"); + // explanation for hacky: I don't care if this code fails, it just means it wasn't a username error try { From 63777bf810275817111fe218c423f25918c8f52e Mon Sep 17 00:00:00 2001 From: alyssa Date: Sun, 23 Feb 2025 18:49:31 +0000 Subject: [PATCH 06/18] feat: add sentry error logging to dotnet-api and rust crates --- Cargo.lock | 60 +++++++++++++++--------------- Cargo.toml | 4 +- PluralKit.API/PluralKit.API.csproj | 1 + PluralKit.API/Program.cs | 10 +++++ PluralKit.API/Startup.cs | 5 +++ PluralKit.API/packages.lock.json | 6 +++ crates/libpk/Cargo.toml | 1 + crates/libpk/src/lib.rs | 37 +++++++++++++++--- 8 files changed, 86 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc949bbe..63502687 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1490,7 +1490,7 @@ dependencies = [ "http 1.1.0", "hyper 1.5.0", "hyper-util", - "rustls 0.23.10", + "rustls 0.23.23", "rustls-pki-types", "tokio", "tokio-rustls 0.26.0", @@ -1698,6 +1698,7 @@ dependencies = [ "metrics", "metrics-exporter-prometheus", "sentry", + "sentry-tracing", "serde", "serde_json", "sqlx", @@ -2349,7 +2350,7 @@ dependencies = [ "quinn-proto", "quinn-udp", "rustc-hash", - "rustls 0.23.10", + "rustls 0.23.23", "socket2 0.5.7", "thiserror", "tokio", @@ -2366,7 +2367,7 @@ dependencies = [ "rand", "ring 0.17.8", "rustc-hash", - "rustls 0.23.10", + "rustls 0.23.23", "slab", "thiserror", "tinyvec", @@ -2599,7 +2600,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "quinn", - "rustls 0.23.10", + "rustls 0.23.23", "rustls-pemfile 2.1.2", "rustls-pki-types", "serde", @@ -2809,25 +2810,24 @@ version = "0.22.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf4ef73721ac7bcd79b2b315da7779d8fc09718c6b3d2d1b2d94850eb8c18432" dependencies = [ - "log", "ring 0.17.8", "rustls-pki-types", - "rustls-webpki 0.102.4", + "rustls-webpki 0.102.8", "subtle", "zeroize", ] [[package]] name = "rustls" -version = "0.23.10" +version = "0.23.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05cff451f60db80f490f3c182b77c35260baace73209e9cdbbe526bfe3a4d402" +checksum = "47796c98c480fce5406ef69d1c76378375492c3b0a0de587be0c1d9feb12f395" dependencies = [ "log", "once_cell", "ring 0.17.8", "rustls-pki-types", - "rustls-webpki 0.102.4", + "rustls-webpki 0.102.8", "subtle", "zeroize", ] @@ -2882,9 +2882,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.102.4" +version = "0.102.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff448f7e92e913c4b7d4c6d8e4540a1724b319b4152b8aef6d4cf8339712b33e" +checksum = "64ca1bc8749bd4cf37b5ce386cc146580777b4e8572c7b97baf22c83f444bee9" dependencies = [ "ring 0.17.8", "rustls-pki-types", @@ -3002,13 +3002,13 @@ checksum = "58bc9567378fc7690d6b2addae4e60ac2eeea07becb2c64b9f218b53865cba2a" [[package]] name = "sentry" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5484316556650182f03b43d4c746ce0e3e48074a21e2f51244b648b6542e1066" +checksum = "3a7332159e544e34db06b251b1eda5e546bd90285c3f58d9c8ff8450b484e0da" dependencies = [ "httpdate", "reqwest 0.12.8", - "rustls 0.22.4", + "rustls 0.23.23", "sentry-backtrace", "sentry-contexts", "sentry-core", @@ -3022,9 +3022,9 @@ dependencies = [ [[package]] name = "sentry-backtrace" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "40aa225bb41e2ec9d7c90886834367f560efc1af028f1c5478a6cce6a59c463a" +checksum = "565ec31ad37bab8e6d9f289f34913ed8768347b133706192f10606dabd5c6bc4" dependencies = [ "backtrace", "once_cell", @@ -3034,9 +3034,9 @@ dependencies = [ [[package]] name = "sentry-contexts" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a8dd746da3d16cb8c39751619cefd4fcdbd6df9610f3310fd646b55f6e39910" +checksum = "e860275f25f27e8c0c7726ce116c7d5c928c5bba2ee73306e52b20a752298ea6" dependencies = [ "hostname", "libc", @@ -3048,9 +3048,9 @@ dependencies = [ [[package]] name = "sentry-core" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "161283cfe8e99c8f6f236a402b9ccf726b201f365988b5bb637ebca0abbd4a30" +checksum = "653942e6141f16651273159f4b8b1eaeedf37a7554c00cd798953e64b8a9bf72" dependencies = [ "once_cell", "rand", @@ -3061,9 +3061,9 @@ dependencies = [ [[package]] name = "sentry-debug-images" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fc6b25e945fcaa5e97c43faee0267eebda9f18d4b09a251775d8fef1086238a" +checksum = "2a60bc2154e6df59beed0ac13d58f8dfaf5ad20a88548a53e29e4d92e8e835c2" dependencies = [ "findshlibs", "once_cell", @@ -3072,9 +3072,9 @@ dependencies = [ [[package]] name = "sentry-panic" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc74f229c7186dd971a9491ffcbe7883544aa064d1589bd30b83fb856cd22d63" +checksum = "105e3a956c8aa9dab1e4087b1657b03271bfc49d838c6ae9bfc7c58c802fd0ef" dependencies = [ "sentry-backtrace", "sentry-core", @@ -3082,9 +3082,9 @@ dependencies = [ [[package]] name = "sentry-tracing" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cd3c5faf2103cd01eeda779ea439b68c4ee15adcdb16600836e97feafab362ec" +checksum = "64e75c831b4d8b34a5aec1f65f67c5d46a26c7c5d3c7abd8b5ef430796900cf8" dependencies = [ "sentry-backtrace", "sentry-core", @@ -3094,9 +3094,9 @@ dependencies = [ [[package]] name = "sentry-types" -version = "0.34.0" +version = "0.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d68cdf6bc41b8ff3ae2a9c4671e97426dcdd154cc1d4b6b72813f285d6b163f" +checksum = "2d4203359e60724aa05cf2385aaf5d4f147e837185d7dd2b9ccf1ee77f4420c8" dependencies = [ "debugid", "hex", @@ -3819,7 +3819,7 @@ version = "0.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0c7bc40d0e5a97695bb96e27995cd3a08538541b0a846f65bba7a359f36700d4" dependencies = [ - "rustls 0.23.10", + "rustls 0.23.23", "rustls-pki-types", "tokio", ] @@ -4233,7 +4233,7 @@ dependencies = [ "base64 0.22.1", "log", "once_cell", - "rustls 0.23.10", + "rustls 0.23.23", "rustls-pki-types", "url", "webpki-roots 0.26.6", diff --git a/Cargo.toml b/Cargo.toml index af0f6674..c605303a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,13 +15,13 @@ futures = "0.3.30" lazy_static = "1.4.0" metrics = "0.23.0" reqwest = { version = "0.12.7" , default-features = false, features = ["rustls-tls", "trust-dns"]} -sentry = { version = "0.34.0", default-features = false, features = ["backtrace", "contexts", "panic", "debug-images", "reqwest", "rustls"] } # replace native-tls with rustls +sentry = { version = "0.36.0", default-features = false, features = ["backtrace", "contexts", "panic", "debug-images", "reqwest", "rustls"] } # replace native-tls with rustls serde = { version = "1.0.196", features = ["derive"] } serde_json = "1.0.117" signal-hook = "0.3.17" sqlx = { version = "0.8.2", features = ["runtime-tokio", "postgres", "time", "macros", "uuid"] } tokio = { version = "1.36.0", features = ["full"] } -tracing = "0.1.40" +tracing = "0.1" tracing-subscriber = { version = "0.3.16", features = ["env-filter", "json"] } uuid = { version = "1.7.0", features = ["serde"] } diff --git a/PluralKit.API/PluralKit.API.csproj b/PluralKit.API/PluralKit.API.csproj index ea792b98..e35518dd 100644 --- a/PluralKit.API/PluralKit.API.csproj +++ b/PluralKit.API/PluralKit.API.csproj @@ -31,6 +31,7 @@ + diff --git a/PluralKit.API/Program.cs b/PluralKit.API/Program.cs index d3466d4f..4f746b57 100644 --- a/PluralKit.API/Program.cs +++ b/PluralKit.API/Program.cs @@ -14,6 +14,16 @@ public class Program await BuildInfoService.LoadVersion(); var host = CreateHostBuilder(args).Build(); var config = host.Services.GetRequiredService(); + + // Initialize Sentry SDK, and make sure it gets dropped at the end + using var _ = SentrySdk.Init(opts => + { + opts.Dsn = config.SentryUrl ?? ""; + opts.Release = BuildInfoService.FullVersion; + opts.AutoSessionTracking = true; + // opts.DisableTaskUnobservedTaskExceptionCapture(); + }); + await host.Services.GetRequiredService().InitAsync(config); await host.RunAsync(); } diff --git a/PluralKit.API/Startup.cs b/PluralKit.API/Startup.cs index a564cbdc..ecb91710 100644 --- a/PluralKit.API/Startup.cs +++ b/PluralKit.API/Startup.cs @@ -62,8 +62,13 @@ public class Startup await ctx.Response.WriteJSON(400, "{\"message\":\"400: Bad Request\",\"code\":0}"); else if (exc.Error is not PKError) + { await ctx.Response.WriteJSON(500, "{\"message\":\"500: Internal Server Error\",\"code\":0}"); + var sentryEvent = new SentryEvent(exc.Error); + SentrySdk.CaptureEvent(sentryEvent); + } + // for some reason, if we don't specifically cast to ModelParseError, it uses the base's ToJson method else if (exc.Error is ModelParseError fe) await ctx.Response.WriteJSON(fe.ResponseCode, JsonConvert.SerializeObject(fe.ToJson())); diff --git a/PluralKit.API/packages.lock.json b/PluralKit.API/packages.lock.json index 8c8be7c4..adc4279b 100644 --- a/PluralKit.API/packages.lock.json +++ b/PluralKit.API/packages.lock.json @@ -28,6 +28,12 @@ "Microsoft.AspNetCore.Mvc.Versioning": "5.1.0" } }, + "Sentry": { + "type": "Direct", + "requested": "[4.13.0, )", + "resolved": "4.13.0", + "contentHash": "Wfw3M1WpFcrYaGzPm7QyUTfIOYkVXQ1ry6p4WYjhbLz9fPwV23SGQZTFDpdox67NHM0V0g1aoQ4YKLm4ANtEEg==" + }, "Serilog.AspNetCore": { "type": "Direct", "requested": "[9.0.0, )", diff --git a/crates/libpk/Cargo.toml b/crates/libpk/Cargo.toml index 4b084038..40d430c8 100644 --- a/crates/libpk/Cargo.toml +++ b/crates/libpk/Cargo.toml @@ -21,3 +21,4 @@ uuid = { workspace = true } config = "0.14.0" json-subscriber = { version = "0.2.2", features = ["env-filter"] } metrics-exporter-prometheus = { version = "0.15.3", default-features = false, features = ["tokio", "http-listener", "tracing"] } +sentry-tracing = "0.36.0" diff --git a/crates/libpk/src/lib.rs b/crates/libpk/src/lib.rs index ee7a6536..39643fb6 100644 --- a/crates/libpk/src/lib.rs +++ b/crates/libpk/src/lib.rs @@ -5,6 +5,8 @@ use metrics_exporter_prometheus::PrometheusBuilder; use sentry::IntoDsn; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilter}; +use sentry_tracing::event_from_event; + pub mod db; pub mod state; pub mod util; @@ -14,8 +16,20 @@ pub use crate::_config::CONFIG as config; // functions in this file are only used by the main function below -pub fn init_logging(component: &str) -> anyhow::Result<()> { +pub fn init_logging(component: &str) { + // todo: clean up the sentry duplication if config.json_log { + let sentry_layer = + sentry_tracing::layer().event_mapper(|md, ctx| match md.metadata().level() { + &tracing::Level::ERROR => { + // for some reason this works, but letting the library handle it doesn't + let event = event_from_event(md, ctx); + sentry::capture_event(event); + sentry_tracing::EventMapping::Ignore + } + _ => sentry_tracing::EventMapping::Ignore, + }); + let mut layer = json_subscriber::layer(); layer.inner_layer_mut().add_static_field( "component", @@ -23,15 +37,26 @@ pub fn init_logging(component: &str) -> anyhow::Result<()> { ); tracing_subscriber::registry() .with(layer) + .with(sentry_layer) .with(EnvFilter::from_default_env()) .init(); } else { - tracing_subscriber::fmt() - .with_env_filter(EnvFilter::from_default_env()) + let sentry_layer = + sentry_tracing::layer().event_mapper(|md, ctx| match md.metadata().level() { + &tracing::Level::ERROR => { + // for some reason this works, but letting the library handle it doesn't + let event = event_from_event(md, ctx); + sentry::capture_event(event); + sentry_tracing::EventMapping::Ignore + } + _ => sentry_tracing::EventMapping::Ignore, + }); + + tracing_subscriber::registry() + .with(tracing_subscriber::fmt::layer()) + .with(sentry_layer) .init(); } - - Ok(()) } pub fn init_metrics() -> anyhow::Result<()> { @@ -61,7 +86,7 @@ macro_rules! main { fn main() -> anyhow::Result<()> { let _sentry_guard = libpk::init_sentry(); // we might also be able to use env!("CARGO_CRATE_NAME") here - libpk::init_logging($component)?; + libpk::init_logging($component); tokio::runtime::Builder::new_multi_thread() .enable_all() .build() From d74a6678c85bba13076a2b21d7d68ab119d7d1bd Mon Sep 17 00:00:00 2001 From: alyssa Date: Mon, 24 Feb 2025 10:53:18 +0000 Subject: [PATCH 07/18] fix(bot): clean up "not found in cache" errors in sentry --- Myriad/Extensions/CacheExtensions.cs | 16 ++++++++++++++-- PluralKit.Bot/Bot.cs | 7 +++++++ PluralKit.Tests/packages.lock.json | 1 + 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Myriad/Extensions/CacheExtensions.cs b/Myriad/Extensions/CacheExtensions.cs index 84045784..084450bf 100644 --- a/Myriad/Extensions/CacheExtensions.cs +++ b/Myriad/Extensions/CacheExtensions.cs @@ -9,14 +9,14 @@ public static class CacheExtensions public static async Task GetGuild(this IDiscordCache cache, ulong guildId) { if (!(await cache.TryGetGuild(guildId) is Guild guild)) - throw new KeyNotFoundException($"Guild {guildId} not found in cache"); + throw new NotFoundInCacheException(guildId, "guild"); return guild; } public static async Task GetChannel(this IDiscordCache cache, ulong guildId, ulong channelId) { if (!(await cache.TryGetChannel(guildId, channelId) is Channel channel)) - throw new KeyNotFoundException($"Channel {channelId} not found in cache"); + throw new NotFoundInCacheException(channelId, "channel"); return channel; } @@ -54,4 +54,16 @@ public static class CacheExtensions if (parent == null) throw new Exception($"failed to find parent channel for thread {channelOrThread} in cache"); return parent; } +} + +public class NotFoundInCacheException: Exception +{ + public ulong EntityId { get; init; } + public string EntityType { get; init; } + + public NotFoundInCacheException(ulong id, string type) : base("expected entity in discord cache but was not found") + { + EntityId = id; + EntityType = type; + } } \ No newline at end of file diff --git a/PluralKit.Bot/Bot.cs b/PluralKit.Bot/Bot.cs index 6e15799c..d74428ee 100644 --- a/PluralKit.Bot/Bot.cs +++ b/PluralKit.Bot/Bot.cs @@ -213,6 +213,13 @@ public class Bot { _metrics.Measure.Meter.Mark(BotMetrics.BotErrors, exc.GetType().FullName); + if (exc is Myriad.Extensions.NotFoundInCacheException ce) + { + var scope = serviceScope.Resolve(); + scope.SetTag("entity.id", ce.EntityId.ToString()); + scope.SetTag("entity.type", ce.EntityType); + } + // Make this beforehand so we can access the event ID for logging var sentryEvent = new SentryEvent(exc); diff --git a/PluralKit.Tests/packages.lock.json b/PluralKit.Tests/packages.lock.json index 419ec053..8946e898 100644 --- a/PluralKit.Tests/packages.lock.json +++ b/PluralKit.Tests/packages.lock.json @@ -949,6 +949,7 @@ "Microsoft.AspNetCore.Mvc.Versioning": "[5.1.0, )", "Microsoft.AspNetCore.Mvc.Versioning.ApiExplorer": "[5.1.0, )", "PluralKit.Core": "[1.0.0, )", + "Sentry": "[4.13.0, )", "Serilog.AspNetCore": "[9.0.0, )" } }, From 5fa9266d8dcfd8fb64daf660a575007c9d005f46 Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 11:49:01 +0000 Subject: [PATCH 08/18] fix(avatars): shorter error message for image pull failure --- crates/avatars/src/pull.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/avatars/src/pull.rs b/crates/avatars/src/pull.rs index e229443c..8b9064d0 100644 --- a/crates/avatars/src/pull.rs +++ b/crates/avatars/src/pull.rs @@ -33,10 +33,10 @@ pub async fn pull( // terrible let mut s = format!("{}", e); if let Some(src) = e.source() { - let _ = write!(s, "\n\nCaused by: {}", src); + let _ = write!(s, ": {}", src); let mut err = src; while let Some(src) = err.source() { - let _ = write!(s, "\n\nCaused by: {}", src); + let _ = write!(s, ": {}", src); err = src; } } From ec49ead7839c790bd8b9aca231fe4cf5c4c1ad5e Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 11:58:54 +0000 Subject: [PATCH 09/18] fix(gateway): remove superflous redis error handling code that actually created more errors --- crates/gateway/src/discord/shard_state.rs | 8 ++------ crates/libpk/src/lib.rs | 1 - crates/libpk/src/util/mod.rs | 1 - crates/libpk/src/util/redis.rs | 15 --------------- 4 files changed, 2 insertions(+), 23 deletions(-) delete mode 100644 crates/libpk/src/util/mod.rs delete mode 100644 crates/libpk/src/util/redis.rs diff --git a/crates/gateway/src/discord/shard_state.rs b/crates/gateway/src/discord/shard_state.rs index 7c3d9bf7..a7579583 100644 --- a/crates/gateway/src/discord/shard_state.rs +++ b/crates/gateway/src/discord/shard_state.rs @@ -3,7 +3,7 @@ use metrics::{counter, gauge}; use tracing::info; use twilight_gateway::{Event, Latency}; -use libpk::{state::*, util::redis::*}; +use libpk::state::ShardState; #[derive(Clone)] pub struct ShardStateManager { @@ -24,11 +24,7 @@ impl ShardStateManager { } async fn get_shard(&self, shard_id: u32) -> anyhow::Result { - let data: Option = self - .redis - .hget("pluralkit:shardstatus", shard_id) - .await - .to_option_or_error()?; + let data: Option = self.redis.hget("pluralkit:shardstatus", shard_id).await?; match data { Some(buf) => Ok(serde_json::from_str(&buf).expect("could not decode shard data!")), None => Ok(ShardState::default()), diff --git a/crates/libpk/src/lib.rs b/crates/libpk/src/lib.rs index 39643fb6..03c420e7 100644 --- a/crates/libpk/src/lib.rs +++ b/crates/libpk/src/lib.rs @@ -9,7 +9,6 @@ use sentry_tracing::event_from_event; pub mod db; pub mod state; -pub mod util; pub mod _config; pub use crate::_config::CONFIG as config; diff --git a/crates/libpk/src/util/mod.rs b/crates/libpk/src/util/mod.rs deleted file mode 100644 index 027fbef5..00000000 --- a/crates/libpk/src/util/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod redis; diff --git a/crates/libpk/src/util/redis.rs b/crates/libpk/src/util/redis.rs deleted file mode 100644 index 25a5acdb..00000000 --- a/crates/libpk/src/util/redis.rs +++ /dev/null @@ -1,15 +0,0 @@ -use fred::error::RedisError; - -pub trait RedisErrorExt { - fn to_option_or_error(self) -> Result, RedisError>; -} - -impl RedisErrorExt for Result { - fn to_option_or_error(self) -> Result, RedisError> { - match self { - Ok(v) => Ok(Some(v)), - Err(error) if error.is_not_found() => Ok(None), - Err(error) => Err(error), - } - } -} From 3cf71112d6a1025b28228bce75527fe3cdba776f Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 12:04:13 +0000 Subject: [PATCH 10/18] feat(gateway): add metric for shard close code --- crates/gateway/src/discord/gateway.rs | 56 +++++++++++++-------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/crates/gateway/src/discord/gateway.rs b/crates/gateway/src/discord/gateway.rs index cbc5f1a1..6aedca23 100644 --- a/crates/gateway/src/discord/gateway.rs +++ b/crates/gateway/src/discord/gateway.rs @@ -83,29 +83,38 @@ pub async fn runner( cache: Arc, ) { // let _span = info_span!("shard_runner", shard_id = shard.id().number()).entered(); + let shard_id = shard.id().number(); + info!("waiting for events"); while let Some(item) = shard.next().await { let raw_event = match item { Ok(evt) => match evt { Message::Close(frame) => { - info!( - "shard {} closed: {}", - shard.id().number(), - if let Some(close) = frame { - format!("{} ({})", close.code, close.reason) - } else { - "unknown".to_string() - } - ); - if let Err(error) = shard_state.socket_closed(shard.id().number()).await { + let close_code = if let Some(close) = frame { + close.code.to_string() + } else { + "unknown".to_string() + }; + + info!("shard {shard_id} closed: {close_code}"); + + counter!( + "pluralkit_gateway_shard_closed", + "shard_id" => shard_id.to_string(), + "close_code" => close_code, + ) + .increment(1); + + if let Err(error) = shard_state.socket_closed(shard_id).await { error!("failed to update shard state for socket closure: {error}"); } + continue; } Message::Text(text) => text, }, Err(error) => { - tracing::warn!(?error, "error receiving event from shard {}", shard.id()); + tracing::warn!(?error, "error receiving event from shard {shard_id}"); continue; } }; @@ -118,11 +127,7 @@ pub async fn runner( continue; } Err(error) => { - error!( - "shard {} failed to parse gateway event: {}", - shard.id().number(), - error - ); + error!("shard {shard_id} failed to parse gateway event: {error}"); continue; } }; @@ -137,29 +142,24 @@ pub async fn runner( .increment(1); counter!( "pluralkit_gateway_events_shard", - "shard_id" => shard.id().number().to_string(), + "shard_id" => shard_id.to_string(), ) .increment(1); // update shard state and discord cache - if let Err(error) = shard_state - .handle_event(shard.id().number(), event.clone()) - .await - { - tracing::warn!(?error, "error updating redis state"); + if let Err(error) = shard_state.handle_event(shard_id, event.clone()).await { + tracing::error!(?error, "error updating redis state"); } // need to do heartbeat separately, to get the latency if let Event::GatewayHeartbeatAck = event - && let Err(error) = shard_state - .heartbeated(shard.id().number(), shard.latency()) - .await + && let Err(error) = shard_state.heartbeated(shard_id, shard.latency()).await { - tracing::warn!(?error, "error updating redis state for latency"); + tracing::error!(?error, "error updating redis state for latency"); } if let Event::Ready(_) = event { - if !cache.2.read().await.contains(&shard.id().number()) { - cache.2.write().await.push(shard.id().number()); + if !cache.2.read().await.contains(&shard_id) { + cache.2.write().await.push(shard_id); } } cache.0.update(&event); From f2583904ef4c709b5e2acf9df05bba7483faaf7d Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 12:05:00 +0000 Subject: [PATCH 11/18] chore(rust): clean up duplicated sentry code --- crates/libpk/src/lib.rs | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/crates/libpk/src/lib.rs b/crates/libpk/src/lib.rs index 03c420e7..af967728 100644 --- a/crates/libpk/src/lib.rs +++ b/crates/libpk/src/lib.rs @@ -16,44 +16,32 @@ pub use crate::_config::CONFIG as config; // functions in this file are only used by the main function below pub fn init_logging(component: &str) { - // todo: clean up the sentry duplication - if config.json_log { - let sentry_layer = - sentry_tracing::layer().event_mapper(|md, ctx| match md.metadata().level() { - &tracing::Level::ERROR => { - // for some reason this works, but letting the library handle it doesn't - let event = event_from_event(md, ctx); - sentry::capture_event(event); - sentry_tracing::EventMapping::Ignore - } - _ => sentry_tracing::EventMapping::Ignore, - }); + let sentry_layer = + sentry_tracing::layer().event_mapper(|md, ctx| match md.metadata().level() { + &tracing::Level::ERROR => { + // for some reason this works, but letting the library handle it doesn't + let event = event_from_event(md, ctx); + sentry::capture_event(event); + sentry_tracing::EventMapping::Ignore + } + _ => sentry_tracing::EventMapping::Ignore, + }); + if config.json_log { let mut layer = json_subscriber::layer(); layer.inner_layer_mut().add_static_field( "component", serde_json::Value::String(component.to_string()), ); tracing_subscriber::registry() - .with(layer) .with(sentry_layer) + .with(layer) .with(EnvFilter::from_default_env()) .init(); } else { - let sentry_layer = - sentry_tracing::layer().event_mapper(|md, ctx| match md.metadata().level() { - &tracing::Level::ERROR => { - // for some reason this works, but letting the library handle it doesn't - let event = event_from_event(md, ctx); - sentry::capture_event(event); - sentry_tracing::EventMapping::Ignore - } - _ => sentry_tracing::EventMapping::Ignore, - }); - tracing_subscriber::registry() - .with(tracing_subscriber::fmt::layer()) .with(sentry_layer) + .with(tracing_subscriber::fmt::layer()) .init(); } } From c58eb274df1a0830900d014378109bca89c8ee17 Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 12:08:38 +0000 Subject: [PATCH 12/18] chore(bot): remove unused library ImageSharp --- PluralKit.Bot/PluralKit.Bot.csproj | 1 - PluralKit.Bot/Utils/AvatarUtils.cs | 2 -- PluralKit.Bot/packages.lock.json | 6 ------ PluralKit.Tests/packages.lock.json | 8 +------- 4 files changed, 1 insertion(+), 16 deletions(-) diff --git a/PluralKit.Bot/PluralKit.Bot.csproj b/PluralKit.Bot/PluralKit.Bot.csproj index f15e7051..01d89e1d 100644 --- a/PluralKit.Bot/PluralKit.Bot.csproj +++ b/PluralKit.Bot/PluralKit.Bot.csproj @@ -24,6 +24,5 @@ - diff --git a/PluralKit.Bot/Utils/AvatarUtils.cs b/PluralKit.Bot/Utils/AvatarUtils.cs index a6e05c70..65a208ed 100644 --- a/PluralKit.Bot/Utils/AvatarUtils.cs +++ b/PluralKit.Bot/Utils/AvatarUtils.cs @@ -2,8 +2,6 @@ using System.Text.RegularExpressions; using PluralKit.Core; -using SixLabors.ImageSharp; - namespace PluralKit.Bot; public static class AvatarUtils diff --git a/PluralKit.Bot/packages.lock.json b/PluralKit.Bot/packages.lock.json index fbe69392..e2eda930 100644 --- a/PluralKit.Bot/packages.lock.json +++ b/PluralKit.Bot/packages.lock.json @@ -14,12 +14,6 @@ "resolved": "4.13.0", "contentHash": "Wfw3M1WpFcrYaGzPm7QyUTfIOYkVXQ1ry6p4WYjhbLz9fPwV23SGQZTFDpdox67NHM0V0g1aoQ4YKLm4ANtEEg==" }, - "SixLabors.ImageSharp": { - "type": "Direct", - "requested": "[3.1.6, )", - "resolved": "3.1.6", - "contentHash": "dHQ5jugF9v+5/LCVHCWVzaaIL6WOehqJy6eju/0VFYFPEj2WtqkGPoEV9EVQP83dHsdoqYaTuWpZdwAd37UwfA==" - }, "App.Metrics": { "type": "Transitive", "resolved": "4.3.0", diff --git a/PluralKit.Tests/packages.lock.json b/PluralKit.Tests/packages.lock.json index 8946e898..2ba02316 100644 --- a/PluralKit.Tests/packages.lock.json +++ b/PluralKit.Tests/packages.lock.json @@ -654,11 +654,6 @@ "Serilog.Sinks.File": "5.0.0" } }, - "SixLabors.ImageSharp": { - "type": "Transitive", - "resolved": "3.1.6", - "contentHash": "dHQ5jugF9v+5/LCVHCWVzaaIL6WOehqJy6eju/0VFYFPEj2WtqkGPoEV9EVQP83dHsdoqYaTuWpZdwAd37UwfA==" - }, "SqlKata": { "type": "Transitive", "resolved": "2.4.0", @@ -959,8 +954,7 @@ "Humanizer.Core": "[2.14.1, )", "Myriad": "[1.0.0, )", "PluralKit.Core": "[1.0.0, )", - "Sentry": "[4.13.0, )", - "SixLabors.ImageSharp": "[3.1.6, )" + "Sentry": "[4.13.0, )" } }, "pluralkit.core": { From b77390c0cdd087775d50c4c5c94f7d302c7b43cb Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 13:13:14 +0000 Subject: [PATCH 13/18] feat: add metric for remaining gateway sessions --- Cargo.lock | 1 + crates/scheduled_tasks/Cargo.toml | 1 + crates/scheduled_tasks/src/main.rs | 25 +++++++++++++++++++++++++ crates/scheduled_tasks/src/tasks.rs | 8 +++++++- 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 63502687..ee181b20 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2929,6 +2929,7 @@ dependencies = [ "sqlx", "tokio", "tracing", + "twilight-http", ] [[package]] diff --git a/crates/scheduled_tasks/Cargo.toml b/crates/scheduled_tasks/Cargo.toml index e554edd3..7420e087 100644 --- a/crates/scheduled_tasks/Cargo.toml +++ b/crates/scheduled_tasks/Cargo.toml @@ -16,6 +16,7 @@ serde_json = { workspace = true } sqlx = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } +twilight-http = { workspace = true } croner = "2.1.0" num-format = "0.4.4" diff --git a/crates/scheduled_tasks/src/main.rs b/crates/scheduled_tasks/src/main.rs index fc5e68ed..266325ed 100644 --- a/crates/scheduled_tasks/src/main.rs +++ b/crates/scheduled_tasks/src/main.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; + use chrono::Utc; use croner::Cron; use fred::prelude::RedisPool; @@ -14,15 +16,38 @@ pub struct AppCtx { pub messages: PgPool, pub stats: PgPool, pub redis: RedisPool, + + pub discord: Arc, } libpk::main!("scheduled_tasks"); async fn real_main() -> anyhow::Result<()> { + let mut client_builder = twilight_http::Client::builder().token( + libpk::config + .discord + .as_ref() + .expect("missing discord config") + .bot_token + .clone(), + ); + + if let Some(base_url) = libpk::config + .discord + .as_ref() + .expect("missing discord config") + .api_base_url + .clone() + { + client_builder = client_builder.proxy(base_url, true); + } + let ctx = AppCtx { data: libpk::db::init_data_db().await?, messages: libpk::db::init_messages_db().await?, stats: libpk::db::init_stats_db().await?, redis: libpk::db::init_redis().await?, + + discord: Arc::new(client_builder.build()), }; info!("starting scheduled tasks runner"); diff --git a/crates/scheduled_tasks/src/tasks.rs b/crates/scheduled_tasks/src/tasks.rs index 13622281..e0247768 100644 --- a/crates/scheduled_tasks/src/tasks.rs +++ b/crates/scheduled_tasks/src/tasks.rs @@ -25,7 +25,13 @@ pub async fn update_prometheus(ctx: AppCtx) -> anyhow::Result<()> { gauge!("pluralkit_image_cleanup_queue_length").set(count.count as f64); - // todo: remaining shard session_start_limit + let gateway = ctx.discord.gateway().authed().await?.model().await?; + + gauge!("pluralkit_gateway_sessions_remaining") + .set(gateway.session_start_limit.remaining as f64); + gauge!("pluralkit_gateway_sessions_reset_after") + .set(gateway.session_start_limit.reset_after as f64); + Ok(()) } From 2be3fe6a004ab0b055afcd937a2cc932d7cd9924 Mon Sep 17 00:00:00 2001 From: alyssa Date: Sat, 8 Mar 2025 18:57:13 +0000 Subject: [PATCH 14/18] fix: disable twilight ratelimiter when using proxy --- crates/gateway/src/discord/cache.rs | 2 +- crates/scheduled_tasks/src/main.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/gateway/src/discord/cache.rs b/crates/gateway/src/discord/cache.rs index 5f7d6e2c..b4a81664 100644 --- a/crates/gateway/src/discord/cache.rs +++ b/crates/gateway/src/discord/cache.rs @@ -105,7 +105,7 @@ pub fn new() -> DiscordCache { .api_base_url .clone() { - client_builder = client_builder.proxy(base_url, true); + client_builder = client_builder.proxy(base_url, true).ratelimiter(None); } let client = Arc::new(client_builder.build()); diff --git a/crates/scheduled_tasks/src/main.rs b/crates/scheduled_tasks/src/main.rs index 266325ed..cd0a182b 100644 --- a/crates/scheduled_tasks/src/main.rs +++ b/crates/scheduled_tasks/src/main.rs @@ -38,7 +38,7 @@ async fn real_main() -> anyhow::Result<()> { .api_base_url .clone() { - client_builder = client_builder.proxy(base_url, true); + client_builder = client_builder.proxy(base_url, true).ratelimiter(None); } let ctx = AppCtx { From 71e8cf960c90fe5a78385d8751d008914e67b12a Mon Sep 17 00:00:00 2001 From: alyssa Date: Sun, 9 Mar 2025 15:25:33 +0000 Subject: [PATCH 15/18] feat(bot): make links in help/stats embeds clickable --- PluralKit.Bot/Commands/Help.cs | 12 +++++++----- PluralKit.Bot/Commands/Misc.cs | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/PluralKit.Bot/Commands/Help.cs b/PluralKit.Bot/Commands/Help.cs index 09980f48..e148714b 100644 --- a/PluralKit.Bot/Commands/Help.cs +++ b/PluralKit.Bot/Commands/Help.cs @@ -12,10 +12,11 @@ public class Help "If PluralKit is useful to you, please consider donating on [Patreon](https://patreon.com/pluralkit) or [Buy Me A Coffee](https://buymeacoffee.com/pluralkit).\n" + "## Use the buttons below to see more info!"; + public static string EmbedFooter = "-# PluralKit by @ske and contributors | Myriad design by @layl, icon by @tedkalashnikov, banner by @fulmine | GitHub: https://github.com/PluralKit/PluralKit/ | Website: https://pluralkit.me/"; + public static Embed helpEmbed = new() { Title = "PluralKit", - Footer = new("PluralKit by @ske and contributors | Myriad design by @layl, icon by @tedkalashnikov, banner by @fulmine | GitHub: https://github.com/PluralKit/PluralKit/ | Website: https://pluralkit.me/"), Color = DiscordUtils.Blue, }; @@ -142,7 +143,7 @@ public class Help => ctx.Rest.CreateMessage(ctx.Channel.Id, new MessageRequest { Content = $"{Emojis.Warn} If you cannot see the rest of this message see [the FAQ]()", - Embeds = new[] { helpEmbed with { Description = Help.Description.Replace("{prefix}", ctx.DefaultPrefix) } }, + Embeds = new[] { helpEmbed with { Description = Help.Description.Replace("{prefix}", ctx.DefaultPrefix), Fields = new Embed.Field[] { new("", EmbedFooter) } } }, Components = new[] { helpPageButtons(ctx.Author.Id) }, }); @@ -156,7 +157,7 @@ public class Help if (ctx.Event.Message.Components.First().Components.Where(x => x.CustomId == ctx.CustomId).First().Style == ButtonStyle.Primary) return ctx.Respond(InteractionResponse.ResponseType.UpdateMessage, new() { - Embeds = new[] { helpEmbed with { Description = Help.Description.Replace("{prefix}", prefix) } }, + Embeds = new[] { helpEmbed with { Description = Help.Description.Replace("{prefix}", prefix), Fields = new Embed.Field[] { new("", EmbedFooter) } } }, Components = new[] { buttons } }); @@ -164,8 +165,9 @@ public class Help return ctx.Respond(InteractionResponse.ResponseType.UpdateMessage, new() { - Embeds = new[] { helpEmbed with { Fields = helpEmbedPages.GetValueOrDefault(ctx.CustomId.Split("-")[2]).Select((item, index) => - new Embed.Field(item.Name.Replace("{prefix}", prefix), item.Value.Replace("{prefix}", prefix))).ToArray() } }, + Embeds = new[] { helpEmbed with { Fields = helpEmbedPages.GetValueOrDefault(ctx.CustomId.Split("-")[2]).Select( + (item, index) => new Embed.Field(item.Name.Replace("{prefix}", prefix), item.Value.Replace("{prefix}", prefix)) + ).Append(new("", EmbedFooter)).ToArray() } }, Components = new[] { buttons } }); } diff --git a/PluralKit.Bot/Commands/Misc.cs b/PluralKit.Bot/Commands/Misc.cs index 93c90088..8688f9ef 100644 --- a/PluralKit.Bot/Commands/Misc.cs +++ b/PluralKit.Bot/Commands/Misc.cs @@ -92,7 +92,7 @@ public class Misc + $"**{stats.db.switches:N0}** switches, **{stats.db.messages:N0}** messages\n" + $"**{stats.db.guilds:N0}** servers with **{stats.db.channels:N0}** channels")); - embed.Footer(Help.helpEmbed.Footer); + embed.Field(new("", Help.EmbedFooter)); var uptime = ((DateTimeOffset)process.StartTime).ToUnixTimeSeconds(); embed.Description($"### PluralKit [{BuildInfoService.Version}](https://github.com/pluralkit/pluralkit/commit/{BuildInfoService.FullVersion})\n" + From e0c6839cd2ba02d509207740cd4e08a36eee5950 Mon Sep 17 00:00:00 2001 From: alyssa Date: Mon, 10 Mar 2025 15:16:32 +0000 Subject: [PATCH 16/18] fix(avatars): don't send image errors that aren't our fault to sentry --- crates/avatars/src/main.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/avatars/src/main.rs b/crates/avatars/src/main.rs index b880e326..3b621f52 100644 --- a/crates/avatars/src/main.rs +++ b/crates/avatars/src/main.rs @@ -24,7 +24,7 @@ use std::error::Error; use std::sync::Arc; use std::time::Duration; use thiserror::Error; -use tracing::{error, info}; +use tracing::{error, info, warn}; use uuid::Uuid; #[derive(Error, Debug)] @@ -262,7 +262,12 @@ impl IntoResponse for PKAvatarError { }; // print inner error if otherwise hidden - error!("error: {}", self.source().unwrap_or(&self)); + // `error!` calls go to sentry, so only use that if it's our error + if matches!(self, PKAvatarError::InternalError(_)) { + error!("error: {}", self.source().unwrap_or(&self)); + } else { + warn!("error: {}", self.source().unwrap_or(&self)); + } ( status_code, From 2b7f510e17849bfcdf2a3af19db6531d7c6760c9 Mon Sep 17 00:00:00 2001 From: Iris System Date: Tue, 11 Mar 2025 11:22:18 +1300 Subject: [PATCH 17/18] fix(bot): strip excessive linefeeds in reply embed generation --- PluralKit.Bot/Proxy/ProxyService.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/PluralKit.Bot/Proxy/ProxyService.cs b/PluralKit.Bot/Proxy/ProxyService.cs index ead5f1a6..43e1ed5e 100644 --- a/PluralKit.Bot/Proxy/ProxyService.cs +++ b/PluralKit.Bot/Proxy/ProxyService.cs @@ -399,6 +399,10 @@ public class ProxyService if (hasContent) { var msg = repliedTo.Content; + + // strip out overly excessive line breaks + msg = Regex.Replace(msg, @"(?:(?:([_\*]) \1)?\n){2,}", "\n"); + if (msg.Length > 100) { msg = repliedTo.Content.Substring(0, 100); From c4db95796d526cecd5789a195c41fb1f720377b8 Mon Sep 17 00:00:00 2001 From: Nidoskull <109077284+Nidoskull@users.noreply.github.com> Date: Sun, 16 Mar 2025 11:28:42 +0000 Subject: [PATCH 18/18] feat(bot): add "as text" to the redirect to the error support channel Many users send a screenshot of the error embed rather than sending the error code as text. Perhaps this would help? --- PluralKit.Bot/Services/ErrorMessageService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PluralKit.Bot/Services/ErrorMessageService.cs b/PluralKit.Bot/Services/ErrorMessageService.cs index 6dc0dc0d..50f47d1c 100644 --- a/PluralKit.Bot/Services/ErrorMessageService.cs +++ b/PluralKit.Bot/Services/ErrorMessageService.cs @@ -116,7 +116,7 @@ public class ErrorMessageService return new EmbedBuilder() .Color(0xE74C3C) .Title("Internal error occurred") - .Description($"For support, please send the error code above in {channelInfo} with a description of what you were doing at the time.") + .Description($"For support, please send the error code above as text in {channelInfo} with a description of what you were doing at the time.") .Footer(new Embed.EmbedFooter(errorId)) .Timestamp(now.ToDateTimeOffset().ToString("O")) .Build();