Improve error handling and propagation

kbatch-proxy/kbatch_proxy/main.py

@@ -1,3 +1,4 @@
+import json
 import os
 import logging
 from typing import List, Optional, Tuple, Dict, Union
@@ -168,6 +169,22 @@ def get_k8s_api() -> Tuple[kubernetes.client.CoreV1Api, kubernetes.client.BatchV
 # app
 
 
+@app.exception_handler(kubernetes.client.ApiException)
+async def kubernetes_exception_handler(
+    request: Request, exc: kubernetes.client.ApiException
+):
+    """Relay kubernetes errors to users"""
+    try:
+        detail = json.loads(exc.body)["message"]
+    except (ValueError, KeyError):
+        detail = exc.body
+
+    raise HTTPException(
+        status_code=exc.status,
+        detail=detail,
+    )
+
+
 # cronjobs #
 @router.get("/cronjobs/{job_name}")
 async def read_cronjob(job_name: str, user: User = Depends(get_current_user)):
@@ -390,22 +407,11 @@ def _create_job(
         patch.add_submitted_configmap_name(job_to_patch, config_map)
 
     logger.info("Submitting job")
-    try:
-        if issubclass(model, V1Job):
-            resp = batch_api.create_namespaced_job(namespace=user.namespace, body=job)
-        elif issubclass(model, V1CronJob):
-            job.spec.job_template = job_to_patch
-            resp = batch_api.create_namespaced_cron_job(
-                namespace=user.namespace, body=job
-            )
-
-    except kubernetes.client.exceptions.ApiException as e:
-        content_type = e.headers.get("Content-Type")
-        if content_type:
-            headers = {"Content-Type": content_type}
-        else:
-            headers = {}
-        raise HTTPException(status_code=e.status, detail=e.body, headers=headers)
+    if issubclass(model, V1Job):
+        resp = batch_api.create_namespaced_job(namespace=user.namespace, body=job)
+    elif issubclass(model, V1CronJob):
+        job.spec.job_template = job_to_patch
+        resp = batch_api.create_namespaced_cron_job(namespace=user.namespace, body=job)
 
     if config_map:
         logger.info(

kbatch-proxy/tests/test_app.py

@@ -1,3 +1,4 @@
+import json
 import os
 import sys
 import pytest
@@ -38,8 +39,7 @@ def test_read_main():
     assert response.json() == {"message": "kbatch"}
 
 
-@pytest.mark.usefixtures("mock_hub_auth")
-def test_authorized():
+def test_authorized(mock_hub_auth):
     response = client.get("/authorized")
     assert response.status_code == 401
 
@@ -61,3 +61,11 @@ def test_loads_profile():
     subprocess.check_output(
         f"KBATCH_PROFILE_FILE={profile} {sys.executable} -c '{code}'", shell=True
     )
+
+
+def test_error_handling(mock_hub_auth):
+    response = client.get("/jobs/nosuchjob", headers={"Authorization": "token abc"})
+    err = json.loads(response.read().decode("utf8"))
+    assert response.status_code == 404
+    assert "detail" in err
+    assert "nosuchjob" in err["detail"]

kbatch/kbatch/__main__.py

@@ -0,0 +1,4 @@
+if __name__ == "__main__":
+    from kbatch.cli import main
+
+    main()

kbatch/kbatch/_core.py

@@ -131,11 +131,7 @@ def _request_action(
         headers=headers,
         json=json_data,
     )
-    try:
-        r.raise_for_status()
-    except Exception:
-        logger.exception(r.text)
-        raise
+    r.raise_for_status()
 
     return r.json()
 

kbatch/kbatch/cli.py

@@ -1,6 +1,9 @@
+import sys
 import logging
+from contextlib import contextmanager
 
 import click
+import httpx
 import rich
 import rich.logging
 from kubernetes.client.models import V1Job, V1CronJob
@@ -16,8 +19,23 @@
     datefmt="[%X]",
     handlers=[rich.logging.RichHandler()],
 )
-# don't log every http request
-logging.getLogger("httpx").setLevel(logging.WARNING)
+# don't log http requests or responses
+logging.getLogger("httpx").propagate = False
+
+
+@contextmanager
+def _render_http_error():
+    """Render an HTTP Error from kbatch nicely"""
+    try:
+        yield
+    except httpx.HTTPStatusError as e:
+        response = e.response
+        try:
+            response_json: dict = response.json()
+            msg = response_json["detail"]
+        except (ValueError, KeyError):
+            msg = response.text
+        sys.exit(f"kbatch-proxy error {response.status_code}: {msg}")
 
 
 @click.group()
@@ -354,3 +372,11 @@ def logs(pod_name, kbatch_url, token, stream, pretty, read_timeout):
             print(line)
     else:
         print(result)
+
+
+def main():
+    """Launch kbatch CLI"""
+    # catch and format HTTP errors
+    # not sure if there's a better way to inject this into cli() itself?
+    with _render_http_error():
+        cli()

kbatch/setup.cfg

@@ -40,7 +40,7 @@ docs =
 
 [options.entry_points]
 console_scripts =
-    kbatch = kbatch.cli:cli
+    kbatch = kbatch.cli:main
 
 [flake8]
 exclude =
@@ -66,4 +66,4 @@ ignore =
 
 # black is set to 88, but isn't a strict limit so we add some wiggle room for
 # flake8 testing.
-max-line-length = 100
+max-line-length = 100