Evaluate different error cases and actually retry

Author: NotNorom

src/database/guild_schedule.rs

@@ -178,15 +178,15 @@ impl Entry for GuildSchedule {
 
         match db.client.hgetall(Self::key(db, id)).await {
             Ok(schedule) => Ok(Some(schedule)),
-            Err(err) if *err.kind() == ErrorKind::NotFound => return Ok(None),
+            Err(err) if *err.kind() == ErrorKind::NotFound => Ok(None),
             Err(err) => Err(err),
         }
     }
 
     async fn delete(db: &Database, id: impl Into<Key> + Send + Sync) -> Result<Self, Error> {
         let id: Key = id.into();
 
-        let schedule = match Self::get(&db, id.clone()).await {
+        let schedule = match Self::get(db, id.clone()).await {
             Ok(Some(schedule)) => schedule,
             Ok(None) => {
                 return Err(Error::new(

src/error.rs

@@ -4,7 +4,6 @@ use std::{
     num::NonZeroU16,
 };
 
-use async_repeater::RepeaterHandle;
 use chrono::{DateTime, Utc};
 use poise::serenity_prelude::{
     Context, Error as SerenityError, HttpError as SerenityHttpError, JsonErrorCode, MessageBuilder, User,
@@ -16,8 +15,6 @@ use tracing::{info, instrument, warn};
 
 use crate::{
     Settings,
-    database::{Database, guild_schedule::GuildSchedule},
-    schedule::Schedule,
     schedule_runner::{RunnerError, ScheduleAction},
     setting_banner::SetBannerError,
     settings::SettingsError,
@@ -165,11 +162,9 @@ where
 /// This is a needed as well as the normal error handling in [crate::error::on_error] because
 /// the scheduler is running in its own task
 #[instrument(skip_all)]
-pub async fn handle_schedule_error(
+pub async fn evaluate_schedule_error(
     error: &RunnerError,
     ctx: Context,
-    repeater_handle: RepeaterHandle<Schedule>,
-    db: Database,
     owners: HashSet<UserId>,
 ) -> Result<ScheduleAction, Error> {
     let guild_id = error.schedule().guild_id();
@@ -180,7 +175,7 @@ pub async fn handle_schedule_error(
     let message = MessageBuilder::new()
         .push_bold("Error in guild: ")
         .push_mono_line_safe(&*guild_name)
-        .push_codeblock(&*error.to_string(), Some("rust"))
+        .push(&*error.to_string())
         .build();
 
     dm_users(&ctx, owners.clone(), &message).await?;
@@ -192,18 +187,10 @@ pub async fn handle_schedule_error(
                     match error_response.status_code {
                         StatusCode::FORBIDDEN => {
                             // the bot does not have permissions to change the banner.
-                            // remove guild from queue
-                            let _ = repeater_handle.remove(guild_id).await;
-                            db.delete::<GuildSchedule>(error.schedule().guild_id().get())
-                                .await?;
                             warn!("Missing permissions to change banner for {guild_id}. Unscheduling.");
                             return Ok(ScheduleAction::Abort);
                         }
                         StatusCode::NOT_FOUND => {
-                            let _ = repeater_handle.remove(guild_id).await;
-                            db.delete::<GuildSchedule>(error.schedule().guild_id().get())
-                                .await?;
-
                             if error_response.error.code == JsonErrorCode::UnknownChannel {
                                 warn!(
                                     "Channel {channel_id} does not exist in guild: {guild_id}. Unscheduling."
@@ -239,24 +226,17 @@ pub async fn handle_schedule_error(
                                 match error_response.status_code {
                                     StatusCode::FORBIDDEN => {
                                         // the bot does not have permissions to change the banner.
-                                        // remove guild from queue
-                                        let _ = repeater_handle.remove(guild_id).await;
-                                        db.delete::<GuildSchedule>(error.schedule().guild_id().get())
-                                            .await?;
                                         warn!(
                                             "Missing permissions to change banner for {guild_id}. Unscheduling."
                                         );
                                         return Ok(ScheduleAction::Abort);
                                     }
                                     StatusCode::NOT_FOUND => {
-                                        let _ = repeater_handle.remove(guild_id).await;
-                                        db.delete::<GuildSchedule>(error.schedule().guild_id().get())
-                                            .await?;
                                         warn!("Guild does not exist: {guild_id}. Unscheduling.");
                                         return Ok(ScheduleAction::Abort);
                                     }
                                     StatusCode::GATEWAY_TIMEOUT => {
-                                        warn!("Gateway timed out. Retrying once.");
+                                        warn!("Gateway timed out. Retrying");
                                         return Ok(ScheduleAction::RetrySameImage);
                                     }
                                     _ => tracing::error!("unsuccessful http request: {error_response:?}"),
@@ -269,39 +249,41 @@ pub async fn handle_schedule_error(
                         }
                     }
                 }
-                SetBannerError::CouldNotPickAUrl => warn!("guild_id={guild_id}: 'Could not pick a url'"),
-                SetBannerError::CouldNotDeterminFileExtension => {
-                    warn!("guild_id={guild_id}: 'Could not determine file extenstion'");
+                SetBannerError::CouldNotPickAUrl => {
+                    warn!("guild_id={guild_id}: 'Could not pick a url'. RNG failed")
+                }
+                SetBannerError::CouldNotDeterminFileExtension(url) => {
+                    warn!("guild_id={guild_id}: 'Could not determine file extenstion. url={url}'");
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
                 SetBannerError::MissingBannerFeature => {
-                    let _ = repeater_handle.remove(guild_id).await;
-                    db.delete::<GuildSchedule>(error.schedule().guild_id().get())
-                        .await?;
-
                     let partial_guild = guild_id.to_partial_guild(&ctx.http).await?;
                     let guild_owner = partial_guild.owner_id;
-                    info!(
+                    warn!(
                         "Letting owner={guild_owner} of guild={guild_id} know about the missing banner feature"
                     );
 
                     dm_user(&ctx, guild_owner, "Server has lost the required boost level. Stopping schedule. You can restart the bot after gaining the required boost level.").await?;
+                    return Ok(ScheduleAction::Abort);
                 }
                 SetBannerError::MissingAnimatedBannerFeature(url, ..) => {
                     warn!(
                         "guild_id={guild_id} with channel={channel_id} was trying to set an animated banner but does not have the feature. url={url}"
                     );
                     let partial_guild = guild_id.to_partial_guild(&ctx.http).await?;
                     let guild_owner = partial_guild.owner_id;
-                    info!(
+                    warn!(
                         "Letting owner={guild_owner} of guild={guild_id} know about the missing animated banner feature"
                     );
 
                     dm_user(&ctx, guild_owner, &format!("Tried to set an animated banner but the server '{}' does not have the required boost level for animated banners", partial_guild.name)).await?;
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
                 SetBannerError::ImageIsEmpty(url, ..) => {
                     warn!(
                         "guild_id={guild_id} with channel={channel_id} has selected an image with 0 bytes. url={url}"
                     );
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
                 SetBannerError::ImageIsTooBig(url, ..) => {
                     warn!(
@@ -315,16 +297,19 @@ pub async fn handle_schedule_error(
                     );
 
                     dm_user(&ctx, guild_owner, &format!("The channel you've set contains an image that is too big for discord. Maximum size is 10mb. The image is: {url}")).await?;
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
                 SetBannerError::ImageUnkownSize(url, ..) => {
                     warn!(
                         "guild_id={guild_id} with channel={channel_id} has selected an image with unknown size. url={url}"
                     );
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
                 SetBannerError::Base64Encoding(url, ..) => {
                     warn!(
                         "guild_id={guild_id} with channel={channel_id} has selected an image wich could be encoded into base 64. url={url}"
                     );
+                    return Ok(ScheduleAction::RetryNewImage);
                 }
             }
         }

src/schedule_runner.rs

@@ -1,6 +1,6 @@
 use std::num::NonZeroUsize;
 
-use poise::serenity_prelude::GuildId;
+use poise::serenity_prelude::{GuildId, Message};
 use tokio::pin;
 use tokio_stream::StreamExt;
 use tracing::{debug, error, instrument};
@@ -11,7 +11,7 @@ use crate::{
     database::{Database, guild_schedule::GuildSchedule},
     finding_media::find_media_in_channel,
     schedule::Schedule,
-    setting_banner::RandomBanner,
+    setting_banner::{BannerFromUrl, RandomBanner, SetBannerError},
 };
 
 pub struct ScheduleRunner {
@@ -36,12 +36,34 @@ impl ScheduleRunner {
         }
     }
 
+    /// This is run every time a schedule is due
     #[instrument(skip_all)]
-    pub async fn run(self) -> Result<Url, RunnerError> {
+    pub async fn run(
+        &self,
+        avoid_list: &[Url],
+        pick_this: Option<(Url, Message)>,
+    ) -> Result<Url, RunnerError> {
         let schedule = self.schedule.clone();
         let mut guild_id = schedule.guild_id();
-        let channel = schedule.channel_id();
 
+        // if we have an override image given, just use it and skip the rest of the function
+        if let Some((url, message)) = pick_this {
+            debug!("Using override image: {url}");
+            guild_id
+                .set_banner_from_url_and_message(self.ctx.http.clone(), &self.http_client, &url, &message)
+                .await
+                .map_err(|err| RunnerError::new(err.into(), guild_id, self.schedule.clone()))?;
+            debug!("Inserting schedule into database");
+            let schedule = GuildSchedule::from(schedule.clone());
+
+            self.database
+                .insert(&schedule, schedule.guild_id())
+                .await
+                .map_err(|err| RunnerError::new(err.into(), guild_id, self.schedule.clone()))?;
+            return Ok(url);
+        };
+
+        let channel = schedule.channel_id();
         let limit = schedule.message_limit().map_or(usize::MAX, NonZeroUsize::get);
 
         debug!("Fetching images, limited to {} messages", limit);
@@ -50,7 +72,7 @@ impl ScheduleRunner {
         pin!(stream_of_media);
 
         let mut images = Vec::new();
-        while let Some(url) = stream_of_media
+        while let Some(url_message_pair) = stream_of_media
             .try_next()
             .await
             .map_err(|err| RunnerError::new(err.into(), guild_id, self.schedule.clone()))?
@@ -61,7 +83,11 @@ impl ScheduleRunner {
                 )
             })
         {
-            images.push(url);
+            if avoid_list.contains(&url_message_pair.0) {
+                continue;
+            }
+
+            images.push(url_message_pair);
         }
 
         let img_count = images.len();
@@ -85,9 +111,13 @@ impl ScheduleRunner {
 
 #[derive(Debug)]
 pub enum ScheduleAction {
+    /// Everything's fine
     Continue,
+    /// Use this url next run
     RetrySameImage,
+    /// Avoid this image
     RetryNewImage,
+    /// OH GOD WHY
     Abort,
 }
 
@@ -117,6 +147,26 @@ impl RunnerError {
         &self.schedule
     }
 
+    pub fn attempted_url_and_message(&self) -> (Option<Url>, Option<Box<Message>>) {
+        match &self.source {
+            Error::SetBanner(set_banner_error) => match set_banner_error {
+                SetBannerError::Transport(_) => (None, None),
+                SetBannerError::DiscordApi(_) => (None, None),
+                SetBannerError::CouldNotPickAUrl => (None, None),
+                SetBannerError::CouldNotDeterminFileExtension(url) => (Some(url.clone()), None),
+                SetBannerError::MissingBannerFeature => (None, None),
+                SetBannerError::MissingAnimatedBannerFeature(url, message) => {
+                    (Some(url.clone()), Some(message.clone()))
+                }
+                SetBannerError::ImageIsEmpty(url, message) => (Some(url.clone()), Some(message.clone())),
+                SetBannerError::ImageIsTooBig(url, message) => (Some(url.clone()), Some(message.clone())),
+                SetBannerError::ImageUnkownSize(url, message) => (Some(url.clone()), Some(message.clone())),
+                SetBannerError::Base64Encoding(url, message) => (Some(url.clone()), Some(message.clone())),
+            },
+            _ => (None, None),
+        }
+    }
+
     pub fn source(&self) -> &crate::Error {
         &self.source
     }

src/setting_banner.rs

@@ -26,7 +26,7 @@ pub enum SetBannerError {
     CouldNotPickAUrl,
 
     #[error("Could not determin file extenstion on image")]
-    CouldNotDeterminFileExtension,
+    CouldNotDeterminFileExtension(Url),
 
     #[error("Missing 'banner' feature")]
     MissingBannerFeature,
@@ -74,7 +74,7 @@ impl BannerFromUrl for GuildId {
             .path()
             .split('.')
             .next_back()
-            .ok_or_else(|| SetBannerError::CouldNotDeterminFileExtension)?;
+            .ok_or_else(|| SetBannerError::CouldNotDeterminFileExtension(url.clone()))?;
 
         debug!("Found extension: {extension}");
         // Disable banner feature check when in dev environment