Graceful error handling

- Closes #51
- Bundles errors together to display them all at the end
- Generates incomplete clients on error

Author: dbanty

CHANGELOG.md

@@ -20,6 +20,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Support for Union types ("anyOf" in OpenAPI document)
 - Support for more basic response types (integer, number, boolean)
 - Better error messages when failing to parse an OpenAPI document
+    - Error messages will contain some useful information about why it failed instead of just a stack trace
+    - Client will still be generated if there are errors, excluding endpoints that had errors
 
 ### Changes
 - The way most imports are handled was changed which *should* lead to fewer unused imports in generated files.

openapi_python_client/__init__.py

@@ -15,6 +15,7 @@
 from openapi_python_client import utils
 
 from .openapi_parser import OpenAPI, import_string_from_reference
+from .openapi_parser.errors import MultipleParseError
 
 __version__ = version(__package__)
 
@@ -88,6 +89,7 @@ def build(self) -> None:
         self._build_models()
         self._build_api()
         self._reformat()
+        self._raise_errors()
 
     def update(self) -> None:
         """ Update an existing project """
@@ -100,11 +102,19 @@ def update(self) -> None:
         self._build_models()
         self._build_api()
         self._reformat()
+        self._raise_errors()
 
     def _reformat(self) -> None:
         subprocess.run("isort --recursive --apply", cwd=self.project_dir, shell=True)
         subprocess.run("black .", cwd=self.project_dir, shell=True)
 
+    def _raise_errors(self) -> None:
+        errors = []
+        for collection in self.openapi.endpoint_collections_by_tag.values():
+            errors.extend(collection.parse_errors)
+        if errors:
+            raise MultipleParseError(parse_errors=errors)
+
     def _create_package(self) -> None:
         self.package_dir.mkdir()
         # Package __init__.py

openapi_python_client/cli.py

@@ -5,7 +5,7 @@
 
 import typer
 
-from openapi_python_client.openapi_parser.errors import ParseError
+from openapi_python_client.openapi_parser.errors import MultipleParseError, ParseError
 
 app = typer.Typer()
 
@@ -42,22 +42,31 @@ def cli(
     pass
 
 
+def _print_parser_error(e: ParseError) -> None:
+    formatted_data = pformat(e.data)
+    typer.secho(e.header, bold=True, fg=typer.colors.BRIGHT_RED, err=True)
+    typer.secho(formatted_data, fg=typer.colors.RED, err=True)
+    if e.message:
+        typer.secho(e.message, fg=typer.colors.BRIGHT_RED, err=True)
+    gh_link = typer.style(
+        "https://github.com/triaxtec/openapi-python-client/issues/new/choose", fg=typer.colors.BRIGHT_BLUE
+    )
+    typer.secho(f"Please open an issue at {gh_link}", fg=typer.colors.RED, err=True)
+    typer.secho()
+
+
 @contextmanager
 def handle_errors() -> Generator[None, None, None]:
     """ Turn custom errors into formatted error messages """
     try:
         yield
     except ParseError as e:
-        formatted_data = pformat(e.data)
-        typer.secho("ERROR: Unable to parse this part of your OpenAPI document: ", fg=typer.colors.BRIGHT_RED, err=True)
-        typer.secho(formatted_data, fg=typer.colors.RED, err=True)
-        if e.message:
-            typer.secho(e.message, fg=typer.colors.BRIGHT_RED, err=True)
-        gh_link = typer.style(
-            "https://github.com/triaxtec/openapi-python-client/issues/new/choose", fg=typer.colors.BRIGHT_BLUE
-        )
-        typer.secho(f"Please open an issue at {gh_link}", fg=typer.colors.RED, err=True)
-        typer.secho()
+        _print_parser_error(e)
+        raise typer.Exit(code=1)
+    except MultipleParseError as e:
+        typer.secho("MULTIPLE ERRORS WHILE PARSING:", underline=True, bold=True, fg=typer.colors.BRIGHT_RED, err=True)
+        for err in e.parse_errors:
+            _print_parser_error(err)
         raise typer.Exit(code=1)
 
 

openapi_python_client/openapi_parser/errors.py

@@ -1,10 +1,24 @@
-from typing import Any, Optional
+from typing import Any, List, Optional
 
 
 class ParseError(ValueError):
     """ An error raised when there's a problem parsing an OpenAPI document """
 
-    def __init__(self, data: Any, message: Optional[str] = None):
+    def __init__(
+        self,
+        data: Any,
+        header: str = "ERROR: Unable to parse this part of your OpenAPI document: ",
+        message: Optional[str] = None,
+    ):
         super().__init__()
         self.data = data
         self.message = message
+        self.header = header
+
+
+class MultipleParseError(ValueError):
+    """ Higher level error combining multiple ParseErrors """
+
+    def __init__(self, parse_errors: List[ParseError]):
+        super().__init__()
+        self.parse_errors = parse_errors

openapi_python_client/openapi_parser/openapi.py

@@ -4,6 +4,7 @@
 from enum import Enum
 from typing import Any, Dict, Generator, Iterable, List, Optional, Set
 
+from .errors import ParseError
 from .properties import EnumProperty, ListProperty, Property, property_from_dict
 from .reference import Reference
 from .responses import ListRefResponse, RefResponse, Response, response_from_dict
@@ -28,17 +29,25 @@ class EndpointCollection:
     tag: str
     endpoints: List[Endpoint] = field(default_factory=list)
     relative_imports: Set[str] = field(default_factory=set)
+    parse_errors: List[ParseError] = field(default_factory=list)
 
     @staticmethod
     def from_dict(d: Dict[str, Dict[str, Dict[str, Any]]], /) -> Dict[str, EndpointCollection]:
         """ Parse the openapi paths data to get EndpointCollections by tag """
         endpoints_by_tag: Dict[str, EndpointCollection] = {}
+
         for path, path_data in d.items():
             for method, method_data in path_data.items():
-                endpoint = Endpoint.from_data(data=method_data, path=path, method=method)
-                collection = endpoints_by_tag.setdefault(endpoint.tag, EndpointCollection(tag=endpoint.tag))
-                collection.endpoints.append(endpoint)
-                collection.relative_imports.update(endpoint.relative_imports)
+                tag = method_data.get("tags", ["default"])[0]
+                collection = endpoints_by_tag.setdefault(tag, EndpointCollection(tag=tag))
+                try:
+                    endpoint = Endpoint.from_data(data=method_data, path=path, method=method, tag=tag)
+                    collection.endpoints.append(endpoint)
+                    collection.relative_imports.update(endpoint.relative_imports)
+                except ParseError as e:
+                    e.header = f"ERROR parsing {method.upper()} {path} within {tag}. Endpoint will not be generated."
+                    collection.parse_errors.append(e)
+
         return endpoints_by_tag
 
 
@@ -127,7 +136,7 @@ def _add_parameters(self, data: Dict[str, Any]) -> None:
                 raise ValueError(f"Don't know where to put this parameter: {param_dict}")
 
     @staticmethod
-    def from_data(*, data: Dict[str, Any], path: str, method: str) -> Endpoint:
+    def from_data(*, data: Dict[str, Any], path: str, method: str, tag: str) -> Endpoint:
         """ Construct an endpoint from the OpenAPI data """
 
         endpoint = Endpoint(
@@ -136,7 +145,7 @@ def from_data(*, data: Dict[str, Any], path: str, method: str) -> Endpoint:
             description=data.get("description"),
             name=data["operationId"],
             requires_security=bool(data.get("security")),
-            tag=data.get("tags", ["default"])[0],
+            tag=tag,
         )
         endpoint._add_parameters(data)
         endpoint._add_responses(data)

openapi_python_client/openapi_parser/properties.py

@@ -335,7 +335,7 @@ def _string_based_property(
     elif string_format == "binary":
         return FileProperty(name=name, required=required, default=data.get("default"))
     else:
-        raise ValueError(f'Unsupported string format:{data["format"]}')
+        raise ParseError(data=data, message=f'Unsupported string format:{data["format"]}')
 
 
 def property_from_dict(name: str, required: bool, data: Dict[str, Any]) -> Property:

openapi_python_client/openapi_parser/responses.py

@@ -109,4 +109,4 @@ def response_from_dict(*, status_code: int, data: _ResponseDict) -> Response:
         return ListRefResponse(status_code=status_code, reference=Reference.from_ref(schema_data["items"]["$ref"]),)
     if response_type in openapi_types_to_python_type_strings:
         return BasicResponse(status_code=status_code, openapi_type=response_type)
-    raise ParseError(data, f"Unrecognized type {schema_data['type']}")
+    raise ParseError(data, message=f"Unrecognized type {schema_data['type']}")

tests/test___init__.py

@@ -136,6 +136,7 @@ def test_build(self, mocker):
         project._build_api = mocker.MagicMock()
         project._create_package = mocker.MagicMock()
         project._reformat = mocker.MagicMock()
+        project._raise_errors = mocker.MagicMock()
 
         project.build()
 
@@ -145,6 +146,7 @@ def test_build(self, mocker):
         project._build_models.assert_called_once()
         project._build_api.assert_called_once()
         project._reformat.assert_called_once()
+        project._raise_errors.assert_called_once()
 
     def test_update(self, mocker):
         from openapi_python_client import _Project, shutil
@@ -157,6 +159,7 @@ def test_update(self, mocker):
         project._build_api = mocker.MagicMock()
         project._create_package = mocker.MagicMock()
         project._reformat = mocker.MagicMock()
+        project._raise_errors = mocker.MagicMock()
 
         project.update()
 
@@ -165,6 +168,7 @@ def test_update(self, mocker):
         project._build_models.assert_called_once()
         project._build_api.assert_called_once()
         project._reformat.assert_called_once()
+        project._raise_errors.assert_called_once()
 
     def test_update_missing_dir(self, mocker):
         from openapi_python_client import _Project
@@ -448,6 +452,25 @@ def test__reformat(mocker):
     )
 
 
+def test__raise_errors(mocker):
+    from openapi_python_client import _Project, OpenAPI, MultipleParseError
+    from openapi_python_client.openapi_parser.openapi import EndpointCollection
+
+    openapi = mocker.MagicMock(
+        autospec=OpenAPI,
+        title="My Test API",
+        endpoint_collections_by_tag={
+            "default": mocker.MagicMock(autospec=EndpointCollection, parse_errors=[1]),
+            "other": mocker.MagicMock(autospec=EndpointCollection, parse_errors=[2]),
+        },
+    )
+    project = _Project(openapi=openapi)
+
+    with pytest.raises(MultipleParseError) as exc_info:
+        project._raise_errors()
+    assert exc_info.value.parse_errors == [1, 2]
+
+
 def test_load_config(mocker):
     my_data = {"class_overrides": {"_MyCLASSName": {"class_name": "MyClassName", "module_name": "my_module_name"}}}
     safe_load = mocker.patch("yaml.safe_load", return_value=my_data)

tests/test_cli.py

@@ -4,6 +4,7 @@
 import pytest
 from typer.testing import CliRunner
 
+from openapi_python_client import MultipleParseError
 from openapi_python_client.openapi_parser.errors import ParseError
 
 runner = CliRunner()
@@ -90,7 +91,7 @@ def test_generate_path(self, _create_new_client):
         _create_new_client.assert_called_once_with(url=None, path=PosixPath(path))
 
     def test_generate_handle_errors(self, _create_new_client):
-        _create_new_client.side_effect = ParseError({"test": "data"}, "this is a message")
+        _create_new_client.side_effect = ParseError({"test": "data"}, message="this is a message")
         path = "cool/path"
         from openapi_python_client.cli import app
 
@@ -104,6 +105,29 @@ def test_generate_handle_errors(self, _create_new_client):
             "Please open an issue at https://github.com/triaxtec/openapi-python-client/issues/new/choose\n\n"
         )
 
+    def test_generate_handle_multiple_parse_error(self, _create_new_client):
+        error_1 = ParseError({"test": "data"}, message="this is a message")
+        error_2 = ParseError({"other": "data"}, message="this is another message", header="Custom Header")
+        error = MultipleParseError([error_1, error_2])
+        _create_new_client.side_effect = error
+        path = "cool/path"
+        from openapi_python_client.cli import app
+
+        result = runner.invoke(app, ["generate", f"--path={path}"])
+
+        assert result.exit_code == 1
+        assert result.output == (
+            "MULTIPLE ERRORS WHILE PARSING:\n"
+            "ERROR: Unable to parse this part of your OpenAPI document: \n"
+            "{'test': 'data'}\n"
+            "this is a message\n"
+            "Please open an issue at https://github.com/triaxtec/openapi-python-client/issues/new/choose\n\n"
+            "Custom Header\n"
+            "{'other': 'data'}\n"
+            "this is another message\n"
+            "Please open an issue at https://github.com/triaxtec/openapi-python-client/issues/new/choose\n\n"
+        )
+
 
 @pytest.fixture
 def _update_existing_client(mocker):

tests/test_openapi_parser/test_openapi.py

@@ -456,7 +456,7 @@ def test_from_data(self, mocker):
             "security": {"blah": "bloo"},
         }
 
-        endpoint = Endpoint.from_data(data=data, path=path, method=method)
+        endpoint = Endpoint.from_data(data=data, path=path, method=method, tag="default")
 
         assert endpoint.path == path
         assert endpoint.method == method
@@ -468,10 +468,9 @@ def test_from_data(self, mocker):
         _add_responses.assert_called_once_with(data)
         _add_body.assert_called_once_with(data)
 
-        data["tags"] = ["a", "b"]
         del data["security"]
 
-        endpoint = Endpoint.from_data(data=data, path=path, method=method)
+        endpoint = Endpoint.from_data(data=data, path=path, method=method, tag="a")
 
         assert not endpoint.requires_security
         assert endpoint.tag == "a"
@@ -502,16 +501,16 @@ class TestEndpointCollection:
     def test_from_dict(self, mocker):
         from openapi_python_client.openapi_parser.openapi import EndpointCollection, Endpoint
 
-        data_1 = mocker.MagicMock()
-        data_2 = mocker.MagicMock()
-        data_3 = mocker.MagicMock()
+        data_1 = {}
+        data_2 = {"tags": ["tag_2", "tag_3"]}
+        data_3 = {}
         data = {
-            "path_1": {"method_1": data_1, "method_2": data_2,},
-            "path_2": {"method_1": data_3,},
+            "path_1": {"method_1": data_1, "method_2": data_2},
+            "path_2": {"method_1": data_3},
         }
-        endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="tag_1", relative_imports={"1", "2"})
+        endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"1", "2"})
         endpoint_2 = mocker.MagicMock(autospec=Endpoint, tag="tag_2", relative_imports={"2"})
-        endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="tag_1", relative_imports={"2", "3"})
+        endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"2", "3"})
         endpoint_from_data = mocker.patch.object(
             Endpoint, "from_data", side_effect=[endpoint_1, endpoint_2, endpoint_3]
         )
@@ -520,12 +519,41 @@ def test_from_dict(self, mocker):
 
         endpoint_from_data.assert_has_calls(
             [
-                mocker.call(data=data_1, path="path_1", method="method_1"),
-                mocker.call(data=data_2, path="path_1", method="method_2"),
-                mocker.call(data=data_3, path="path_2", method="method_1"),
+                mocker.call(data=data_1, path="path_1", method="method_1", tag="default"),
+                mocker.call(data=data_2, path="path_1", method="method_2", tag="tag_2"),
+                mocker.call(data=data_3, path="path_2", method="method_1", tag="default"),
             ]
         )
         assert result == {
-            "tag_1": EndpointCollection("tag_1", endpoints=[endpoint_1, endpoint_3], relative_imports={"1", "2", "3"}),
+            "default": EndpointCollection(
+                "default", endpoints=[endpoint_1, endpoint_3], relative_imports={"1", "2", "3"}
+            ),
             "tag_2": EndpointCollection("tag_2", endpoints=[endpoint_2], relative_imports={"2"}),
         }
+
+    def test_from_dict_errors(self, mocker):
+        from openapi_python_client.openapi_parser.openapi import EndpointCollection, Endpoint, ParseError
+
+        data_1 = {}
+        data_2 = {"tags": ["tag_2", "tag_3"]}
+        data_3 = {}
+        data = {
+            "path_1": {"method_1": data_1, "method_2": data_2},
+            "path_2": {"method_1": data_3},
+        }
+        endpoint_from_data = mocker.patch.object(
+            Endpoint, "from_data", side_effect=[ParseError("1"), ParseError("2"), ParseError("3")]
+        )
+
+        result = EndpointCollection.from_dict(data)
+
+        endpoint_from_data.assert_has_calls(
+            [
+                mocker.call(data=data_1, path="path_1", method="method_1", tag="default"),
+                mocker.call(data=data_2, path="path_1", method="method_2", tag="tag_2"),
+                mocker.call(data=data_3, path="path_2", method="method_1", tag="default"),
+            ]
+        )
+        assert result["default"].parse_errors[0].data == "1"
+        assert result["default"].parse_errors[1].data == "3"
+        assert result["tag_2"].parse_errors[0].data == "2"