refactor: capture errors with line numbers

refactor: update file api
This commit is contained in:
Paul Makles
2024-09-29 14:37:08 +01:00
parent 4fd66b2719
commit 3080ec1f5a
22 changed files with 273 additions and 130 deletions

View File

@@ -14,6 +14,7 @@ use futures::{
FutureExt, SinkExt, StreamExt, TryStreamExt,
};
use redis_kiss::{PayloadType, REDIS_PAYLOAD_TYPE, REDIS_URI};
use revolt_config::report_internal_error;
use revolt_database::{
events::{client::EventV1, server::ClientMessage},
Database, User, UserHint,
@@ -99,27 +100,21 @@ pub async fn client(db: &'static Database, stream: TcpStream, addr: SocketAddr)
let user_id = state.cache.user_id.clone();
// Notify socket we have authenticated.
if let Err(err) = write.send(config.encode(&EventV1::Authenticated)).await {
error!("Failed to write: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(write.send(config.encode(&EventV1::Authenticated)).await).is_err() {
return;
}
// Download required data to local cache and send Ready payload.
let ready_payload = match state
.generate_ready_payload(db, config.get_ready_payload_fields())
.await
{
let ready_payload = match report_internal_error!(
state
.generate_ready_payload(db, config.get_ready_payload_fields())
.await
) {
Ok(ready_payload) => ready_payload,
Err(err) => {
sentry::capture_error(&err);
return;
}
Err(_) => return,
};
if let Err(err) = write.send(config.encode(&ready_payload)).await {
error!("Failed to write: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(write.send(config.encode(&ready_payload)).await).is_err() {
return;
}
@@ -214,21 +209,16 @@ async fn listener(
write: &Mutex<WsWriter>,
) {
let redis_config = RedisConfig::from_url(&REDIS_URI).unwrap();
let subscriber = match fred::types::Builder::from_config(redis_config).build_subscriber_client()
{
let subscriber = match report_internal_error!(
fred::types::Builder::from_config(redis_config).build_subscriber_client()
) {
Ok(subscriber) => subscriber,
Err(err) => {
error!("Failed to build a subscriber: {err:?}");
sentry::capture_error(&err);
return;
}
Err(_) => return,
};
if let Err(err) = subscriber.init().await {
error!("Failed to init subscriber: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(subscriber.init().await).is_err() {
return;
};
}
// Handle Redis connection dropping
let (clean_up_s, clean_up_r) = async_channel::bounded(1);
@@ -249,17 +239,13 @@ async fn listener(
// Check for state changes for subscriptions.
match state.apply_state().await {
SubscriptionStateChange::Reset => {
if let Err(err) = subscriber.unsubscribe_all().await {
error!("Unsubscribe all failed: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(subscriber.unsubscribe_all().await).is_err() {
break 'out;
}
let subscribed = state.subscribed.read().await;
for id in subscribed.iter() {
if let Err(err) = subscriber.subscribe(id).await {
error!("Subscribe failed: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(subscriber.subscribe(id).await).is_err() {
break 'out;
}
}
@@ -272,9 +258,7 @@ async fn listener(
#[cfg(debug_assertions)]
info!("{addr:?} unsubscribing from {id}");
if let Err(err) = subscriber.unsubscribe(id).await {
error!("Unsubscribe failed: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(subscriber.unsubscribe(id).await).is_err() {
break 'out;
}
}
@@ -283,9 +267,7 @@ async fn listener(
#[cfg(debug_assertions)]
info!("{addr:?} subscribing to {id}");
if let Err(err) = subscriber.subscribe(id).await {
error!("Subscribe failed: {err:?}");
sentry::capture_error(&err);
if report_internal_error!(subscriber.subscribe(id).await).is_err() {
break 'out;
}
}
@@ -310,13 +292,9 @@ async fn listener(
_ = t2 => {},
message = t1 => {
// Handle incoming events.
let message = match message {
let message = match report_internal_error!(message) {
Ok(message) => message,
Err(e) => {
error!("Error while consuming pub/sub messages: {e:?}");
sentry::capture_error(&e);
break 'out;
}
Err(_) => break 'out
};
let event = match *REDIS_PAYLOAD_TYPE {
@@ -393,10 +371,7 @@ async fn listener(
}
}
if let Err(err) = subscriber.quit().await {
error!("{}", err);
sentry::capture_error(&err);
}
report_internal_error!(subscriber.quit().await).ok();
}
#[allow(clippy::too_many_arguments)]

View File

@@ -6,7 +6,7 @@ use futures_locks::RwLock;
use once_cell::sync::Lazy;
use serde::Deserialize;
pub use sentry::capture_error;
pub use sentry::{capture_error, capture_message, Level};
#[cfg(feature = "report-macros")]
#[macro_export]
@@ -14,7 +14,10 @@ macro_rules! report_error {
( $expr: expr, $error: ident $( $tt:tt )? ) => {
$expr
.inspect_err(|err| {
$crate::capture_error(err);
$crate::capture_message(
&format!("{err:?} ({}:{}:{})", file!(), line!(), column!()),
$crate::Level::Error,
);
})
.map_err(|_| ::revolt_result::create_error!($error))
};
@@ -26,7 +29,10 @@ macro_rules! report_internal_error {
( $expr: expr ) => {
$expr
.inspect_err(|err| {
$crate::capture_error(err);
$crate::capture_message(
&format!("{err:?} ({}:{}:{})", file!(), line!(), column!()),
$crate::Level::Error,
);
})
.map_err(|_| ::revolt_result::create_error!(InternalError))
};

View File

@@ -24,7 +24,9 @@ default = ["mongodb", "async-std-runtime", "tasks"]
[dependencies]
# Core
revolt-config = { version = "0.7.16", path = "../config" }
revolt-config = { version = "0.7.16", path = "../config", features = [
"report-macros",
] }
revolt-result = { version = "0.7.16", path = "../result" }
revolt-models = { version = "0.7.16", path = "../models", features = [
"validator",

View File

@@ -5,7 +5,7 @@ use crate::{
bson::{doc, from_bson, from_document, to_document, Bson, DateTime, Document},
options::FindOptions,
},
Invite, MongoDb, DISCRIMINATOR_SEARCH_SPACE,
AbstractChannels, AbstractServers, Channel, Invite, MongoDb, DISCRIMINATOR_SEARCH_SPACE,
};
use bson::oid::ObjectId;
use futures::StreamExt;
@@ -20,7 +20,7 @@ struct MigrationInfo {
revision: i32,
}
pub const LATEST_REVISION: i32 = 29;
pub const LATEST_REVISION: i32 = 30;
pub async fn migrate_database(db: &MongoDb) {
let migrations = db.col::<Document>("migrations");
@@ -1139,6 +1139,54 @@ pub async fn run_migrations(db: &MongoDb, revision: i32) -> i32 {
.expect("Failed to create attachment_hashes index.");
}
if revision <= 29 {
info!("Running migration [revision 29 / 29-09-2024]: Add creator_id to webhooks.");
#[derive(serde::Serialize, serde::Deserialize)]
struct WebhookShell {
_id: String,
channel_id: String,
}
let invites = db
.db()
.collection::<WebhookShell>("channel_webhooks")
.find(doc! {}, None)
.await
.expect("webhooks")
.filter_map(|s| async { s.ok() })
.collect::<Vec<WebhookShell>>()
.await;
for invite in invites {
let channel = db.fetch_channel(&invite.channel_id).await.expect("channel");
let creator_id = match channel {
Channel::Group { owner, .. } => owner,
Channel::TextChannel { server, .. } | Channel::VoiceChannel { server, .. } => {
let server = db.fetch_server(&server).await.expect("server");
server.owner
}
_ => unreachable!("not server or group channel!"),
};
db.db()
.collection::<Document>("channel_webhooks")
.update_one(
doc! {
"_id": invite._id,
},
doc! {
"$set" : {
"creator_id": creator_id
}
},
None,
)
.await
.expect("update webhook");
}
}
// Need to migrate fields on attachments, change `user_id`, `object_id`, etc to `parent`.
// Reminder to update LATEST_REVISION when adding new migrations.

View File

@@ -17,6 +17,9 @@ auto_derived_partial!(
#[serde(skip_serializing_if = "Option::is_none")]
pub avatar: Option<File>,
/// User that created this webhook
pub creator_id: String,
/// The channel this webhook belongs to
pub channel_id: String,
@@ -43,6 +46,7 @@ impl Default for Webhook {
id: Default::default(),
name: Default::default(),
avatar: None,
creator_id: Default::default(),
channel_id: Default::default(),
permissions: Default::default(),
token: Default::default(),

View File

@@ -19,9 +19,11 @@ auto_derived_partial!(
/// When this file was uploaded
pub uploaded_at: Option<Timestamp>, // these are Option<>s to not break file uploads on legacy Autumn
/// ID of user who uploaded this file
#[serde(skip_serializing_if = "Option::is_none")]
pub uploader_id: Option<String>, // these are Option<>s to not break file uploads on legacy Autumn
/// What the file was used for
#[serde(skip_serializing_if = "Option::is_none")]
pub used_for: Option<FileUsedFor>,
/// Whether this file was deleted
@@ -63,6 +65,7 @@ auto_derived!(
ServerBanner,
Emoji,
UserAvatar,
WebhookAvatar,
UserProfileBackground,
LegacyGroupIcon,
ChannelIcon,
@@ -86,44 +89,154 @@ impl File {
}
/// Use a file for a message attachment
pub async fn use_attachment(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "attachments", "message", parent)
.await
pub async fn use_attachment(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"attachments",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::Message,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for a user profile background
pub async fn use_background(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "backgrounds", "user", parent)
.await
pub async fn use_background(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"backgrounds",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::UserProfileBackground,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for a user avatar
pub async fn use_avatar(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "avatars", "user", parent)
.await
pub async fn use_user_avatar(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"avatars",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::UserAvatar,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for an icon
pub async fn use_icon(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "icons", "object", parent)
.await
/// Use a file for a webhook avatar
pub async fn use_webhook_avatar(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"avatars",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::WebhookAvatar,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for a server icon
pub async fn use_server_icon(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "icons", "object", parent)
.await
pub async fn use_server_icon(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"icons",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::ServerIcon,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for a channel icon
pub async fn use_channel_icon(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"icons",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::ChannelIcon,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for a server banner
pub async fn use_banner(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "banners", "server", parent)
.await
pub async fn use_server_banner(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"banners",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::ServerBanner,
},
uploader_id.to_owned(),
)
.await
}
/// Use a file for an emoji
pub async fn use_emoji(db: &Database, id: &str, parent: &str) -> Result<File> {
db.find_and_use_attachment(id, "emojis", "object", parent)
.await
pub async fn use_emoji(
db: &Database,
id: &str,
parent: &str,
uploader_id: &str,
) -> Result<File> {
db.find_and_use_attachment(
id,
"emojis",
FileUsedFor {
id: parent.to_owned(),
object_type: FileUsedForType::Emoji,
},
uploader_id.to_owned(),
)
.await
}
}

View File

@@ -2,6 +2,8 @@ use revolt_result::Result;
use crate::File;
use super::FileUsedFor;
mod mongodb;
mod reference;
@@ -18,8 +20,8 @@ pub trait AbstractAttachments: Sync + Send {
&self,
id: &str,
tag: &str,
parent_type: &str,
parent_id: &str,
used_for: FileUsedFor,
uploader_id: String,
) -> Result<File>;
/// Mark an attachment as having been reported.

View File

@@ -1,7 +1,10 @@
use bson::to_document;
use bson::Document;
use revolt_config::report_internal_error;
use revolt_result::Result;
use crate::File;
use crate::FileUsedFor;
use crate::MongoDb;
use super::AbstractAttachments;
@@ -34,10 +37,9 @@ impl AbstractAttachments for MongoDb {
&self,
id: &str,
tag: &str,
parent_type: &str,
parent_id: &str,
used_for: FileUsedFor,
uploader_id: String,
) -> Result<File> {
let key = format!("{parent_type}_id");
let file = query!(
self,
find_one,
@@ -45,7 +47,7 @@ impl AbstractAttachments for MongoDb {
doc! {
"_id": id,
"tag": tag,
&key: {
"used_for": {
"$exists": false
}
}
@@ -59,7 +61,8 @@ impl AbstractAttachments for MongoDb {
},
doc! {
"$set": {
key: parent_id
"used_for": report_internal_error!(to_document(&used_for))?,
"uploader_id": uploader_id
}
},
None,

View File

@@ -1,6 +1,7 @@
use revolt_result::Result;
use crate::File;
use crate::FileUsedFor;
use crate::ReferenceDb;
use super::AbstractAttachments;
@@ -37,19 +38,14 @@ impl AbstractAttachments for ReferenceDb {
&self,
id: &str,
tag: &str,
parent_type: &str,
parent_id: &str,
used_for: FileUsedFor,
uploader_id: String,
) -> Result<File> {
let mut files = self.files.lock().await;
if let Some(file) = files.get_mut(id) {
if file.tag == tag {
match parent_type {
"message" => file.message_id = Some(parent_id.to_owned()),
"user" => file.user_id = Some(parent_id.to_owned()),
"object" => file.object_id = Some(parent_id.to_owned()),
"server" => file.server_id = Some(parent_id.to_owned()),
_ => unreachable!(),
}
file.uploader_id = Some(uploader_id);
file.used_for = Some(used_for);
Ok(file.clone())
} else {

View File

@@ -370,10 +370,8 @@ impl Message {
}
for attachment_id in data.attachments.as_deref().unwrap_or_default() {
attachments.push(
db.find_and_use_attachment(attachment_id, "attachments", "message", &message_id)
.await?,
);
attachments
.push(File::use_attachment(db, attachment_id, &message_id, author.id()).await?);
}
if !attachments.is_empty() {
@@ -499,10 +497,7 @@ impl Message {
})?;
let media = if let Some(id) = embed.media {
Some(
db.find_and_use_attachment(&id, "attachments", "message", &self.id)
.await?,
)
Some(File::use_attachment(db, &id, &self.id, &self.author).await?)
} else {
None
};
@@ -661,7 +656,7 @@ impl Message {
) -> Result<()> {
let media: Option<v0::File> = if let Some(id) = embed.media {
Some(
db.find_and_use_attachment(&id, "attachments", "message", &self.id)
File::use_attachment(db, &id, &self.id, &self.author)
.await?
.into(),
)

View File

@@ -20,6 +20,7 @@ use crate::Database;
/// Payload information, before assembly
#[derive(Debug)]
#[allow(non_snake_case)]
pub struct ApnPayload {
message: Message,
url: String,
@@ -29,6 +30,7 @@ pub struct ApnPayload {
}
#[derive(Serialize, Debug)]
#[allow(non_snake_case)]
struct Payload<'a> {
aps: APS<'a>,
#[serde(skip_serializing)]

View File

@@ -11,7 +11,7 @@ use base64::{
use deadqueue::limited::Queue;
use fcm_v1::auth::{Authenticator, ServiceAccountKey};
use once_cell::sync::Lazy;
use revolt_config::config;
use revolt_config::{config, report_internal_error};
use revolt_models::v0::PushNotification;
use revolt_presence::filter_online;
use serde_json::json;
@@ -116,12 +116,11 @@ pub async fn worker(db: Database, authifier_db: AuthifierDatabase) {
if fcm_error.contains("404 (Not Found)") {
println!("Unregistering {:?}", session.id);
if let Err(err) = db
.remove_push_subscription_by_session_id(&session.id)
.await
{
revolt_config::capture_error(&err);
}
report_internal_error!(
db.remove_push_subscription_by_session_id(&session.id)
.await
)
.ok();
}
}
} else {

View File

@@ -108,6 +108,7 @@ impl From<crate::Webhook> for Webhook {
id: value.id,
name: value.name,
avatar: value.avatar.map(|file| file.into()),
creator_id: value.creator_id,
channel_id: value.channel_id,
token: value.token,
permissions: value.permissions,
@@ -121,6 +122,7 @@ impl From<crate::PartialWebhook> for PartialWebhook {
id: value.id,
name: value.name,
avatar: value.avatar.map(|file| file.into()),
creator_id: value.creator_id,
channel_id: value.channel_id,
token: value.token,
permissions: value.permissions,

View File

@@ -16,6 +16,9 @@ auto_derived_partial!(
#[cfg_attr(feature = "serde", serde(skip_serializing_if = "Option::is_none"))]
pub avatar: Option<File>,
/// User that created this webhook
pub creator_id: String,
/// The channel this webhook belongs to
pub channel_id: String,

View File

@@ -124,7 +124,7 @@ pub async fn edit(
}
if let Some(icon_id) = data.icon {
partial.icon = Some(File::use_icon(db, &icon_id, id).await?);
partial.icon = Some(File::use_channel_icon(db, &icon_id, id, &user.id).await?);
*icon = partial.icon.clone();
}

View File

@@ -1,6 +1,6 @@
use revolt_database::{
util::{permissions::DatabasePermissionQuery, reference::Reference},
Channel, Database, User, Webhook,
Channel, Database, File, User, Webhook,
};
use revolt_models::v0;
use revolt_permissions::{
@@ -43,10 +43,7 @@ pub async fn create_webhook(
let webhook_id = Ulid::new().to_string();
let avatar = match &data.avatar {
Some(id) => Some(
db.find_and_use_attachment(id, "avatars", "user", &webhook_id)
.await?,
),
Some(id) => Some(File::use_webhook_avatar(db, id, &webhook_id, &user.id).await?),
None => None,
};
@@ -54,6 +51,7 @@ pub async fn create_webhook(
id: webhook_id,
name: data.name,
avatar,
creator_id: user.id,
channel_id: channel.id().to_string(),
permissions: *DEFAULT_WEBHOOK_PERMISSIONS,
token: Some(nanoid::nanoid!(64)),

View File

@@ -55,7 +55,7 @@ pub async fn create_emoji(
};
// Find the relevant attachment
let attachment = File::use_emoji(db, &id, &id).await?;
let attachment = File::use_emoji(db, &id, &id, &user.id).await?;
// Create the emoji object
let emoji = Emoji {

View File

@@ -142,7 +142,7 @@ pub async fn edit(
// 2. Apply new avatar
if let Some(avatar) = avatar {
partial.avatar = Some(File::use_avatar(db, &avatar, &user.id).await?);
partial.avatar = Some(File::use_user_avatar(db, &avatar, &user.id, &user.id).await?);
}
member

View File

@@ -138,13 +138,13 @@ pub async fn edit(
// 3. Apply new icon
if let Some(icon) = icon {
partial.icon = Some(File::use_server_icon(db, &icon, &server.id).await?);
partial.icon = Some(File::use_server_icon(db, &icon, &server.id, &user.id).await?);
server.icon = partial.icon.clone();
}
// 4. Apply new banner
if let Some(banner) = banner {
partial.banner = Some(File::use_banner(db, &banner, &server.id).await?);
partial.banner = Some(File::use_server_banner(db, &banner, &server.id, &user.id).await?);
server.banner = partial.banner.clone();
}

View File

@@ -89,7 +89,7 @@ pub async fn edit(
// 2. Apply new avatar
if let Some(avatar) = data.avatar {
partial.avatar = Some(File::use_avatar(db, &avatar, &user.id).await?);
partial.avatar = Some(File::use_user_avatar(db, &avatar, &user.id, &user.id).await?);
}
// 3. Apply new status
@@ -114,7 +114,8 @@ pub async fn edit(
}
if let Some(background) = profile.background {
new_profile.background = Some(File::use_background(db, &background, &user.id).await?);
new_profile.background =
Some(File::use_background(db, &background, &user.id, &user.id).await?);
}
partial.profile = Some(new_profile);

View File

@@ -1,6 +1,6 @@
use revolt_database::{
util::{permissions::DatabasePermissionQuery, reference::Reference},
Database, PartialWebhook, User,
Database, File, PartialWebhook, User,
};
use revolt_models::v0::{DataEditWebhook, Webhook};
use revolt_permissions::{calculate_channel_permissions, ChannelPermission};
@@ -52,10 +52,7 @@ pub async fn webhook_edit(
};
if let Some(avatar) = avatar {
let file = db
.find_and_use_attachment(&avatar, "avatars", "user", &webhook.id)
.await?;
let file = File::use_webhook_avatar(db, &avatar, &webhook.id, &webhook.creator_id).await?;
partial.avatar = Some(file)
}

View File

@@ -1,5 +1,5 @@
use revolt_database::util::reference::Reference;
use revolt_database::{Database, PartialWebhook};
use revolt_database::{Database, File, PartialWebhook};
use revolt_models::v0::{DataEditWebhook, Webhook};
use revolt_models::validator::Validate;
use revolt_result::{create_error, Result};
@@ -34,7 +34,7 @@ pub async fn webhook_edit_token(
name,
avatar,
permissions,
remove
remove,
} = data;
let mut partial = PartialWebhook {
@@ -44,10 +44,7 @@ pub async fn webhook_edit_token(
};
if let Some(avatar) = avatar {
let file = db
.find_and_use_attachment(&avatar, "avatars", "user", &webhook.id)
.await?;
let file = File::use_webhook_avatar(db, &avatar, &webhook.id, &webhook.creator_id).await?;
partial.avatar = Some(file)
}