Fail with the exit code 1 when the server answers an error

doc/changelog.rst

@@ -16,6 +16,9 @@ Added
 
 Changed
 ^^^^^^^
+- The commands fail with the exit code 1 when the server answers an error, or a bulk response
+  with failed operations. The response is still displayed on stdout, and a summary of the error
+  is displayed on stderr. :issue:`2`
 - The errors of the server discovery give the URL of the server.
 
 Fixed

doc/tutorial.rst

@@ -256,6 +256,29 @@ See the :doc:`scim2-tester documentation <scim2_tester:index>` for more details
     SUCCESS check_access_invalid_resource_typ
     ...
 
+Errors
+------
+
+When the server answers an error, the command displays it on stdout and fails with the exit code 1.
+A summary of the error is displayed on stderr, so the output can still be read by another program.
+
+.. code-block:: console
+   :caption: Querying an user that does not exist.
+
+   $ scim query user unknown-id
+   {
+       "schemas": [
+           "urn:ietf:params:scim:api:messages:2.0:Error"
+       ],
+       "status": "404",
+       "detail": "Resource unknown-id not found"
+   }
+   Error: 404 Resource unknown-id not found
+   $ echo $?
+   1
+
+The :ref:`reference:bulk` command fails the same way when some operations of the bulk response failed.
+
 JSON input
 ----------
 

scim2_cli/bulk.py

@@ -3,8 +3,8 @@
 
 from scim2_cli.utils import DOC_URL
 from scim2_cli.utils import SCIM_EXCEPTIONS
+from scim2_cli.utils import echo_response
 from scim2_cli.utils import exception_to_click_error
-from scim2_cli.utils import formatted_payload
 from scim2_cli.utils import indent_option
 
 
@@ -33,4 +33,4 @@ def bulk_cli(ctx: click.Context, indent: bool) -> None:
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    click.echo(formatted_payload(response.model_dump(), indent))
+    echo_response(response, indent)

scim2_cli/create.py

@@ -15,9 +15,9 @@
 from scim2_cli.utils import SCIM_EXCEPTIONS
 from scim2_cli.utils import ModelCommand
 from scim2_cli.utils import command_name
+from scim2_cli.utils import echo_response
 from scim2_cli.utils import escape_options_help
 from scim2_cli.utils import exception_to_click_error
-from scim2_cli.utils import formatted_payload
 from scim2_cli.utils import indent_option
 from scim2_cli.utils import inherited
 from scim2_cli.utils import me_option
@@ -44,7 +44,7 @@ def create_payload(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    click.echo(formatted_payload(response.model_dump(), indent))
+    echo_response(response, indent)
 
 
 def create_factory(

scim2_cli/delete.py

@@ -1,8 +1,6 @@
 import click
 from click import ClickException
 from scim2_client import Me
-from scim2_models import Message
-from scim2_models import Resource
 from sphinx_click.rst_to_ansi_formatter import make_rst_to_ansi_formatter
 
 from scim2_cli.utils import exception_to_click_error
@@ -11,7 +9,7 @@
 
 from .utils import DOC_URL
 from .utils import SCIM_EXCEPTIONS
-from .utils import formatted_payload
+from .utils import echo_response
 
 
 @click.command(cls=make_rst_to_ansi_formatter(DOC_URL), name="delete")
@@ -58,11 +56,4 @@ def delete_cli(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    if response:
-        payload = (
-            response.model_dump()
-            if isinstance(response, Resource | Message)
-            else response
-        )
-        payload = formatted_payload(payload, indent)
-        click.echo(payload)
+    echo_response(response, indent)

scim2_cli/modify.py

@@ -18,10 +18,10 @@
 
 from scim2_cli.utils import DOC_URL
 from scim2_cli.utils import SCIM_EXCEPTIONS
+from scim2_cli.utils import echo_response
 from scim2_cli.utils import escape_control_characters
 from scim2_cli.utils import exception_to_click_error
 from scim2_cli.utils import find_target
-from scim2_cli.utils import formatted_payload
 from scim2_cli.utils import indent_option
 from scim2_cli.utils import me_option
 
@@ -206,5 +206,4 @@ def modify_cli(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    if response is not None:
-        click.echo(formatted_payload(response.model_dump(), indent))
+    echo_response(response, indent)

scim2_cli/query.py

@@ -17,7 +17,7 @@
 
 from .utils import DOC_URL
 from .utils import SCIM_EXCEPTIONS
-from .utils import formatted_payload
+from .utils import echo_response
 
 DISCOVERY_MODELS = (Schema, ResourceType, ServiceProviderConfig)
 
@@ -169,5 +169,4 @@ def query_cli(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    payload = formatted_payload(response.model_dump(), indent)
-    click.echo(payload)
+    echo_response(response, indent)

scim2_cli/replace.py

@@ -21,7 +21,7 @@
 from .utils import DOC_URL
 from .utils import SCIM_EXCEPTIONS
 from .utils import ModelCommand
-from .utils import formatted_payload
+from .utils import echo_response
 from .utils import renamed_fields
 from .utils import unacceptable_fields
 
@@ -45,7 +45,7 @@ def replace_payload(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    click.echo(formatted_payload(response.model_dump(), indent))
+    echo_response(response, indent)
 
 
 def replace_factory(

scim2_cli/search.py

@@ -7,7 +7,7 @@
 
 from .utils import DOC_URL
 from .utils import SCIM_EXCEPTIONS
-from .utils import formatted_payload
+from .utils import echo_response
 
 
 @click.command(cls=make_rst_to_ansi_formatter(DOC_URL), name="search")
@@ -113,5 +113,4 @@ def search_cli(
     except SCIM_EXCEPTIONS as scim_exc:
         raise exception_to_click_error(scim_exc) from scim_exc
 
-    payload = formatted_payload(response.model_dump(), indent)
-    click.echo(payload)
+    echo_response(response, indent)

scim2_cli/utils.py

@@ -12,7 +12,9 @@
 from click.core import ParameterSource
 from scim2_client import SCIMClientException
 from scim2_models import BaseModel
+from scim2_models import BulkResponse
 from scim2_models import Context
+from scim2_models import Error
 from scim2_models import Mutability
 from scim2_models import Resource
 from scim2_models import ResourceType
@@ -129,6 +131,30 @@ def formatted_payload(obj: Any, indent: bool) -> str:
     return json.dumps(obj, indent=INDENTATION_SIZE if indent else None)
 
 
+def echo_response(response: Any, indent: bool) -> None:
+    """Display the response of the server, and exit with an error code if it reports a failure."""
+    if response is None:
+        return
+
+    click.echo(formatted_payload(response.model_dump(), indent))
+    if isinstance(response, Error):
+        summary = " ".join(
+            str(part) for part in (response.status, response.detail) if part
+        )
+        click.echo(f"Error: {escape_control_characters(summary)}", err=True)
+        click.get_current_context().exit(1)
+
+    if isinstance(response, BulkResponse):
+        operations = response.operations or []
+        failed = [op for op in operations if op.status and op.status >= 400]
+        if failed:
+            click.echo(
+                f"Error: {len(failed)} of {len(operations)} bulk operations failed",
+                err=True,
+            )
+            click.get_current_context().exit(1)
+
+
 def split_headers(headers: list[str]) -> dict[str, str]:
     """Make a dict from header strings.
 

tests/test_bulk.py

@@ -109,7 +109,7 @@ def test_too_many_operations(invoke):
 
 
 def test_error_response(invoke, httpserver):
-    """The error the server answers is displayed."""
+    """The error the server answers is displayed, and the command fails."""
     httpserver.expect_oneshot_request("/Bulk", method="POST").respond_with_json(
         {
             "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
@@ -122,4 +122,36 @@ def test_error_response(invoke, httpserver):
 
     result = invoke("bulk", input=json.dumps(BULK_REQUEST))
 
-    assert json.loads(result.output)["status"] == "413"
+    assert result.exit_code == 1, result.output
+    assert json.loads(result.stdout)["status"] == "413"
+    assert result.stderr == (
+        "Error: 413 The size of the bulk operation exceeds the maxPayloadSize\n"
+    )
+
+
+def test_failed_operations(invoke, httpserver):
+    """A bulk response with failed operations is displayed, and the command fails."""
+    operations = [
+        BULK_RESPONSE["Operations"][0],
+        {
+            "method": "POST",
+            "bulkId": "ytrewq",
+            "status": "409",
+            "response": {
+                "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+                "scimType": "uniqueness",
+                "status": "409",
+                "detail": "userName is already taken",
+            },
+        },
+    ]
+    httpserver.expect_oneshot_request("/Bulk", method="POST").respond_with_json(
+        {**BULK_RESPONSE, "Operations": operations},
+        content_type="application/scim+json",
+    )
+
+    result = invoke("bulk", input=json.dumps(BULK_REQUEST))
+
+    assert result.exit_code == 1, result.output
+    assert len(json.loads(result.stdout)["Operations"]) == 2
+    assert result.stderr == "Error: 1 of 2 bulk operations failed\n"

tests/test_delete.py

@@ -72,7 +72,7 @@ def test_bad_resource_type(runner, httpserver):
 
 
 def test_not_found(runner, httpserver):
-    """Test pydantic errors handling."""
+    """An error answered by the server is displayed, and the command fails."""
     httpserver.expect_request(
         "/Users/unknown-id",
         method="DELETE",
@@ -97,8 +97,12 @@ def test_not_found(runner, httpserver):
         ],
         catch_exceptions=False,
     )
-    assert result.exit_code == 0, result.output
-    json_output = json.loads(result.output)
+    assert result.exit_code == 1, result.output
+    assert (
+        result.stderr
+        == "Error: 404 Resource 2819c223-7f76-453a-919d-413861904646 not found\n"
+    )
+    json_output = json.loads(result.stdout)
     assert json_output == {
         "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
         "detail": "Resource 2819c223-7f76-453a-919d-413861904646 not found",

tests/test_errors.py

@@ -0,0 +1,73 @@
+import json
+
+import pytest
+
+from scim2_cli import cli
+
+ERROR = {
+    "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+    "status": "404",
+    "detail": "Resource 1 not found",
+}
+
+
+@pytest.mark.parametrize(
+    ("arguments", "method", "path"),
+    [
+        (["query", "user", "1"], "GET", "/Users/1"),
+        (["query", "user"], "GET", "/Users"),
+        (["search", "user"], "POST", "/Users/.search"),
+        (["create", "user", "--user-name", "bjensen"], "POST", "/Users"),
+        (
+            ["replace", "user", "--id", "1", "--user-name", "bjensen"],
+            "PUT",
+            "/Users/1",
+        ),
+        (["modify", "user", "1", "remove", "nickName"], "PATCH", "/Users/1"),
+        (["delete", "user", "1"], "DELETE", "/Users/1"),
+        (["query", "--me"], "GET", "/Me"),
+    ],
+)
+def test_error_responses_fail_the_command(runner, httpserver, arguments, method, path):
+    """An error answered by the server is displayed on stdout, summed up on stderr, and fails the command."""
+    httpserver.expect_oneshot_request(path, method=method).respond_with_json(
+        ERROR, status=404, content_type="application/scim+json"
+    )
+
+    result = runner.invoke(cli, ["--url", httpserver.url_for("/"), *arguments])
+
+    assert result.exit_code == 1, result.output
+    assert json.loads(result.stdout) == ERROR
+    assert result.stderr == "Error: 404 Resource 1 not found\n"
+
+
+def test_error_without_detail(runner, httpserver):
+    """An error without detail is summed up with its status."""
+    error = {
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+        "status": "500",
+    }
+    httpserver.expect_oneshot_request("/Users/1").respond_with_json(
+        error, status=500, content_type="application/scim+json"
+    )
+
+    result = runner.invoke(
+        cli, ["--url", httpserver.url_for("/"), "query", "user", "1"]
+    )
+
+    assert result.exit_code == 1, result.output
+    assert result.stderr == "Error: 500\n"
+
+
+def test_success_does_not_fail_the_command(runner, httpserver, simple_user_payload):
+    """A successful response leaves the command successful, with nothing on stderr."""
+    httpserver.expect_oneshot_request("/Users/1").respond_with_json(
+        simple_user_payload("1"), content_type="application/scim+json"
+    )
+
+    result = runner.invoke(
+        cli, ["--url", httpserver.url_for("/"), "query", "user", "1"]
+    )
+
+    assert result.exit_code == 0, result.output
+    assert result.stderr == ""

tests/test_escape.py

@@ -145,3 +145,25 @@ def test_line_breaks_and_tabulations_are_kept(runner, httpserver):
         ).output
 
     assert "first\n\tsecond" in output
+
+
+@pytest.mark.parametrize(("sequence", "escaped"), HOSTILE_SEQUENCES)
+def test_error_summary_is_escaped(runner, httpserver, sequence, escaped):
+    """The detail of an error is escaped in the summary on stderr."""
+    httpserver.expect_request("/Users/1").respond_with_json(
+        {
+            "schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
+            "status": "404",
+            "detail": f"not found{sequence}",
+        },
+        status=404,
+        content_type="application/scim+json",
+    )
+
+    result = runner.invoke(
+        cli, ["--url", httpserver.url_for("/"), "query", "user", "1"], color=True
+    )
+
+    assert result.exit_code == 1
+    assert f"not found{escaped}" in result.stderr
+    assert sequence not in result.stderr

tests/test_me.py

@@ -63,7 +63,9 @@ def test_query_error(invoke, httpserver):
 
     result = invoke("query", "--me")
 
-    assert json.loads(result.output)["status"] == "501"
+    assert result.exit_code == 1, result.output
+    assert json.loads(result.stdout)["status"] == "501"
+    assert result.stderr == "Error: 501 /Me is not supported\n"
 
 
 @pytest.mark.parametrize("arguments", [["user"], ["user", "1"]])