modifying python and go files for error-ux modification (#415)

* modifying python and go files for error-ux modification * addressing review comments * v0.2.19 --------- Co-authored-by: Ashutosh Kumar <[email protected]>
open-traffic-generator · Apr 10, 2023 · adec4e6 · adec4e6
1 parent 0f37d1c
commit adec4e6
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 60 deletions.
diff --git a/openapiart/common.go b/openapiart/common.go
@@ -272,7 +272,6 @@ type Validation interface {
 func (obj *validation) validationResult() error {
 	obj.constraints = make(map[string]map[string]Constraints)
 	if len(obj.validationErrors) > 0 {
-		obj.validationErrors = append(obj.validationErrors, "validation errors")
 		errors := strings.Join(obj.validationErrors, "\n")
 		obj.validationErrors = nil
 		return fmt.Errorf(errors)

diff --git a/openapiart/common.py b/openapiart/common.py
@@ -133,6 +133,22 @@ def set_verify(self, verify):
             urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
             self.logger.warning("Certificate verification is disabled")
 
+    def _parse_response_error(self, response_code, response_text):
+        error_response = ""
+        try:
+            error_response = yaml.safe_load(response_text)
+        except Exception as _:
+            error_response = response_text
+
+        err_obj = Error()
+        try:
+            err_obj.deserialize(error_response)
+        except Exception as _:
+            err_obj.code = response_code
+            err_obj.errors = [str(error_response)]
+
+        raise Exception(err_obj)
+
     def send_recv(
         self,
         method,
@@ -183,9 +199,7 @@ def send_recv(
                 # content types
                 return response
         else:
-            raise Exception(
-                response.status_code, yaml.safe_load(response.text)
-            )
+            self._parse_response_error(response.status_code, response.text)
 
 
 class OpenApiStatus:

diff --git a/openapiart/generator.py b/openapiart/generator.py
@@ -406,17 +406,14 @@ def _serialize_payload(self, payload):
             payload = json.dumps(payload)
         return payload
 
-    def from_exception(self, grpc_error):
-        # type: (grpc.RpcError) -> Error
-        err = None
-        if isinstance(grpc_error, grpc.RpcError):
-            err = self.error()
-            try:
-                err.deserialize(grpc_error.details())
-            except Exception as e:
-                err.code = 2 # code for unknown error
-                err.errors = [grpc_error.details()]
-        return err
+    def _raise_exception(self, grpc_error):
+        err = self.error()
+        try:
+            err.deserialize(grpc_error.details())
+        except Exception as _:
+            err.code = grpc_error.code().value[0]
+            err.errors = [grpc_error.details()]
+        raise Exception(err)
 
     @property
     def request_timeout(self):
@@ -498,11 +495,14 @@ def close(self):
                         ),
                     )
                     self._write(2, "stub = self._get_stub()")
+                    self._write(2, "try:")
                     self._write(
-                        2,
+                        3,
                         "res_obj = stub.%s(req_obj, timeout=self._request_timeout)"
                         % rpc_method.operation_name,
                     )
+                    self._write(2, "except grpc.RpcError as grpc_error:")
+                    self._write(3, "self._raise_exception(grpc_error)")
                 including_default, return_byte = self._process_good_response(
                     rpc_method
                 )
@@ -609,23 +609,6 @@ def _write_http_api_class(self, methods):
             self._write(1, "@verify.setter")
             self._write(1, "def verify(self, value):")
             self._write(2, "self._transport.set_verify(value)")
-            self._write()
-            self._write(1, "def from_exception(self, exception):")
-            self._write(2, "# type (Exception) -> Error")
-            self._write(2, "err_obj = None")
-            self._write(2, "if len(exception.args) != 2:")
-            self._write(3, "return err_obj")
-            self._write(2, "if isinstance(exception.args[0], int):")
-            self._write(3, "err_obj = self.error()")
-            self._write(3, "err_obj.code = exception.args[0]")
-            self._write(3, "if not isinstance(exception.args[1], dict):")
-            self._write(4, "err_obj.errors = [str(exception.args[1])]")
-            self._write(3, "else:")
-            self._write(4, "try:")
-            self._write(5, "err_obj.deserialize(exception.args[1])")
-            self._write(4, "except Exception as e:")
-            self._write(5, "err_obj.errors = [exception.args[1]]")
-            self._write(2, "return err_obj")
 
             for method in methods:
                 print("generating method %s" % method["name"])
@@ -726,6 +709,37 @@ def _write_api_class(self, methods, factories):
             self._write(2, "print('[WARNING]: %s' % msg)")
             self._write(2, "self.__warnings__.append(msg)")
 
+            self._write()
+            self._write(1, "def _deserialize_error(self, err_string):")
+            self._write(2, "# type: (str) -> Union[Error, None]")
+            self._write(2, "err = self.error()")
+            self._write(2, "try:")
+            self._write(3, "err.deserialize(err_string)")
+            self._write(2, "except Exception:")
+            self._write(3, "err = None")
+            self._write(2, "return err")
+
+            self._write()
+            self._write(1, "def from_exception(self, error):")
+            self._write(2, "# type: (Exception) -> Union[Error, None]")
+            self._write(2, "if isinstance(error, Error):")
+            self._write(3, "return error")
+            self._write(2, "elif isinstance(error, grpc.RpcError):")
+            self._write(3, "err = self._deserialize_error(error.details())")
+            self._write(3, "if err is not None:")
+            self._write(4, "return err")
+            self._write(3, "err = self.error()")
+            self._write(3, "err.code = error.code().value[0]")
+            self._write(3, "err.errors = [error.details()]")
+            self._write(3, "return err")
+            self._write(2, "elif isinstance(error, Exception):")
+            self._write(3, "if len(error.args) != 1:")
+            self._write(4, "return None")
+            self._write(3, "if isinstance(error.args[0], Error):")
+            self._write(4, "return error.args[0]")
+            self._write(3, "elif isinstance(error.args[0], str):")
+            self._write(4, "return self._deserialize_error(error.args[0])")
+
             for method in methods:
                 print("generating method %s" % method["name"])
                 self._write()

diff --git a/openapiart/goserver/go_controller_generator.py b/openapiart/goserver/go_controller_generator.py
@@ -166,16 +166,16 @@ def _write_method(self, w, ctrl, route):
                         item = {new_modelname}()
                         err := item.FromJson(string(body))
                         if err != nil {{
-                            ctrl.{rsp_400_error}(w, 400, err)
+                            ctrl.{rsp_400_error}(w, "validation", err)
                             return
                         }}
                     }} else {{
-                        ctrl.{rsp_400_error}(w, 400, readError)
+                        ctrl.{rsp_400_error}(w, "validation", readError)
                         return
                     }}
                 }} else {{
                     bodyError := errors.New(\"Request does not have a body\")
-                    ctrl.{rsp_500_error}(w, 400, bodyError)
+                    ctrl.{rsp_500_error}(w, "validation", bodyError)
                     return
                 }}
                 result, err := ctrl.handler.{operation_name}(item, r)""".format(
@@ -195,7 +195,7 @@ def _write_method(self, w, ctrl, route):
 
         w.write_line(
             """if err != nil {{
-                ctrl.{rsp_error}(w, 500, err)
+                ctrl.{rsp_error}(w, "internal", err)
                 return
             }}
             """.format(
@@ -226,7 +226,7 @@ def _write_method(self, w, ctrl, route):
             if self._need_warning_check(route, response):
                 rsp_section = """data, err := {mrl_name}MrlOpts.Marshal(result.{struct}().Msg())
                         if err != nil {{
-                            ctrl.{rsp_400_error}(w, 400, err)
+                            ctrl.{rsp_400_error}(w, "validation", err)
                         }}
                         httpapi.WriteCustomJSONResponse(w, 200, data)
                     """.format(
@@ -256,7 +256,7 @@ def _write_method(self, w, ctrl, route):
                 )
             )
         w.write_line(
-            'ctrl.{rsp_500_error}(w, 500, errors.New("Unknown error"))'.format(
+            'ctrl.{rsp_500_error}(w, "internal", errors.New("Unknown error"))'.format(
                 rsp_500_error=rsp_error
             )
         )
@@ -278,26 +278,30 @@ def _write_method(self, w, ctrl, route):
                     result = {models_prefix}NewError()
                     err := result.FromJson(rsp_err.Error())
                     if err != nil {{
-                        result = nil
+                        result.Msg().Code = statusCode
+                        err = result.SetKind(errorKind)
+                        if err != nil {{
+                            log.Print(err.Error())
+                        }}
+                        result.Msg().Errors = []string{{rsp_err.Error()}}
                     }}
                 }}
             """.format(
                 models_prefix=self._ctx.models_prefix,
             )
 
             w.write_line(
-                """func (ctrl *{struct_name}) {method_name}(w http.ResponseWriter, status_code int, rsp_err error) {{
+                """func (ctrl *{struct_name}) {method_name}(w http.ResponseWriter, errorKind {models_prefix}ErrorKindEnum, rsp_err error) {{
                 var result {models_prefix}{schema}
+                var statusCode int32
+                if errorKind == "validation" {{
+                    statusCode = 400
+                }} else if errorKind == "internal" {{
+                    statusCode = 500
+                }}
                 {set_errors}
-                if result != nil {{
-                    if _, err := httpapi.WriteJSONResponse(w, int(result.Code()), result); err != nil {{
-                        log.Print(err.Error())
-                    }}
-                }} else {{
-                    data := []byte(rsp_err.Error())
-                    if _, err := httpapi.WriteCustomJSONResponse(w, status_code, data); err != nil {{
-                        log.Print(err.Error())
-                    }}
+                if _, err := httpapi.WriteJSONResponse(w, int(result.Code()), result); err != nil {{
+                    log.Print(err.Error())
                 }}
             }}
             """.format(

diff --git a/openapiart/tests/test_grpc_response.py b/openapiart/tests/test_grpc_response.py
@@ -1,3 +1,5 @@
+import grp
+from traceback import format_exception
 import grpc
 import pytest
 import json
@@ -71,10 +73,13 @@ def test_grpc_set_config_error_struct(utils, grpc_api):
     payload["l"]["integer"] = 100
     try:
         grpc_api.set_config(payload)
-    except grpc.RpcError as e:
+    except Exception as e:
+        e_obj = e.args[0]
+        assert e_obj.code == 13
+        assert e_obj.errors[1] == "err2"
         err_obj = grpc_api.from_exception(e)
+        assert err_obj is not None
         assert err_obj.code == 13
-        assert len(err_obj.errors) == 2
         assert err_obj.errors[1] == "err2"
 
 
@@ -84,10 +89,13 @@ def test_grpc_set_config_error_str(utils, grpc_api):
     payload["l"]["integer"] = -3
     try:
         grpc_api.set_config(payload)
-    except grpc.RpcError as e:
+    except Exception as e:
+        e_obj = e.args[0]
+        assert e_obj.code == 13
+        assert e_obj.errors[0] == "some random error!"
         err_obj = grpc_api.from_exception(e)
-        assert err_obj.code == 2
-        assert len(err_obj.errors) == 1
+        assert err_obj is not None
+        assert err_obj.code == 13
         assert err_obj.errors[0] == "some random error!"
 
 

diff --git a/openapiart/tests/test_http_response.py b/openapiart/tests/test_http_response.py
@@ -51,12 +51,14 @@ def test_error_for_non_okay_error_codes(api):
     config.d_values = [config.A, config.B, config.C]
     config.level.l1_p1.l2_p1.l3_p1 = "test"
     config.level.l1_p2.l4_p1.l1_p2.l4_p1.l1_p1.l2_p1.l3_p1 = "test"
-    rest_error = "(500, {'detail': 'invalid data type'})"
     with pytest.raises(Exception) as execinfo:
         api.set_config(config)
-    assert str(execinfo.value.args) == rest_error
 
+    e = execinfo.value.args[0]
+    e.code == 500
+    assert str(e.errors[0]) == "{'detail': 'invalid data type'}"
     err = api.from_exception(execinfo.value)
+    assert err is not None
     assert err.code == 500
     assert str(err.errors[0]) == "{'detail': 'invalid data type'}"
 
@@ -74,7 +76,12 @@ def test_error_structure_for_non_okay_error_codes(api):
     with pytest.raises(Exception) as execinfo:
         api.set_config(config)
 
+    e = execinfo.value.args[0]
+    e.code == 400
+    assert e.kind == "validation"
+    assert e.errors[0] == "err for validation"
     err = api.from_exception(execinfo.value)
+    assert err is not None
     assert err.code == 400
     assert err.kind == "validation"
     assert err.errors[0] == "err for validation"
diff --git a/pkg/common.go b/pkg/common.go
@@ -274,7 +274,6 @@ type Validation interface {
 func (obj *validation) validationResult() error {
 	obj.constraints = make(map[string]map[string]Constraints)
 	if len(obj.validationErrors) > 0 {
-		obj.validationErrors = append(obj.validationErrors, "validation errors")
 		errors := strings.Join(obj.validationErrors, "\n")
 		obj.validationErrors = nil
 		return fmt.Errorf(errors)

diff --git a/pkg/transport_test.go b/pkg/transport_test.go
@@ -361,7 +361,7 @@ func TestHttpErrorStringSetConfig(t *testing.T) {
 	// if user wants to get the json now
 	errSt, _ := api.FromError(err)
 	assert.Equal(t, errSt.Code(), int32(500))
-	assert.False(t, errSt.HasKind())
+	assert.Equal(t, errSt.Kind(), openapiart.ErrorKind.INTERNAL)
 	assert.Equal(t, errSt.Errors()[0], "client error !!!!")
 }
 

diff --git a/setup.py b/setup.py
@@ -5,7 +5,7 @@
 import setuptools
 
 pkg_name = "openapiart"
-version = "0.2.18"
+version = "0.2.19"
 
 base_dir = os.path.dirname(os.path.abspath(__file__))
 with open(os.path.join(base_dir, "README.md")) as fid: