Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for unused errors, move federation errors into separate struct #5024

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions crates/apub/src/activities/block/block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@ use lemmy_db_schema::{
},
traits::{Bannable, Crud, Followable},
};
use lemmy_utils::{
error::{LemmyError, LemmyResult},
LemmyErrorType,
};
use lemmy_utils::error::{FederationError, LemmyError, LemmyResult};
use url::Url;

impl BlockUser {
Expand Down Expand Up @@ -135,7 +132,7 @@ impl ActivityHandler for BlockUser {
.object
.inner()
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?;
.ok_or(FederationError::UrlWithoutDomain)?;
if context.settings().hostname == domain {
return Err(
anyhow!("Site bans from remote instance can't affect user's home instance").into(),
Expand Down
8 changes: 4 additions & 4 deletions crates/apub/src/activities/community/announce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use lemmy_db_schema::{
source::{activity::ActivitySendTargets, community::CommunityFollower},
CommunityVisibility,
};
use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyErrorType, LemmyResult};
use serde_json::Value;
use url::Url;

Expand Down Expand Up @@ -54,7 +54,7 @@ impl ActivityHandler for RawAnnouncableActivities {

// This is only for sending, not receiving so we reject it.
if let AnnouncableActivities::Page(_) = activity {
Err(LemmyErrorType::CannotReceivePage)?
Err(FederationError::CannotReceivePage)?
}

// Need to treat community as optional here because `Delete/PrivateMessage` gets routed through
Expand Down Expand Up @@ -165,7 +165,7 @@ impl ActivityHandler for AnnounceActivity {

// This is only for sending, not receiving so we reject it.
if let AnnouncableActivities::Page(_) = object {
Err(LemmyErrorType::CannotReceivePage)?
Err(FederationError::CannotReceivePage)?
}

let community = object.community(context).await?;
Expand Down Expand Up @@ -216,7 +216,7 @@ async fn can_accept_activity_in_community(
if !community.local
&& !CommunityFollower::has_local_followers(&mut context.pool(), community.id).await?
{
Err(LemmyErrorType::CommunityHasNoFollowers)?
Err(FederationError::CommunityHasNoFollowers)?
}
// Local only community can't federate
if community.visibility != CommunityVisibility::Public {
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/activities/deletion/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use lemmy_db_schema::{
},
traits::{Crud, Reportable},
};
use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyErrorType, LemmyResult};
use url::Url;

#[async_trait::async_trait]
Expand Down Expand Up @@ -118,7 +118,7 @@ pub(in crate::activities) async fn receive_remove_action(
match DeletableObjects::read_from_db(object, context).await? {
DeletableObjects::Community(community) => {
if community.local {
Err(LemmyErrorType::OnlyLocalAdminCanRemoveCommunity)?
Err(FederationError::OnlyLocalAdminCanRemoveCommunity)?
}
let form = ModRemoveCommunityForm {
mod_person_id: actor.id,
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/activities/deletion/undo_delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use lemmy_db_schema::{
},
traits::Crud,
};
use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyErrorType, LemmyResult};
use url::Url;

#[async_trait::async_trait]
Expand Down Expand Up @@ -100,7 +100,7 @@ impl UndoDelete {
match DeletableObjects::read_from_db(object, context).await? {
DeletableObjects::Community(community) => {
if community.local {
Err(LemmyErrorType::OnlyLocalAdminCanRestoreCommunity)?
Err(FederationError::OnlyLocalAdminCanRestoreCommunity)?
}
let form = ModRemoveCommunityForm {
mod_person_id: actor.id,
Expand Down
12 changes: 6 additions & 6 deletions crates/apub/src/activities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use lemmy_db_schema::{
traits::Crud,
};
use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult};
use serde::Serialize;
use tracing::info;
use url::{ParseError, Url};
Expand Down Expand Up @@ -81,15 +81,15 @@ pub(crate) async fn verify_person_in_community(
) -> LemmyResult<()> {
let person = person_id.dereference(context).await?;
if person.banned {
Err(LemmyErrorType::PersonIsBannedFromSite(
Err(FederationError::PersonIsBannedFromSite(
person.actor_id.to_string(),
))?
}
let person_id = person.id;
let community_id = community.id;
let is_banned = CommunityPersonBanView::get(&mut context.pool(), person_id, community_id).await?;
if is_banned {
Err(LemmyErrorType::PersonIsBannedFromCommunity)?
Err(FederationError::PersonIsBannedFromCommunity)?
} else {
Ok(())
}
Expand Down Expand Up @@ -126,7 +126,7 @@ pub(crate) async fn verify_mod_action(

pub(crate) fn verify_is_public(to: &[Url], cc: &[Url]) -> LemmyResult<()> {
if ![to, cc].iter().any(|set| set.contains(&public())) {
Err(LemmyErrorType::ObjectIsNotPublic)?
Err(FederationError::ObjectIsNotPublic)?
} else {
Ok(())
}
Expand All @@ -138,15 +138,15 @@ where
{
let b: ObjectId<ApubCommunity> = b.into();
if a != &b {
Err(LemmyErrorType::InvalidCommunity)?
Err(FederationError::InvalidCommunity)?
} else {
Ok(())
}
}

pub(crate) fn check_community_deleted_or_removed(community: &Community) -> LemmyResult<()> {
if community.deleted || community.removed {
Err(LemmyErrorType::CannotCreatePostOrCommentInDeletedOrRemovedCommunity)?
Err(FederationError::CannotCreatePostOrCommentInDeletedOrRemovedCommunity)?
} else {
Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions crates/apub/src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use lemmy_db_schema::{
source::{activity::SentActivity, community::Community},
CommunityVisibility,
};
use lemmy_utils::error::{LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyErrorType, LemmyResult};
use serde::{Deserialize, Serialize};
use std::{ops::Deref, time::Duration};
use tokio::time::timeout;
Expand Down Expand Up @@ -45,7 +45,7 @@ pub async fn shared_inbox(
// consider the activity broken and move on.
timeout(INCOMING_ACTIVITY_TIMEOUT, receive_fut)
.await
.map_err(|_| LemmyErrorType::InboxTimeout)?
.map_err(|_| FederationError::InboxTimeout)?
}

/// Convert the data to json and turn it into an HTTP Response with the correct ActivityPub
Expand Down Expand Up @@ -108,7 +108,7 @@ pub(crate) async fn get_activity(
.into();
let activity = SentActivity::read_from_apub_id(&mut context.pool(), &activity_id)
.await?
.ok_or(LemmyErrorType::CouldntFindActivity)?;
.ok_or(FederationError::CouldntFindActivity)?;

let sensitive = activity.sensitive;
if sensitive {
Expand Down
23 changes: 12 additions & 11 deletions crates/apub/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use lemmy_db_schema::{
utils::{ActualDbPool, DbPool},
};
use lemmy_utils::{
error::{LemmyError, LemmyErrorType, LemmyResult},
error::{FederationError, LemmyError, LemmyErrorType, LemmyResult},
CACHE_DURATION_FEDERATION,
};
use moka::future::Cache;
Expand Down Expand Up @@ -51,17 +51,18 @@ impl UrlVerifier for VerifyUrlData {
let local_site_data = local_site_data_cached(&mut (&self.0).into())
.await
.expect("read local site data");
use FederationError::*;
check_apub_id_valid(url, &local_site_data).map_err(|err| match err {
LemmyError {
error_type: LemmyErrorType::FederationDisabled,
error_type: LemmyErrorType::FederationError(Some(FederationDisabled)),
..
} => ActivityPubError::Other("Federation disabled".into()),
LemmyError {
error_type: LemmyErrorType::DomainBlocked(domain),
error_type: LemmyErrorType::FederationError(Some(DomainBlocked(domain))),
..
} => ActivityPubError::Other(format!("Domain {domain:?} is blocked")),
LemmyError {
error_type: LemmyErrorType::DomainNotInAllowList(domain),
error_type: LemmyErrorType::FederationError(Some(DomainNotInAllowList(domain))),
..
} => ActivityPubError::Other(format!("Domain {domain:?} is not in allowlist")),
_ => ActivityPubError::Other("Failed validating apub id".into()),
Expand All @@ -81,7 +82,7 @@ impl UrlVerifier for VerifyUrlData {
fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> LemmyResult<()> {
let domain = apub_id
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?
.ok_or(FederationError::UrlWithoutDomain)?
.to_string();

if !local_site_data
Expand All @@ -90,15 +91,15 @@ fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> LemmyR
.map(|l| l.federation_enabled)
.unwrap_or(true)
{
Err(LemmyErrorType::FederationDisabled)?
Err(FederationError::FederationDisabled)?
}

if local_site_data
.blocked_instances
.iter()
.any(|i| domain.to_lowercase().eq(&i.domain.to_lowercase()))
{
Err(LemmyErrorType::DomainBlocked(domain.clone()))?
Err(FederationError::DomainBlocked(domain.clone()))?
}

// Only check this if there are instances in the allowlist
Expand All @@ -108,7 +109,7 @@ fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> LemmyR
.iter()
.any(|i| domain.to_lowercase().eq(&i.domain.to_lowercase()))
{
Err(LemmyErrorType::DomainNotInAllowList(domain))?
Err(FederationError::DomainNotInAllowList(domain))?
}

Ok(())
Expand Down Expand Up @@ -164,7 +165,7 @@ pub(crate) async fn check_apub_id_valid_with_strictness(
) -> LemmyResult<()> {
let domain = apub_id
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?
.ok_or(FederationError::UrlWithoutDomain)?
.to_string();
let local_instance = context
.settings()
Expand Down Expand Up @@ -194,10 +195,10 @@ pub(crate) async fn check_apub_id_valid_with_strictness(

let domain = apub_id
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?
.ok_or(FederationError::UrlWithoutDomain)?
.to_string();
if !allowed_and_local.contains(&domain) {
Err(LemmyErrorType::FederationDisabledByStrictAllowList)?
Err(FederationError::FederationDisabledByStrictAllowList)?
}
}
Ok(())
Expand Down
8 changes: 6 additions & 2 deletions crates/apub/src/mentions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ use lemmy_db_schema::{
traits::Crud,
utils::DbPool,
};
use lemmy_utils::{error::LemmyResult, utils::mention::scrape_text_for_mentions, LemmyErrorType};
use lemmy_utils::{
error::{FederationError, LemmyResult},
utils::mention::scrape_text_for_mentions,
LemmyErrorType,
};
use serde::{Deserialize, Serialize};
use serde_json::Value;
use url::Url;
Expand Down Expand Up @@ -57,7 +61,7 @@ pub async fn collect_non_local_mentions(
&parent_creator
.id()
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?
.ok_or(FederationError::UrlWithoutDomain)?
)),
kind: MentionType::Mention,
};
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/objects/comment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use lemmy_db_schema::{
utils::naive_now,
};
use lemmy_utils::{
error::{LemmyError, LemmyErrorType, LemmyResult},
error::{FederationError, LemmyError, LemmyErrorType, LemmyResult},
utils::markdown::markdown_to_html,
};
use std::ops::Deref;
Expand Down Expand Up @@ -157,7 +157,7 @@ impl Object for ApubComment {
.await
.is_ok();
if post.locked && !is_mod_or_admin {
Err(LemmyErrorType::PostIsLocked)?
Err(FederationError::PostIsLocked)?
} else {
Ok(())
}
Expand Down
9 changes: 4 additions & 5 deletions crates/apub/src/objects/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ use lemmy_db_schema::{
utils::naive_now,
};
use lemmy_utils::{
error::{LemmyError, LemmyResult},
error::{FederationError, LemmyError, LemmyResult},
utils::{
markdown::markdown_to_html,
slurs::{check_slurs, check_slurs_opt},
},
LemmyErrorType,
};
use std::ops::Deref;
use tracing::debug;
Expand Down Expand Up @@ -88,7 +87,7 @@ impl Object for ApubSite {
}

async fn delete(self, _data: &Data<Self::DataType>) -> LemmyResult<()> {
Err(LemmyErrorType::CantDeleteSite.into())
Err(FederationError::CantDeleteSite.into())
}

#[tracing::instrument(skip_all)]
Expand Down Expand Up @@ -143,7 +142,7 @@ impl Object for ApubSite {
.id
.inner()
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?;
.ok_or(FederationError::UrlWithoutDomain)?;
let instance = DbInstance::read_or_create(&mut context.pool(), domain.to_string()).await?;

let local_site = LocalSite::read(&mut context.pool()).await.ok();
Expand Down Expand Up @@ -218,7 +217,7 @@ pub(in crate::objects) async fn fetch_instance_actor_for_object<T: Into<Url> + C
debug!("Failed to dereference site for {}: {}", &instance_id, e);
let domain = instance_id
.domain()
.ok_or(LemmyErrorType::UrlWithoutDomain)?;
.ok_or(FederationError::UrlWithoutDomain)?;
Ok(
DbInstance::read_or_create(&mut context.pool(), domain.to_string())
.await?
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/objects/private_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use lemmy_db_schema::{
utils::naive_now,
};
use lemmy_utils::{
error::{LemmyError, LemmyErrorType, LemmyResult},
error::{FederationError, LemmyError, LemmyErrorType, LemmyResult},
utils::markdown::markdown_to_html,
};
use std::ops::Deref;
Expand Down Expand Up @@ -115,7 +115,7 @@ impl Object for ApubPrivateMessage {
check_apub_id_valid_with_strictness(note.id.inner(), false, context).await?;
let person = note.attributed_to.dereference(context).await?;
if person.banned {
Err(LemmyErrorType::PersonIsBannedFromSite(
Err(FederationError::PersonIsBannedFromSite(
person.actor_id.to_string(),
))?
} else {
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/protocol/activities/voting/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
};
use activitypub_federation::{config::Data, fetch::object_id::ObjectId};
use lemmy_api_common::context::LemmyContext;
use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyResult};
use serde::{Deserialize, Serialize};
use strum::Display;
use url::Url;
Expand Down Expand Up @@ -35,7 +35,7 @@ impl TryFrom<i16> for VoteType {
match value {
1 => Ok(VoteType::Like),
-1 => Ok(VoteType::Dislike),
_ => Err(LemmyErrorType::InvalidVoteValue.into()),
_ => Err(FederationError::InvalidVoteValue.into()),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/protocol/objects/page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use activitypub_federation::{
use chrono::{DateTime, Utc};
use itertools::Itertools;
use lemmy_api_common::context::LemmyContext;
use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use lemmy_utils::error::{FederationError, LemmyError, LemmyErrorType, LemmyResult};
use serde::{de::Error, Deserialize, Deserializer, Serialize};
use serde_with::skip_serializing_none;
use url::Url;
Expand Down Expand Up @@ -162,7 +162,7 @@ impl Page {
.iter()
.find(|a| a.kind == PersonOrGroupType::Person)
.map(|a| ObjectId::<ApubPerson>::from(a.id.clone().into_inner()))
.ok_or_else(|| LemmyErrorType::PageDoesNotSpecifyCreator.into()),
.ok_or_else(|| FederationError::PageDoesNotSpecifyCreator.into()),
}
}
}
Expand Down
Loading