From 56a374d7777c2f7c8b2f1f3d3d65eb9ccba1eb0f Mon Sep 17 00:00:00 2001
From: Ada Bohm <ada@kreatrix.org>
Date: Mon, 19 Aug 2024 12:58:25 +0200
Subject: [PATCH] Better error message when submitting to open job

---
 .../src/client/commands/submit/command.rs     | 38 ++++++++++++-------
 crates/hyperqueue/src/server/client/submit.rs | 18 ++++-----
 crates/hyperqueue/src/transfer/messages.rs    |  9 +++--
 3 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/crates/hyperqueue/src/client/commands/submit/command.rs b/crates/hyperqueue/src/client/commands/submit/command.rs
index 28817e724..6d0bcd12e 100644
--- a/crates/hyperqueue/src/client/commands/submit/command.rs
+++ b/crates/hyperqueue/src/client/commands/submit/command.rs
@@ -36,7 +36,8 @@ use crate::common::utils::time::parse_human_time;
 use crate::transfer::connection::ClientSession;
 use crate::transfer::messages::{
     FromClientMessage, IdSelector, JobDescription, JobSubmitDescription, JobTaskDescription,
-    PinMode, SubmitRequest, TaskDescription, TaskKind, TaskKindProgram, ToClientMessage,
+    PinMode, SubmitRequest, SubmitResponse, TaskDescription, TaskKind, TaskKindProgram,
+    ToClientMessage,
 };
 use crate::{rpc_call, JobId, JobTaskCount, Map};
 
@@ -703,22 +704,33 @@ pub(crate) async fn send_submit_request(
     wait: bool,
     progress: bool,
 ) -> anyhow::Result<()> {
+    let job_id = request.job_id.unwrap_or_else(|| JobId::new(0));
     let message = FromClientMessage::Submit(request);
 
     let response =
         rpc_call!(session.connection(), message, ToClientMessage::SubmitResponse(r) => r).await?;
-    let info = response.job.info.clone();
-
-    gsettings.printer().print_job_submitted(response.job);
-    if wait {
-        wait_for_jobs(
-            gsettings,
-            session,
-            IdSelector::Specific(IntArray::from_id(info.id.into())),
-        )
-        .await?;
-    } else if progress {
-        wait_for_jobs_with_progress(session, &[info]).await?;
+
+    match response {
+        SubmitResponse::Ok { job, server_uid: _ } => {
+            let info = job.info.clone();
+
+            gsettings.printer().print_job_submitted(job);
+            if wait {
+                wait_for_jobs(
+                    gsettings,
+                    session,
+                    IdSelector::Specific(IntArray::from_id(info.id.into())),
+                )
+                .await?;
+            } else if progress {
+                wait_for_jobs_with_progress(session, &[info]).await?;
+            }
+        }
+        SubmitResponse::JobNotOpened => bail!("Job {job_id} is not opened."),
+        SubmitResponse::JobNotFound => bail!("Job {job_id} not found."),
+        SubmitResponse::TaskIdAlreadyExists(task_id) => {
+            bail!("Task {task_id} already exists in job {job_id}.")
+        }
     }
     Ok(())
 }
diff --git a/crates/hyperqueue/src/server/client/submit.rs b/crates/hyperqueue/src/server/client/submit.rs
index a94b8621c..f91aee94b 100644
--- a/crates/hyperqueue/src/server/client/submit.rs
+++ b/crates/hyperqueue/src/server/client/submit.rs
@@ -76,7 +76,7 @@ pub(crate) async fn handle_submit(
     let (job_id, new_job) = if let Some(job_id) = message.job_id {
         if let Some(job) = state.get_job(job_id) {
             if !job.is_open {
-                return ToClientMessage::Error(format!("Job {job_id} is not open"));
+                return ToClientMessage::SubmitResponse(SubmitResponse::JobNotOpened);
             }
             match &mut message.submit_desc.task_desc {
                 JobTaskDescription::Array { ids, entries, .. } => {
@@ -93,9 +93,9 @@ pub(crate) async fn handle_submit(
                         for id in ids.iter() {
                             let id = JobTaskId::new(id);
                             if id_set.contains(&id) {
-                                return ToClientMessage::Error(format!(
-                                    "Task {id} already exists in job {job_id}"
-                                ));
+                                return ToClientMessage::SubmitResponse(
+                                    SubmitResponse::TaskIdAlreadyExists(id),
+                                );
                             }
                         }
                     }
@@ -105,15 +105,15 @@ pub(crate) async fn handle_submit(
                     for task in tasks {
                         if id_set.contains(&task.id) {
                             let id = task.id;
-                            return ToClientMessage::Error(format!(
-                                "Task {id} already exists in job {job_id}"
-                            ));
+                            return ToClientMessage::SubmitResponse(
+                                SubmitResponse::TaskIdAlreadyExists(id),
+                            );
                         }
                     }
                 }
             }
         } else {
-            return ToClientMessage::Error(format!("Job {job_id} not found"));
+            return ToClientMessage::SubmitResponse(SubmitResponse::JobNotFound);
         }
         (job_id, false)
     } else {
@@ -181,7 +181,7 @@ pub(crate) async fn handle_submit(
         r => panic!("Invalid response: {r:?}"),
     };
 
-    ToClientMessage::SubmitResponse(SubmitResponse {
+    ToClientMessage::SubmitResponse(SubmitResponse::Ok {
         job: job_detail,
         server_uid: state_ref.get().server_info().server_uid.clone(),
     })
diff --git a/crates/hyperqueue/src/transfer/messages.rs b/crates/hyperqueue/src/transfer/messages.rs
index 9aa501de9..5487adcf4 100644
--- a/crates/hyperqueue/src/transfer/messages.rs
+++ b/crates/hyperqueue/src/transfer/messages.rs
@@ -386,9 +386,12 @@ pub struct StatsResponse {
 }
 
 #[derive(Serialize, Deserialize, Debug)]
-pub struct SubmitResponse {
-    pub job: JobDetail,
-    pub server_uid: String,
+#[allow(clippy::large_enum_variant)]
+pub enum SubmitResponse {
+    Ok { job: JobDetail, server_uid: String },
+    JobNotOpened,
+    JobNotFound,
+    TaskIdAlreadyExists(JobTaskId),
 }
 
 #[derive(Serialize, Deserialize, Debug)]