Merge pull request #467 from arayabrain/feature/improve-api-error-han…

…dling

Improve API Error Handling, etc

frontend/src/api/experiments/Experiments.ts

@@ -96,7 +96,7 @@ export async function downloadExperimentConfigApi(
   return response.data
 }
 
-export async function renameExperiment(
+export async function renameExperimentApi(
   workspaceId: number,
   uid: string,
   new_name: string,

frontend/src/components/Workspace/Experiment/Button/DeleteButton.tsx

@@ -1,6 +1,8 @@
 import { memo, useContext, useState } from "react"
 import { useSelector, useDispatch } from "react-redux"
 
+import { useSnackbar } from "notistack"
+
 import DeleteOutlineIcon from "@mui/icons-material/DeleteOutline"
 import IconButton from "@mui/material/IconButton"
 
@@ -26,12 +28,20 @@ export const DeleteButton = memo(function DeleteButton() {
   })
   const name = useSelector(selectExperimentName(uid))
   const [open, setOpen] = useState(false)
+  const { enqueueSnackbar } = useSnackbar()
 
   const openDialog = () => {
     setOpen(true)
   }
   const handleDelete = () => {
     dispatch(deleteExperimentByUid(uid))
+      .unwrap()
+      .then(() => {
+        // do nothing.
+      })
+      .catch(() => {
+        enqueueSnackbar("Failed to delete", { variant: "error" })
+      })
     uid === currentPipelineUid && dispatch(clearCurrentPipeline())
   }
 

frontend/src/components/Workspace/Experiment/ExperimentTable.tsx

@@ -12,6 +12,8 @@ import {
 } from "react"
 import { useSelector, useDispatch } from "react-redux"
 
+import { useSnackbar } from "notistack"
+
 import DeleteIcon from "@mui/icons-material/Delete"
 import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown"
 import KeyboardArrowUpIcon from "@mui/icons-material/KeyboardArrowUp"
@@ -34,7 +36,7 @@ import TableRow from "@mui/material/TableRow"
 import TableSortLabel from "@mui/material/TableSortLabel"
 import Typography from "@mui/material/Typography"
 
-import { renameExperiment } from "api/experiments/Experiments"
+import { renameExperimentApi } from "api/experiments/Experiments"
 import { ConfirmDialog } from "components/common/ConfirmDialog"
 import { useLocalStorage } from "components/utils/LocalStorageUtil"
 import { DeleteButton } from "components/Workspace/Experiment/Button/DeleteButton"
@@ -122,6 +124,8 @@ const TableImple = memo(function TableImple() {
     const isPending = selectPipelineIsStartedSuccess(state)
     return checkedList.includes(currentUid as string) && isPending
   })
+  const { enqueueSnackbar } = useSnackbar()
+
   const onClickReload = () => {
     dispatch(getExperiments())
   }
@@ -157,6 +161,14 @@ const TableImple = memo(function TableImple() {
   }
   const onClickOk = () => {
     dispatch(deleteExperimentByList(checkedList))
+      .unwrap()
+      .then(() => {
+        // do nothing.
+      })
+      .catch(() => {
+        enqueueSnackbar("Failed to delete", { variant: "error" })
+      })
+
     checkedList.filter((v) => v === currentPipelineUid).length > 0 &&
       dispatch(clearCurrentPipeline())
     setCheckedList([])
@@ -424,6 +436,7 @@ const RowItem = memo(function RowItem({
   const [errorEdit, setErrorEdit] = useState("")
   const [valueEdit, setValueEdit] = useState(name)
   const dispatch = useDispatch<AppDispatch>()
+  const { enqueueSnackbar } = useSnackbar()
 
   const onBlurEdit = (event: FocusEvent) => {
     event.preventDefault()
@@ -451,7 +464,13 @@ const RowItem = memo(function RowItem({
 
   const onSaveNewName = async () => {
     if (valueEdit === name || workspaceId === void 0) return
-    await renameExperiment(workspaceId, uid, valueEdit)
+
+    try {
+      await renameExperimentApi(workspaceId, uid, valueEdit)
+    } catch (e) {
+      enqueueSnackbar("Failed to rename", { variant: "error" })
+    }
+
     dispatch(getExperiments())
   }
 

frontend/src/const/API.ts

@@ -1,10 +1,13 @@
 const HOST =
-  process.env.REACT_APP_SERVER_HOST ?? window.location.hostname ?? "localhost"
-const PORT = process.env.REACT_APP_SERVER_PORT ?? window.location.port ?? 8000
+  process.env.REACT_APP_SERVER_HOST || window.location.hostname || "localhost"
 const PROTO =
-  process.env.REACT_APP_SERVER_PROTO ??
-  window.location.protocol.replace(":", "") ??
+  process.env.REACT_APP_SERVER_PROTO ||
+  window.location.protocol.replace(":", "") ||
   "http"
+const PORT =
+  process.env.REACT_APP_SERVER_PORT ||
+  window.location.port ||
+  (PROTO == "https" ? 443 : 8000)
 
 export const BASE_URL =
   PORT == null ? `${PROTO}://${HOST}` : `${PROTO}://${HOST}:${PORT}`

frontend/src/store/slice/Pipeline/PipelineHook.ts

@@ -88,18 +88,28 @@ export function useRunPipeline() {
   const filePathIsUndefined = useSelector(selectFilePathIsUndefined)
   const algorithmNodeNotExist = useSelector(selectAlgorithmNodeNotExist)
   const runPostData = useSelector(selectRunPostData)
+  const { enqueueSnackbar } = useSnackbar()
+
   const handleRunPipeline = useCallback(
     (name: string) => {
       dispatch(run({ runPostData: { name, ...runPostData, forceRunList: [] } }))
+        .unwrap()
+        .catch(() => {
+          enqueueSnackbar("Failed to Run workflow", { variant: "error" })
+        })
     },
-    [dispatch, runPostData],
+    [dispatch, enqueueSnackbar, runPostData],
   )
   const handleClickVariant = (variant: VariantType, mess: string) => {
     enqueueSnackbar(mess, { variant })
   }
   const handleRunPipelineByUid = useCallback(() => {
     dispatch(runByCurrentUid({ runPostData }))
-  }, [dispatch, runPostData])
+      .unwrap()
+      .catch(() => {
+        enqueueSnackbar("Failed to Run workflow", { variant: "error" })
+      })
+  }, [dispatch, enqueueSnackbar, runPostData])
   const handleCancelPipeline = useCallback(async () => {
     if (uid != null) {
       const data = await dispatch(cancelResult({ uid }))
@@ -123,7 +133,6 @@ export function useRunPipeline() {
     }
   }, [dispatch, uid, isCanceled, isStartedSuccess])
   const status = useSelector(selectPipelineStatus)
-  const { enqueueSnackbar } = useSnackbar()
   // タブ移動による再レンダリングするたびにスナックバーが実行されてしまう挙動を回避するために前回の値を保持
   const [prevStatus, setPrevStatus] = useState(status)
   useEffect(() => {

studio/app/common/core/experiment/experiment_writer.py

@@ -48,7 +48,7 @@ def write(self) -> None:
         else:
             self.create_config()
 
-        self.function_from_nodeDict()
+        self.build_function_from_nodeDict()
 
         ConfigWriter.write(
             dirname=join_filepath(
@@ -72,12 +72,14 @@ def create_config(self) -> ExptConfig:
 
     def add_run_info(self) -> ExptConfig:
         return (
-            self.builder.set_started_at(datetime.now().strftime(DATE_FORMAT))  # 時間を更新
+            self.builder.set_started_at(
+                datetime.now().strftime(DATE_FORMAT)
+            )  # Update time
             .set_success("running")
             .build()
         )
 
-    def function_from_nodeDict(self) -> ExptConfig:
+    def build_function_from_nodeDict(self) -> ExptConfig:
         func_dict: Dict[str, ExptFunction] = {}
         node_dict = WorkflowConfigReader.read(
             join_filepath(
@@ -113,10 +115,13 @@ def __init__(
         self.unique_id = unique_id
 
     def delete_data(self) -> bool:
+        result = True
+
         shutil.rmtree(
             join_filepath([DIRPATH.OUTPUT_DIR, self.workspace_id, self.unique_id])
         )
-        return True
+
+        return result
 
     def rename(self, new_name: str) -> ExptConfig:
         filepath = join_filepath(
@@ -128,6 +133,9 @@ def rename(self, new_name: str) -> ExptConfig:
             ]
         )
 
+        # validate params
+        new_name = "" if new_name is None else new_name  # filter None
+
         # Note: "r+" option is not used here because it requires file pointer control.
         with open(filepath, "r") as f:
             config = yaml.safe_load(f)

studio/app/common/routers/experiment.py

@@ -2,12 +2,13 @@
 from glob import glob
 from typing import Dict
 
-from fastapi import APIRouter, Depends, HTTPException
+from fastapi import APIRouter, Depends, HTTPException, status
 from fastapi.responses import FileResponse
 
 from studio.app.common.core.experiment.experiment import ExptConfig
 from studio.app.common.core.experiment.experiment_reader import ExptConfigReader
 from studio.app.common.core.experiment.experiment_writer import ExptDataWriter
+from studio.app.common.core.logger import AppLogger
 from studio.app.common.core.utils.filepath_creater import join_filepath
 from studio.app.common.core.workspace.workspace_dependencies import (
     is_workspace_available,
@@ -18,6 +19,8 @@
 
 router = APIRouter(prefix="/experiments", tags=["experiments"])
 
+logger = AppLogger.get_logger()
+
 
 @router.get(
     "/{workspace_id}",
@@ -33,7 +36,8 @@ async def get_experiments(workspace_id: str):
         try:
             config = ExptConfigReader.read(path)
             exp_config[config.unique_id] = config
-        except Exception:
+        except Exception as e:
+            logger.error(e, exc_info=True)
             pass
 
     return exp_config
@@ -49,10 +53,18 @@ async def rename_experiment(workspace_id: str, unique_id: str, item: RenameItem)
         workspace_id,
         unique_id,
     ).rename(item.new_name)
-    config.nodeDict = []
-    config.edgeDict = []
+    try:
+        config.nodeDict = []
+        config.edgeDict = []
+
+        return config
 
-    return config
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="rename experiment failed",
+        )
 
 
 @router.delete(
@@ -67,8 +79,12 @@ async def delete_experiment(workspace_id: str, unique_id: str):
             unique_id,
         ).delete_data()
         return True
-    except Exception:
-        return False
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="can not delete record.",
+        )
 
 
 @router.post(
@@ -84,8 +100,12 @@ async def delete_experiment_list(workspace_id: str, deleteItem: DeleteItem):
                 unique_id,
             ).delete_data()
         return True
-    except Exception:
-        return False
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="can not delete record.",
+        )
 
 
 @router.get(
@@ -99,4 +119,6 @@ async def download_config_experiment(workspace_id: str, unique_id: str):
     if os.path.exists(config_filepath):
         return FileResponse(config_filepath)
     else:
-        raise HTTPException(status_code=404, detail="file not found")
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND, detail="file not found"
+        )

studio/app/common/routers/run.py

@@ -1,7 +1,7 @@
 import uuid
 from typing import Dict
 
-from fastapi import APIRouter, BackgroundTasks, Depends
+from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, status
 
 from studio.app.common.core.logger import AppLogger
 from studio.app.common.core.workflow.workflow import Message, NodeItem, RunItem
@@ -23,12 +23,20 @@
     dependencies=[Depends(is_workspace_owner)],
 )
 async def run(workspace_id: str, runItem: RunItem, background_tasks: BackgroundTasks):
-    unique_id = str(uuid.uuid4())[:8]
-    WorkflowRunner(workspace_id, unique_id, runItem).run_workflow(background_tasks)
+    try:
+        unique_id = str(uuid.uuid4())[:8]
+        WorkflowRunner(workspace_id, unique_id, runItem).run_workflow(background_tasks)
 
-    logger.info("run snakemake")
+        logger.info("run snakemake")
 
-    return unique_id
+        return unique_id
+
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Failed to run workflow.",
+        )
 
 
 @router.post(
@@ -39,12 +47,20 @@ async def run(workspace_id: str, runItem: RunItem, background_tasks: BackgroundT
 async def run_id(
     workspace_id: str, uid: str, runItem: RunItem, background_tasks: BackgroundTasks
 ):
-    WorkflowRunner(workspace_id, uid, runItem).run_workflow(background_tasks)
+    try:
+        WorkflowRunner(workspace_id, uid, runItem).run_workflow(background_tasks)
+
+        logger.info("run snakemake")
+        logger.info("forcerun list: %s", runItem.forceRunList)
 
-    logger.info("run snakemake")
-    logger.info("forcerun list: %s", runItem.forceRunList)
+        return uid
 
-    return uid
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Failed to run workflow.",
+        )
 
 
 @router.post(
@@ -53,7 +69,14 @@ async def run_id(
     dependencies=[Depends(is_workspace_available)],
 )
 async def run_result(workspace_id: str, uid: str, nodeDict: NodeItem):
-    return WorkflowResult(workspace_id, uid).get(nodeDict.pendingNodeIdList)
+    try:
+        return WorkflowResult(workspace_id, uid).get(nodeDict.pendingNodeIdList)
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Failed to result workflow.",
+        )
 
 
 @router.post(
@@ -62,4 +85,11 @@ async def run_result(workspace_id: str, uid: str, nodeDict: NodeItem):
     dependencies=[Depends(is_workspace_owner)],
 )
 async def run_cancel(workspace_id: str, uid: str):
-    return WorkflowResult(workspace_id, uid).cancel()
+    try:
+        return WorkflowResult(workspace_id, uid).cancel()
+    except Exception as e:
+        logger.error(e, exc_info=True)
+        raise HTTPException(
+            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
+            detail="Failed to cencel workflow.",
+        )