Allow edit failures to be dispatched to users as an internal error, prevent empty components on embed send (#234)

Author: Headline

src/cache.rs

@@ -20,7 +20,7 @@ use lru_cache::LruCache;
 use serenity::all::{ApplicationId, CommandInteraction, ShardManager};
 use serenity::model::channel::Message;
 
-/** Caching **/
+/* Caching */
 
 /// Contains bot configuration information provided mostly from environment variables
 pub struct ConfigCache;

src/commands/asm.rs

@@ -6,7 +6,6 @@ use serenity::{
 };
 
 use crate::cache::{CompilerCache, ConfigCache, LinkAPICache, MessageCache, MessageCacheEntry};
-use crate::utls::constants::*;
 use crate::utls::discordhelpers;
 
 use crate::managers::compilation::CompilationDetails;
@@ -41,8 +40,7 @@ pub async fn asm(ctx: &Context, msg: &Message, _args: Args) -> CommandResult {
     let asm_embed = msg.channel_id.send_message(&ctx.http, new_msg).await?;
 
     // Success/fail react
-    let compilation_successful = asm_embed.embeds[0].colour.unwrap().0 == COLOR_OKAY;
-    discordhelpers::send_completion_react(ctx, &asm_embed, compilation_successful).await?;
+    discordhelpers::send_completion_react(ctx, &asm_embed, compilation_details.success).await?;
 
     let data_read = ctx.data.read().await;
     let mut message_cache = data_read.get::<MessageCache>().unwrap().lock().await;

src/commands/compile.rs

@@ -3,7 +3,6 @@ use std::fmt::Write as _;
 use serenity::framework::standard::{macros::command, Args, CommandResult};
 
 use crate::cache::{LinkAPICache, MessageCache, MessageCacheEntry};
-use crate::utls::constants::COLOR_OKAY;
 use crate::utls::discordhelpers::embeds;
 use crate::utls::{discordhelpers, parser};
 
@@ -46,8 +45,7 @@ pub async fn compile(ctx: &Context, msg: &Message, _args: Args) -> CommandResult
     let sent = msg.channel_id.send_message(&ctx.http, new_msg).await?;
 
     // Success/fail react
-    let compilation_successful = sent.embeds[0].colour.unwrap().0 == COLOR_OKAY;
-    discordhelpers::send_completion_react(ctx, &sent, compilation_successful).await?;
+    discordhelpers::send_completion_react(ctx, &sent, compilation_details.success).await?;
 
     let mut delete_cache = data_read.get::<MessageCache>().unwrap().lock().await;
     delete_cache.insert(msg.id.get(), MessageCacheEntry::new(sent, msg.clone()));
@@ -123,7 +121,7 @@ pub async fn handle_request(
     // remove our loading emote
     let _ = discordhelpers::delete_bot_reacts(&ctx, msg, loading_reaction).await;
 
-    let is_success = &result.0.success;
+    let is_success = result.0.success;
     {
         // stats manager is used in events.rs, lets keep our locks very short
         let stats = data_read.get::<StatsManagerCache>().unwrap().lock().await;
@@ -142,7 +140,7 @@ pub async fn handle_request(
                 "<<unknown>>".to_owned()
             };
             let emb = embeds::build_complog_embed(
-                *is_success,
+                is_success,
                 &parse_result.code,
                 &parse_result.target,
                 &msg.author.name,

src/events.rs

@@ -187,17 +187,33 @@ impl EventHandler for Handler {
                 .map(|msg| msg.clone())
         };
 
-        if let Some(msg) = maybe_message {
+        if let Some(mut msg) = maybe_message {
             if let Some(new_msg) = new_data.content {
                 if let Some(author) = new_data.author {
-                    discordhelpers::handle_edit(
+                    if let Err(e) = discordhelpers::handle_edit(
                         &ctx,
                         new_msg,
-                        author,
+                        author.clone(),
                         msg.our_msg.clone(),
                         msg.original_msg.clone(),
                     )
-                    .await;
+                    .await
+                    {
+                        let emb = embeds::build_fail_embed(
+                            &author,
+                            &format!("An internal error occurred doing your request:\n{}", e),
+                        );
+                        if let Err(e) = embeds::edit_message_embed(
+                            &ctx,
+                            &mut msg.our_msg,
+                            &mut emb.clone(),
+                            None,
+                        )
+                        .await
+                        {
+                            error!("Could not edit sent message! Original error: {}", e);
+                        }
+                    }
                 }
             }
         }

src/managers/compilation.rs

@@ -306,6 +306,7 @@ impl CompilationManager {
 
         builder.build(wbox)?;
         let res = builder.dispatch().await?;
+        details.success = res.status.eq("0");
         Ok((details, res))
     }
 }

src/tests/mod.rs

@@ -1,5 +1,5 @@
 pub mod boilerplate;
 pub mod cpp;
 #[cfg(test)]
-#[cfg_attr(feature = "cargo-clippy", allow(clippy::all))]
+#[cfg_attr(feature = "clippy", allow(clippy::all))]
 pub mod parser;

src/utls/discordhelpers/embeds.rs

@@ -1,6 +1,9 @@
 use std::fmt::Write as _;
 use std::{env, str};
 
+use crate::apis::insights::InsightsResponse;
+use crate::cache::LinkAPICache;
+use crate::managers::compilation::CompilationDetails;
 use serenity::all::{CreateActionRow, CreateButton, CreateEmbedFooter, EditMessage};
 use serenity::http::Http;
 use serenity::{
@@ -9,10 +12,6 @@ use serenity::{
     model::prelude::*,
 };
 
-use crate::apis::insights::InsightsResponse;
-use crate::cache::LinkAPICache;
-use crate::managers::compilation::CompilationDetails;
-
 use crate::utls::constants::*;
 use crate::utls::discordhelpers;
 
@@ -49,19 +48,16 @@ impl ToEmbed for wandbox::CompilationResult {
 
         if !self.signal.is_empty() {
             // If we received 'Signal', then the application successfully ran, but was timed out
-            // by wandbox. We should skin this as successful, so we set status to 0 (success).
-            // This is done to ensure that the checkmark is added at the end of the compile
-            // command hook.
+            // by wandbox. We should skin this as successful as it causes confusion as to who
+            // actually failed (ourselves vs wandbox)
             embed = embed.color(COLOR_OKAY);
         }
         if !self.compiler_all.is_empty() {
-            let str =
-                discordhelpers::conform_external_str(&self.compiler_all, MAX_ERROR_LEN, true);
+            let str = discordhelpers::conform_external_str(&self.compiler_all, MAX_ERROR_LEN, true);
             embed = embed.field("Compiler Output", format!("```{}\n```", str), false);
         }
         if !self.program_all.is_empty() {
-            let str =
-                discordhelpers::conform_external_str(&self.program_all, MAX_OUTPUT_LEN, true);
+            let str = discordhelpers::conform_external_str(&self.program_all, MAX_OUTPUT_LEN, true);
             embed = embed.field("Program Output", format!("```\n{}\n```", str), false);
         }
         if !self.url.is_empty() {
@@ -186,14 +182,8 @@ impl ToEmbed for godbolt::GodboltResponse {
             }
 
             if !output {
-                embed = embed
-                    .title("Compilation successful")
-                    .description("No output.");
+                embed = embed.title("Compilation successful");
             }
-
-            // Execution time can be displayed here, but I don't think it's useful enough
-            // to show...
-            //embed.field("Execution Time", format!("`{}ms`", self.execution_time), true);
         }
 
         let mut appendstr = author.name.clone();
@@ -215,17 +205,17 @@ impl ToEmbed for godbolt::GodboltResponse {
 pub async fn edit_message_embed(
     ctx: &Context,
     old: &mut Message,
-    emb: CreateEmbed,
+    emb: &mut CreateEmbed,
     compilation_details: Option<CompilationDetails>,
-) {
+) -> serenity::Result<()> {
     let mut url = None;
     if let Some(details) = compilation_details {
         let data = ctx.data.read().await;
         if let Some(link_cache) = data.get::<LinkAPICache>() {
             if let Some(b64) = details.base64 {
                 let long_url = format!("https://godbolt.org/clientstate/{}", b64);
                 let link_cache_lock = link_cache.read().await;
-                url = link_cache_lock.get_link(long_url).await
+                url = link_cache_lock.get_link(long_url).await;
             }
         }
     }
@@ -236,10 +226,18 @@ pub async fn edit_message_embed(
         btns.push(CreateButton::new_link(shorturl).label("View on godbolt.org"));
     }
 
-    let edit = EditMessage::new()
-        .embed(emb)
-        .components(vec![CreateActionRow::Buttons(btns)]);
-    let _ = old.edit(ctx, edit).await;
+    let edit = {
+        if btns.is_empty() {
+            EditMessage::default().embed(emb.clone())
+        } else {
+            EditMessage::default()
+                .components(vec![CreateActionRow::Buttons(btns)])
+                .embed(emb.clone())
+        }
+    };
+
+    old.edit(ctx, edit).await?;
+    Ok(())
 }
 
 pub fn build_insights_response_embed(author: &User, res: InsightsResponse) -> CreateEmbed {

src/utls/discordhelpers/mod.rs

@@ -91,7 +91,7 @@ pub async fn handle_edit(
     author: User,
     mut old: Message,
     original_message: Message,
-) {
+) -> serenity::Result<()> {
     let prefix = {
         let data = ctx.data.read().await;
         let info = data.get::<ConfigCache>().unwrap().read().await;
@@ -122,8 +122,8 @@ pub async fn handle_edit(
         )
         .await
         {
-            let err = embeds::build_fail_embed(&author, &e.to_string());
-            embeds::edit_message_embed(ctx, &mut old, err, None).await;
+            let mut err = embeds::build_fail_embed(&author, &e.to_string());
+            embeds::edit_message_embed(ctx, &mut old, &mut err, None).await?;
         }
     } else if content.starts_with(&format!("{}compile", prefix)) {
         if let Err(e) = handle_edit_compile(
@@ -135,8 +135,8 @@ pub async fn handle_edit(
         )
         .await
         {
-            let err = embeds::build_fail_embed(&author, &e.to_string());
-            embeds::edit_message_embed(ctx, &mut old, err, None).await;
+            let mut err = embeds::build_fail_embed(&author, &e.to_string());
+            embeds::edit_message_embed(ctx, &mut old, &mut err, None).await?;
         }
     } else if content.starts_with(&format!("{}cpp", prefix)) {
         if let Err(e) = handle_edit_cpp(
@@ -148,8 +148,8 @@ pub async fn handle_edit(
         )
         .await
         {
-            let err = embeds::build_fail_embed(&author, &e.to_string());
-            embeds::edit_message_embed(ctx, &mut old, err, None).await;
+            let mut err = embeds::build_fail_embed(&author, &e.to_string());
+            embeds::edit_message_embed(ctx, &mut old, &mut err, None).await?;
         }
     } else if content.starts_with(&format!("{}insights", prefix)) {
         if let Err(e) = handle_edit_insights(
@@ -161,13 +161,15 @@ pub async fn handle_edit(
         )
         .await
         {
-            let err = embeds::build_fail_embed(&author, &e.to_string());
-            embeds::edit_message_embed(ctx, &mut old, err, None).await;
+            let mut err = embeds::build_fail_embed(&author, &e.to_string());
+            embeds::edit_message_embed(ctx, &mut old, &mut err, None).await?;
         }
     } else {
-        let err = embeds::build_fail_embed(&author, "Invalid command for edit functionality!");
-        embeds::edit_message_embed(ctx, &mut old, err, None).await;
+        let mut err = embeds::build_fail_embed(&author, "Invalid command for edit functionality!");
+        embeds::edit_message_embed(ctx, &mut old, &mut err, None).await?;
     }
+
+    Ok(())
 }
 
 pub async fn handle_edit_insights(
@@ -177,14 +179,13 @@ pub async fn handle_edit_insights(
     mut old: Message,
     original_msg: Message,
 ) -> CommandResult {
-    let (details, embed) =
+    let (details, mut embed) =
         crate::commands::insights::handle_request(ctx.clone(), content, author, &original_msg)
             .await?;
 
-    let compilation_successful = details.success;
-    discordhelpers::send_completion_react(ctx, &old, compilation_successful).await?;
+    discordhelpers::send_completion_react(ctx, &old, details.success).await?;
 
-    embeds::edit_message_embed(ctx, &mut old, embed, None).await;
+    embeds::edit_message_embed(ctx, &mut old, &mut embed, None).await?;
     Ok(())
 }
 
@@ -195,13 +196,12 @@ pub async fn handle_edit_cpp(
     mut old: Message,
     original_msg: Message,
 ) -> CommandResult {
-    let (embed, details) =
+    let (mut embed, details) =
         crate::commands::cpp::handle_request(ctx.clone(), content, author, &original_msg).await?;
 
-    let compilation_successful = details.success;
-    discordhelpers::send_completion_react(ctx, &old, compilation_successful).await?;
+    discordhelpers::send_completion_react(ctx, &old, details.success).await?;
 
-    embeds::edit_message_embed(ctx, &mut old, embed, Some(details)).await;
+    embeds::edit_message_embed(ctx, &mut old, &mut embed, Some(details)).await?;
     Ok(())
 }
 
@@ -212,13 +212,13 @@ pub async fn handle_edit_compile(
     mut old: Message,
     original_msg: Message,
 ) -> CommandResult {
-    let (embed, compilation_details) =
+    let (mut embed, compilation_details) =
         compile::handle_request(ctx.clone(), content, author, &original_msg).await?;
 
     let compilation_successful = compilation_details.success;
     discordhelpers::send_completion_react(ctx, &old, compilation_successful).await?;
 
-    embeds::edit_message_embed(ctx, &mut old, embed, Some(compilation_details)).await;
+    embeds::edit_message_embed(ctx, &mut old, &mut embed, Some(compilation_details)).await?;
     Ok(())
 }
 
@@ -229,13 +229,11 @@ pub async fn handle_edit_asm(
     mut old: Message,
     original_msg: Message,
 ) -> CommandResult {
-    let (emb, details) =
+    let (mut emb, details) =
         crate::commands::asm::handle_request(ctx.clone(), content, author, &original_msg).await?;
 
-    let success = details.success;
-    embeds::edit_message_embed(ctx, &mut old, emb, Some(details)).await;
-
-    send_completion_react(ctx, &old, success).await?;
+    send_completion_react(ctx, &old, details.success).await?;
+    embeds::edit_message_embed(ctx, &mut old, &mut emb, Some(details)).await?;
     Ok(())
 }