fix error status code

Author: CamDavidsonPilon

CHANGELOG.md

@@ -2,6 +2,9 @@
 
  - removed System tab from the Pioreactor's "Control" dialog. You can see (most) of this data on the Inventory page
  - added a Self-test tab back to the Pioreactor's "Control" dialog, and to the "Control all Pioreactors"
+
+
+ ## Agents
  - improved unit API retry safety by returning in-progress responses when Huey task locks are held
  - improved API error messages with structured causes and remediation hints for agents and UI clients
 

core/pioreactor/web/api.py

@@ -1455,7 +1455,12 @@ def get_all_calibrations_as_yamls(pioreactor_unit: str) -> ResponseReturnValue:
     try:
         results = task.get(blocking=True, timeout=60)
     except (HueyException, TaskException):
-        return {"result": False, "filename": None, "msg": "Timed out"}, 500
+        abort_with(
+            500,
+            "Timed out fetching calibrations",
+            cause="Timed out waiting for workers to provide calibration archives.",
+            remediation="Retry the request and check worker connectivity.",
+        )
 
     aggregate_buffer = BytesIO()
 
@@ -1507,7 +1512,12 @@ def get_entire_dot_pioreactor(pioreactor_unit: str) -> ResponseReturnValue:
     try:
         results = task.get(blocking=True, timeout=120)
     except (HueyException, TaskException):
-        return {"result": False, "filename": None, "msg": "Timed out"}, 500
+        abort_with(
+            500,
+            "Timed out fetching .pioreactor archive",
+            cause="Timed out waiting for worker responses.",
+            remediation="Retry the request and check worker connectivity.",
+        )
 
     # If only one worker, proxy its ZIP directly
     if isinstance(results, dict) and len(results) == 1:
@@ -2223,12 +2233,21 @@ def export_datasets() -> ResponseReturnValue:
     try:
         status, msg = result(blocking=True, timeout=5 * 60)
     except (HueyException, TaskException):
-        status = False
-        return {"result": status, "filename": None, "msg": "Task error, or time out"}, 500
+        abort_with(
+            500,
+            "Export task failed or timed out",
+            cause="Task error or timeout while exporting datasets.",
+            remediation="Retry the export and check server logs if it persists.",
+        )
 
     if not status:
         publish_to_error_log(msg, "export_datasets")
-        return {"result": status, "filename": None, "msg": msg}, 500
+        abort_with(
+            500,
+            "Export task failed",
+            cause=msg,
+            remediation="Check server logs for details and retry the export.",
+        )
 
     return {"result": status, "filename": filename, "msg": "Finished"}, 200
 
@@ -2323,7 +2342,12 @@ def create_experiment() -> ResponseReturnValue:
         return {"status": "success"}, 201
 
     except sqlite3.IntegrityError:
-        return {"status": "error"}, 409
+        abort_with(
+            409,
+            "Experiment already exists",
+            cause="Experiment name conflicts with an existing experiment.",
+            remediation="Choose a different experiment name and retry.",
+        )
     except Exception as e:
         publish_to_error_log(str(e), "create_experiment")
         abort_with(500, str(e))
@@ -3287,7 +3311,7 @@ def get_experiment_assignment_for_worker(pioreactor_unit: str) -> ResponseReturn
     if result is None:
         abort_with(
             404,
-            f"Worker {pioreactor_unit} does not exist in the cluster.",
+            f"Worker {pioreactor_unit} not found.",
             cause=f"Worker '{pioreactor_unit}' not in leader database.",
             remediation="Check the unit name or add the worker to the inventory.",
         )

core/pioreactor/web/app.py

@@ -23,6 +23,7 @@
 from pioreactor.whoami import am_I_leader
 from pioreactor.whoami import get_unit_name
 from pioreactor.whoami import UNIVERSAL_EXPERIMENT
+from werkzeug.exceptions import HTTPException
 
 VERSION = __version__
 HOSTNAME = get_unit_name()
@@ -128,6 +129,10 @@ def handle_bad_gateway(e):
             502,
         )
 
+    @app.errorhandler(HTTPException)
+    def handle_http_exception(e: HTTPException):
+        return jsonify({"error": e.description}), e.code or 500
+
     @app.after_request
     def ensure_error_payload(response: t.Any) -> t.Any:
         if response.status_code < 400:

core/pioreactor/web/unit_api.py

@@ -386,7 +386,12 @@ def get_clock_time():
         current_time = current_utc_timestamp()
         return jsonify({"status": "success", "clock_time": current_time}), 200
     except Exception as e:
-        return jsonify({"status": "error", "message": str(e)}), 500
+        abort_with(
+            500,
+            "Failed to read clock time",
+            cause=str(e),
+            remediation="Check system clock availability and server logs, then retry.",
+        )
 
 
 # PATCH / POST to set clock time

core/pioreactor/web/utils.py

@@ -40,7 +40,9 @@ def abort_with(
     if merged_error_info:
         payload["error_info"] = merged_error_info
 
-    abort(jsonify(payload), status)
+    response = jsonify(payload)
+    response.status_code = status
+    abort(response)
     raise AssertionError("abort should not return")
 
 

frontend/src/Pioreactor.jsx

@@ -733,7 +733,6 @@ function SettingsActionsDialog(props) {
   const {client, subscribeToTopic, unsubscribeFromTopic} = useMQTT();
   const selfTestExperiment = "$experiment";
   const selfTestSettings = props.jobs?.self_test?.publishedSettings || null;
-  const selfTestDefinitionAvailable = Boolean(selfTestSettings);
   const selfTestSettingTypes = useMemo(() => {
     if (!selfTestSettings) {
       return {};
@@ -1533,7 +1532,7 @@ function SettingsActionsDialog(props) {
               loading={isSelfTestRunning || selfTestStartPending}
               loadingPosition="start"
               endIcon={<PlayArrowIcon />}
-              disabled={isSelfTestRunning || selfTestStartPending || !selfTestDefinitionAvailable}
+              disabled={isSelfTestRunning || selfTestStartPending }
               onClick={handleRunSelfTest}
               sx={{textTransform: "none"}}
             >
@@ -1543,54 +1542,47 @@ function SettingsActionsDialog(props) {
 
           <ControlDivider/>
 
-          {!selfTestDefinitionAvailable && (
-            <Alert severity="warning">
-              Self-test is unavailable on this cluster.
-            </Alert>
-          )}
 
-          {selfTestDefinitionAvailable && (
-            <Accordion disableGutters sx={{boxShadow: "none", "&:before": {display: "none"}}}>
-              <AccordionSummary expandIcon={<ExpandMoreIcon />}>
-                <Box sx={{display: "flex", alignItems: "center", gap: 1}}>
-                  {renderSelfTestSummaryIcon()}
-                  <Typography>{props.label ? `${props.label} / ${props.unit}` : props.unit}</Typography>
-                </Box>
-              </AccordionSummary>
-              <AccordionDetails>
-                {availableSelfTestGroups.length === 0 ? (
-                  <Typography variant="body2">No self-test checks available.</Typography>
-                ) : (
-                  <>
-                    {availableSelfTestGroups.map((group) => (
-                      <List
-                        key={`self-test-${props.unit}-${group.title}`}
-                        dense
-                        disablePadding
-                        subheader={
-                          <ListSubheader style={{lineHeight: "20px"}} component="div" disableSticky={true} disableGutters={true}>
-                            {group.title}
-                          </ListSubheader>
-                        }
-                      >
-                        {group.tests.map((test) => (
-                          <ListItem key={`self-test-${props.unit}-${test.key}`} sx={{pt: 0, pb: 0}}>
-                            <ListItemIcon sx={{minWidth: "30px"}}>
-                              {renderSelfTestIcon(test.key)}
-                            </ListItemIcon>
-                            <ListItemText
-                              primary={test.label}
-                              secondary={renderSelfTestSecondary(test)}
-                            />
-                          </ListItem>
-                        ))}
-                      </List>
-                    ))}
-                  </>
-                )}
-              </AccordionDetails>
-            </Accordion>
-          )}
+          <Accordion disableGutters sx={{boxShadow: "none", "&:before": {display: "none"}}}>
+            <AccordionSummary expandIcon={<ExpandMoreIcon />}>
+              <Box sx={{display: "flex", alignItems: "center", gap: 1}}>
+                {renderSelfTestSummaryIcon()}
+                <Typography>{props.label ? `${props.label} / ${props.unit}` : props.unit}</Typography>
+              </Box>
+            </AccordionSummary>
+            <AccordionDetails>
+              {availableSelfTestGroups.length === 0 ? (
+                <Typography variant="body2">No self-test checks available.</Typography>
+              ) : (
+                <>
+                  {availableSelfTestGroups.map((group) => (
+                    <List
+                      key={`self-test-${props.unit}-${group.title}`}
+                      dense
+                      disablePadding
+                      subheader={
+                        <ListSubheader style={{lineHeight: "20px"}} component="div" disableSticky={true} disableGutters={true}>
+                          {group.title}
+                        </ListSubheader>
+                      }
+                    >
+                      {group.tests.map((test) => (
+                        <ListItem key={`self-test-${props.unit}-${test.key}`} sx={{pt: 0, pb: 0}}>
+                          <ListItemIcon sx={{minWidth: "30px"}}>
+                            {renderSelfTestIcon(test.key)}
+                          </ListItemIcon>
+                          <ListItemText
+                            primary={test.label}
+                            secondary={renderSelfTestSecondary(test)}
+                          />
+                        </ListItem>
+                      ))}
+                    </List>
+                  ))}
+                </>
+              )}
+            </AccordionDetails>
+          </Accordion>
         </TabPanel>
 
       </DialogContent>

frontend/src/SingleCalibrationPage.jsx

@@ -4,6 +4,7 @@ import { useConfirm } from 'material-ui-confirm';
 import { CircularProgress, Button, Typography, Box, Divider } from "@mui/material";
 import Dialog from '@mui/material/Dialog';
 import DialogTitle from '@mui/material/DialogTitle';
+import Alert from '@mui/material/Alert';
 import DialogContent from '@mui/material/DialogContent';
 import DialogActions from '@mui/material/DialogActions';
 import IconButton from '@mui/material/IconButton';
@@ -258,7 +259,12 @@ function SingleCalibrationPage(props) {
     const apiUrl = `/api/workers/${pioreactorUnit}/calibrations/${device}/${calibrationName}`;
     try {
       const data = await fetchTaskResult(apiUrl)
-      setCalibration(data.result[pioreactorUnit]);
+      if (data.result[pioreactorUnit].error){
+        setCalibration(null);
+      } else{
+        setCalibration(data.result[pioreactorUnit]);
+      }
+
     } catch (err) {
       console.error("Failed to fetch calibration:", err);
     } finally {
@@ -377,9 +383,9 @@ function SingleCalibrationPageCard({ pioreactorUnit, device, calibrationName, ca
   if (!calibration) {
     return (
       <Box sx={{textAlign: "center", mb: '50px', mt: "50px"}}>
-        <Typography  variant="body2" component="p" color="textSecondary">
+        <Alert severity="error" sx={{ display: "inline-flex", textAlign: "left" }}>
            Unable to find calibration data.
-        </Typography>
+        </Alert>
       </Box>
     );
   }

frontend/src/SingleEstimatorPage.jsx

@@ -5,6 +5,7 @@ import { CircularProgress, Button, Typography, Box, Divider } from "@mui/materia
 import Dialog from '@mui/material/Dialog';
 import DialogTitle from '@mui/material/DialogTitle';
 import DialogContent from '@mui/material/DialogContent';
+import Alert from '@mui/material/Alert';
 import DialogActions from '@mui/material/DialogActions';
 import IconButton from '@mui/material/IconButton';
 import { fetchTaskResult } from "./utilities";
@@ -208,8 +209,12 @@ function SingleEstimatorPage(props) {
     setLoading(true);
     const apiUrl = `/api/workers/${pioreactorUnit}/estimators/${device}/${estimatorName}`;
     try {
-      const data = await fetchTaskResult(apiUrl);
-      setEstimator(data.result[pioreactorUnit]);
+      const data = await fetchTaskResult(apiUrl)
+      if (data.result[pioreactorUnit].error){
+        setEstimator(null);
+      } else{
+        setEstimator(data.result[pioreactorUnit]);
+      }
     } catch (err) {
       console.error("Failed to fetch estimator:", err);
     } finally {
@@ -326,9 +331,9 @@ function SingleEstimatorPageCard({ pioreactorUnit, device, estimatorName, estima
   if (!estimator) {
     return (
       <Box sx={{ textAlign: "center", mb: '50px', mt: "50px" }}>
-        <Typography variant="body2" component="p" color="textSecondary">
+        <Alert severity="error" sx={{ display: "inline-flex", textAlign: "left" }}>
           Unable to find estimator data.
-        </Typography>
+        </Alert>
       </Box>
     );
   }

frontend/src/utilities.js

@@ -131,10 +131,27 @@ export async function checkTaskCallback(callbackURL, {maxRetries = 100, delayMs
 export async function fetchTaskResult(endpoint, {fetchOptions = {}, maxRetries = 100, delayMs = 200} = {}) {
   const response = await fetch(endpoint, fetchOptions);
   if (!response.ok) {
-    throw new Error(`HTTP error! Status: ${response.status}`);
+    let message = `HTTP error! Status: ${response.status}`;
+    try {
+      const payload = await response.json();
+      if (payload?.error_info?.message) {
+        message = payload.error_info.message;
+      } else if (payload?.error) {
+        message = payload.error;
+      }
+    } catch (error) {
+      // ignore JSON parse errors and fall back to default message
+    }
+    throw new Error(message);
   }
   const payload = await response.json();
   if (!payload.result_url_path) {
+    if (payload?.error_info?.message) {
+      throw new Error(payload.error_info.message);
+    }
+    if (payload?.error) {
+      throw new Error(payload.error);
+    }
     throw new Error('No result_url_path in response');
   }
   return checkTaskCallback(payload.result_url_path, {maxRetries, delayMs});