Merge pull request #1337 from OCR-D/improve-network-client-errors

Improve ocrd-network-client error handling

Author: kba

src/ocrd_network/cli/client.py

@@ -1,3 +1,4 @@
+import sys
 import click
 from json import dumps
 from typing import List, Optional, Tuple
@@ -10,11 +11,13 @@
 from ocrd_utils.introspect import set_json_key_value_overrides
 from ocrd_utils.str import parse_json_string_or_file
 from ..client import Client
+from requests import RequestException
 
 
 ADDRESS_HELP = 'The URL of the Processing Server. If not provided, ' + \
     'the "OCRD_NETWORK_SERVER_ADDR_PROCESSING" environment variable is used by default'
 
+
 class URLType(click.types.StringParamType):
     name = "url"
     def convert(self, value, param, ctx):
@@ -53,7 +56,14 @@ def check_deployed_processors(address: Optional[str]):
     Each processor is shown only once regardless of the amount of deployed instances.
     """
     client = Client(server_addr_processing=address)
-    processors_list = client.check_deployed_processors()
+    try:
+        processors_list = client.check_deployed_processors()
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(dumps(processors_list, indent=4))
 
 
@@ -65,7 +75,14 @@ def check_processor_ocrd_tool(address: Optional[str], processor_name: str):
     Get the json tool of a deployed processor specified with `processor_name`
     """
     client = Client(server_addr_processing=address)
-    ocrd_tool = client.check_deployed_processor_ocrd_tool(processor_name=processor_name)
+    try:
+        ocrd_tool = client.check_deployed_processor_ocrd_tool(processor_name=processor_name)
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(dumps(ocrd_tool, indent=4))
 
 
@@ -85,7 +102,14 @@ def check_processing_job_log(address: Optional[str], processing_job_id: str):
     Check the log of a previously submitted processing job.
     """
     client = Client(server_addr_processing=address)
-    response = client.check_job_log(job_id=processing_job_id)
+    try:
+        response = client.check_job_log(job_id=processing_job_id)
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(response._content.decode(encoding='utf-8'))
 
 
@@ -97,8 +121,14 @@ def check_processing_job_status(address: Optional[str], processing_job_id: str):
     Check the status of a previously submitted processing job.
     """
     client = Client(server_addr_processing=address)
-    job_status = client.check_job_status(processing_job_id)
-    assert job_status
+    try:
+        job_status = client.check_job_status(processing_job_id)
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(f"Processing job status: {job_status}")
 
 
@@ -154,12 +184,26 @@ def send_processing_job_request(
     if callback_url:
         req_params["callback_url"] = callback_url
     client = Client(server_addr_processing=address)
-    processing_job_id = client.send_processing_job_request(
-        processor_name=processor_name, req_params=req_params)
-    assert processing_job_id
+    try:
+        processing_job_id = client.send_processing_job_request(
+            processor_name=processor_name, req_params=req_params)
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(f"Processing job id: {processing_job_id}")
+
     if block:
-        client.poll_job_status(job_id=processing_job_id, print_state=print_state)
+        try:
+            client.poll_job_status(job_id=processing_job_id, print_state=print_state)
+        except RequestException as e:
+            print(
+                getattr(e, 'detail_message', str(e)),
+                f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+            )
+            sys.exit(1)
 
 
 @client_cli.group('workflow')
@@ -178,8 +222,14 @@ def check_workflow_job_status(address: Optional[str], workflow_job_id: str):
     Check the status of a previously submitted workflow job.
     """
     client = Client(server_addr_processing=address)
-    job_status = client.check_workflow_status(workflow_job_id)
-    assert job_status
+    try:
+        job_status = client.check_workflow_status(workflow_job_id)
+    except RequestException as e:
+        print(
+            getattr(e, 'detail_message', str(e)),
+            f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+        )
+        sys.exit(1)
     print(f"Workflow job status: {job_status}")
 
 
@@ -209,7 +259,8 @@ def send_workflow_job_request(
     as in ``ocrd process`` tasks arguments), or via `-w` file path
     (same syntax, but newline separated).
     """
-    assert bool(path_to_workflow) != bool(len(tasks)), "requires either --path-to-workflow or task arguments"
+    if (path_to_workflow) != bool(len(tasks)):
+        raise ValueError("either -w/path-to-workflow or task argument(s) is required")
 
     client = Client(server_addr_processing=address)
     with NamedTemporaryFile() as workflow_file:
@@ -221,11 +272,17 @@ def send_workflow_job_request(
             path_to_mets=path_to_mets,
             page_wise=page_wise,
         )
-    assert workflow_job_id
     print(f"Workflow job id: {workflow_job_id}")
     if block:
         print(f"Polling state of workflow job {workflow_job_id}")
-        state = client.poll_workflow_status(job_id=workflow_job_id, print_state=print_state)
+        try:
+            state = client.poll_workflow_status(job_id=workflow_job_id, print_state=print_state)
+        except RequestException as e:
+            print(
+                getattr(e, 'detail_message', str(e)),
+                f"Requested URL: {getattr(getattr(e, 'response', ''), 'url', '')}"
+            )
+            sys.exit(1)
         if state != JobState.success:
             print(f"Workflow failed with {state}")
             exit(1)

src/ocrd_network/client_utils.py

@@ -1,6 +1,7 @@
 import json
 import os
-from requests import get as request_get, post as request_post
+from requests import get as request_get, post as request_post, RequestException, Response
+from requests.exceptions import JSONDecodeError
 from time import sleep
 from .constants import JobState, NETWORK_PROTOCOLS
 
@@ -24,6 +25,19 @@ def _poll_endpoint_status(ps_server_host: str, job_id: str, job_type: str, tries
     return job_state
 
 
+def _raise_if_error(response: Response) -> None:
+    """Check the requests-response and raise an exception if its status code indicates an error"""
+    try:
+        response.raise_for_status()
+    except RequestException as e:
+        try:
+            message = response.json()["detail"]
+        except JSONDecodeError:
+            message = response.text
+        e.detail_message = message
+        raise e
+
+
 def poll_job_status_till_timeout_fail_or_success(
     ps_server_host: str, job_id: str, tries: int, wait: int, print_state: bool = False) -> JobState:
     return _poll_endpoint_status(ps_server_host, job_id, "processor", tries, wait, print_state)
@@ -37,14 +51,14 @@ def poll_wf_status_till_timeout_fail_or_success(
 def get_ps_deployed_processors(ps_server_host: str):
     request_url = f"{ps_server_host}/processor"
     response = request_get(url=request_url, headers={"accept": "application/json; charset=utf-8"})
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     return response.json()
 
 
 def get_ps_deployed_processor_ocrd_tool(ps_server_host: str, processor_name: str):
     request_url = f"{ps_server_host}/processor/info/{processor_name}"
     response = request_get(url=request_url, headers={"accept": "application/json; charset=utf-8"})
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     return response.json()
 
 
@@ -57,18 +71,18 @@ def get_ps_processing_job_log(ps_server_host: str, processing_job_id: str):
 def get_ps_processing_job_status(ps_server_host: str, processing_job_id: str) -> JobState:
     request_url = f"{ps_server_host}/processor/job/{processing_job_id}"
     response = request_get(url=request_url, headers={"accept": "application/json; charset=utf-8"})
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     job_state = response.json()["state"]
-    assert job_state
+    assert job_state, "Propery 'state' is expected to always have a value"
     return getattr(JobState, job_state.lower())
 
 
 def get_ps_workflow_job_status(ps_server_host: str, workflow_job_id: str) -> JobState:
     request_url = f"{ps_server_host}/workflow/job-simple/{workflow_job_id}"
     response = request_get(url=request_url, headers={"accept": "application/json; charset=utf-8"})
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     job_state = response.json()["state"]
-    assert job_state
+    assert job_state, "Property 'state' is expected to always have a value"
     return getattr(JobState, job_state.lower())
 
 
@@ -79,9 +93,9 @@ def post_ps_processing_request(ps_server_host: str, processor: str, job_input: d
         headers={"accept": "application/json; charset=utf-8"},
         json=job_input
     )
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     processing_job_id = response.json()["job_id"]
-    assert processing_job_id
+    assert processing_job_id, "Property 'job_id' is expected to always have a value"
     return processing_job_id
 
 
@@ -102,9 +116,9 @@ def post_ps_workflow_request(
     json_resp_raw = response.text
     # print(f'post_ps_workflow_request >> {response.status_code}')
     # print(f'post_ps_workflow_request >> {json_resp_raw}')
-    assert response.status_code == 200, f"Processing server: {request_url}, {response.status_code}"
+    _raise_if_error(response)
     wf_job_id = json.loads(json_resp_raw)["job_id"]
-    assert wf_job_id
+    assert wf_job_id, "Property 'job_id' is expected to always have a value"
     return wf_job_id