Report unavailable job logs cleanly

Author: krowvin

cwms_batch_events/api/routers/jobs.py

@@ -79,5 +79,15 @@ def get_job_by_id(
 def get_logs_for_job(
     job_id: UUID, job_logger: JobLogger = Depends(get_job_logger)
 ) -> JobLogs:
-    logs = job_logger.get_logs_for_job(job_id)
+    try:
+        logs = job_logger.get_logs_for_job(job_id)
+    except FileNotFoundError as exc:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND, detail=str(exc)
+        ) from exc
+    except ValueError as exc:
+        raise HTTPException(
+            status_code=status.HTTP_409_CONFLICT,
+            detail=f"Logs are not available for job '{job_id}': {exc}",
+        ) from exc
     return JobLogs(logs=logs)

cwms_batch_events/core/job_logger/cloudwatch.py

@@ -32,7 +32,19 @@ def get_batch_log_name(self, external_job_id: str) -> str:
                 f"Multiple jobs found for external_job_id {external_job_id}"
             )
 
-        return jobs[0]["attempts"][-1]["container"]["logStreamName"]
+        attempts = jobs[0].get("attempts", [])
+        if not attempts:
+            raise ValueError(
+                f"No Batch job attempts found for external_job_id {external_job_id}"
+            )
+
+        log_stream_name = attempts[-1].get("container", {}).get("logStreamName")
+        if not log_stream_name:
+            raise ValueError(
+                f"No log stream found for external_job_id {external_job_id}"
+            )
+
+        return log_stream_name
 
     def get_logs_for_job(self, job_id: UUID) -> str:
         job = self.get_job_details(job_id)

cwms_batch_events/core/job_logger/s3.py

@@ -1,4 +1,5 @@
 import boto3
+from botocore.exceptions import ClientError
 from uuid import UUID
 
 from cwms_batch_events.core.settings import settings
@@ -12,11 +13,17 @@ def __init__(self):
         self.s3 = boto3.client(
             "s3",
             endpoint_url=S3_ENDPOINT_URL,
+            region_name=settings.aws_default_region,
         )
 
     def get_logs_for_job(self, job_id: UUID) -> str:
         key = f"logs/{job_id}.log"
-        response = self.s3.get_object(Bucket=S3_BUCKET, Key=key)
+        try:
+            response = self.s3.get_object(Bucket=S3_BUCKET, Key=key)
+        except ClientError as exc:
+            if exc.response.get("Error", {}).get("Code") in {"NoSuchKey", "404"}:
+                raise FileNotFoundError(f"No logs found for job {job_id}") from exc
+            raise
         body: str = response["Body"].read().decode("utf-8")
         return body
 

tests/cwms_batch_events/api/test_jobs.py

@@ -88,3 +88,30 @@ def test_get_logs_for_job_returns_logs(client, job_logger):
     assert response.status_code == 200
     assert response.json() == {"logs": "hello"}
     job_logger.get_logs_for_job.assert_called_once()
+
+
+def test_get_logs_for_job_returns_404_when_logs_missing(client, job_logger):
+    job_id = str(uuid4())
+    job_logger.get_logs_for_job.side_effect = FileNotFoundError(
+        f"No logs found for job {job_id}"
+    )
+
+    response = client.get(f"/jobs/{job_id}/logs")
+
+    assert response.status_code == 404
+    assert response.json() == {"detail": f"No logs found for job {job_id}"}
+
+
+def test_get_logs_for_job_returns_409_when_logs_not_ready(client, job_logger):
+    job_id = str(uuid4())
+    job_logger.get_logs_for_job.side_effect = ValueError("No Batch job attempts found")
+
+    response = client.get(f"/jobs/{job_id}/logs")
+
+    assert response.status_code == 409
+    assert response.json() == {
+        "detail": (
+            f"Logs are not available for job '{job_id}': "
+            "No Batch job attempts found"
+        )
+    }

tests/cwms_batch_events/core/test_job_loggers.py

@@ -3,6 +3,7 @@
 from uuid import uuid4
 
 import pytest
+from botocore.exceptions import ClientError
 
 from cwms_batch_events.core.job_logger.cloudwatch import CloudWatchJobLogger
 from cwms_batch_events.core.job_logger.s3 import S3JobLogger
@@ -24,6 +25,24 @@ def test_s3_job_logger_reads_logs_from_bucket():
     assert logs == "hello"
 
 
+def test_s3_job_logger_reports_missing_logs():
+    job_id = uuid4()
+    s3_client = mock.Mock()
+    s3_client.get_object.side_effect = ClientError(
+        {"Error": {"Code": "NoSuchKey", "Message": "not found"}},
+        "GetObject",
+    )
+
+    with mock.patch(
+        "cwms_batch_events.core.job_logger.s3.boto3.client",
+        return_value=s3_client,
+    ), mock.patch("cwms_batch_events.core.job_logger.s3.S3_BUCKET", "bucket"):
+        logger = S3JobLogger()
+
+    with pytest.raises(FileNotFoundError, match=f"No logs found for job {job_id}"):
+        logger.get_logs_for_job(job_id)
+
+
 def test_s3_job_logger_pushes_logs_to_bucket():
     s3_client = mock.Mock()
     job_id = uuid4()
@@ -84,6 +103,36 @@ def test_cloudwatch_job_logger_rejects_multiple_batch_jobs():
         logger.get_batch_log_name("ext-123")
 
 
+def test_cloudwatch_job_logger_reports_missing_batch_attempts():
+    batch_client = mock.Mock()
+    batch_client.describe_jobs.return_value = {"jobs": [{"attempts": []}]}
+
+    with mock.patch(
+        "cwms_batch_events.core.job_logger.cloudwatch.boto3.client",
+        side_effect=[batch_client, mock.Mock()],
+    ):
+        logger = CloudWatchJobLogger(mock.Mock())
+
+    with pytest.raises(ValueError, match="No Batch job attempts found"):
+        logger.get_batch_log_name("ext-123")
+
+
+def test_cloudwatch_job_logger_reports_missing_log_stream():
+    batch_client = mock.Mock()
+    batch_client.describe_jobs.return_value = {
+        "jobs": [{"attempts": [{"container": {}}]}]
+    }
+
+    with mock.patch(
+        "cwms_batch_events.core.job_logger.cloudwatch.boto3.client",
+        side_effect=[batch_client, mock.Mock()],
+    ):
+        logger = CloudWatchJobLogger(mock.Mock())
+
+    with pytest.raises(ValueError, match="No log stream found"):
+        logger.get_batch_log_name("ext-123")
+
+
 def test_cloudwatch_job_logger_requires_external_job_id():
     job_id = uuid4()
     db = mock.Mock()

ui/src/features/jobs-list/JobLogs.tsx

@@ -7,12 +7,12 @@ interface JobLogsProps {
 }
 
 const JobLogs = ({ jobId, disabled = false }: JobLogsProps) => {
-  const { data, isLoading, isError } = useJobLogs(jobId, !disabled);
+  const { data, error, isLoading, isError } = useJobLogs(jobId, !disabled);
 
   let message: string;
   if (disabled) message = "Logs unavailable until job is finished";
   else if (isLoading) message = "Loading logs...";
-  else if (isError) message = "Error loading logs!";
+  else if (isError) message = error?.message ?? "Error loading logs!";
   else if (!data) message = "No logs found!";
   else message = data.logs;
 

ui/src/features/jobs-list/useJobLogs.ts

@@ -18,7 +18,12 @@ const useJobLogs = (jobId: string, enabled: boolean) => {
 const fetchJobs = async (jobId: string, token?: string): Promise<JobLogs> => {
   const response = await fetchWithAuth(`/api/jobs/${jobId}/logs`, {}, token);
   if (!response.ok) {
-    throw new Error(`Failed to fetch logs for job ${jobId}`);
+    const payload = await response.json().catch(() => undefined);
+    const detail =
+      payload && typeof payload.detail === "string"
+        ? payload.detail
+        : `Failed to fetch logs for job ${jobId}`;
+    throw new Error(detail);
   }
   return response.json();
 };