Merge pull request #20 from cryptosense/fix-error-messages

Fix the error messages/exit codes.

Author: Nemeras

cs_api_cli/cs_api_cli.ml

@@ -28,7 +28,7 @@ let resolve_project_name ~client ~api ~project_id ~project_name =
   | (None, Some name) -> (
     Cs_api_core.build_list_projects_request ~api
     |> Cs_api_io.send_request ~client
-    >>= Cs_api_io.get_response
+    >>= Cs_api_io.get_response_graphql
     >>= fun body ->
     let projects = Cs_api_core.parse_list_projects_response ~body in
     match CCList.Assoc.get ~eq:String.equal name projects with
@@ -51,7 +51,7 @@ let upload_trace
   >>= fun project_id ->
   Cs_api_core.build_s3_signed_post_request ~api
   |> Cs_api_io.send_request ~client
-  >>= Cs_api_io.get_response
+  >>= Cs_api_io.get_response_graphql
   >>= (fun body ->
         match Cs_api_core.parse_s3_signature_request ~body with
         | None ->
@@ -71,13 +71,14 @@ let upload_trace
         in
         Lwt.return (Ok import_request))
   >>= Cs_api_io.send_request ~client
-  >>= Cs_api_io.get_response
-  >|= fun _ -> Printf.printf "Trace uploaded\n")
+  >>= Cs_api_io.get_response_graphql)
   >|= function
-  | Ok _ as ok -> ok
+  | Ok _ ->
+    Printf.printf "Trace uploaded\n";
+    0
   | Error message ->
     Printf.printf "%s\n" message;
-    Error ()
+    1
 
 let trace_file =
   let doc = "Path to the file containing the trace" in
@@ -188,4 +189,5 @@ let default_info = Cmdliner.Term.info "cs-api" ~version:"%%VERSION_NUM%%"
 let default_cmd = (default_term, default_info)
 
 let () =
-  Cmdliner.Term.eval_choice default_cmd [upload_trace_cmd] |> Cmdliner.Term.exit
+  Cmdliner.Term.eval_choice default_cmd [upload_trace_cmd]
+  |> Cmdliner.Term.exit_status

cs_api_io/cs_api_io.ml

@@ -77,3 +77,33 @@ let get_response {Response.code; body} =
   else
     let message = Printf.sprintf "HTTP response: %d\n%s" code body in
     Lwt_result.fail message
+
+let get_graphql_errors (json : Yojson.Safe.t) =
+  (match json with
+  | `Assoc _ -> (Some (Yojson.Safe.Util.member "errors" json), [])
+  | _ -> (None, ["Unexpected response from the server"]))
+  |> function
+  | (None, errors) -> errors
+  | (Some `Null, _) -> []
+  | (Some errors_json, _) -> (
+    match errors_json with
+    | `List error_list ->
+      error_list
+      |> CCList.map (Yojson.Safe.Util.member "message")
+      |> CCList.map (function
+           | `String error_message -> error_message
+           | _ -> "Unexpected response from the server")
+    | _ -> ["Unexpected response from the server"])
+
+let get_response_graphql {Response.code; body} =
+  if code < 300 then
+    try
+      let json = Yojson.Safe.from_string body in
+      match get_graphql_errors json with
+      | [] -> Lwt_result.return body
+      | errors -> CCString.concat "\n" errors |> Lwt_result.fail
+    with
+    | _ -> Lwt_result.return body
+  else
+    let message = Printf.sprintf "HTTP response: %d\n%s" code body in
+    Lwt_result.fail message

cs_api_io/cs_api_io.mli

@@ -21,3 +21,7 @@ val send_request :
   client:t -> Api.Request.t -> (Response.t, string) result Lwt.t
 
 val get_response : Response.t -> (string, string) result Lwt.t
+
+val get_graphql_errors : Yojson.Safe.t -> string list
+
+val get_response_graphql : Response.t -> (string, string) result Lwt.t

test/dune

@@ -1,3 +1,3 @@
 (test
  (name test)
- (libraries api cs_api_core alcotest multipart_form_writer))
+ (libraries api cs_api_core cs_api_io alcotest multipart_form_writer))

test/test.ml

@@ -64,4 +64,5 @@ let request_builder_tests =
 let () =
   Alcotest.run "API Client"
     [ ("Request builders", request_builder_tests)
-    ; ("Multipart writer", Test_writer.accumulator) ]
+    ; ("Multipart writer", Test_writer.accumulator)
+    ; ("Graphql errors parsing", Test_api_io.tests) ]

test/test_api_io.ml

@@ -0,0 +1,52 @@
+let test ~name ~body ~expected_errors =
+  ( name
+  , `Quick
+  , fun () ->
+      let json = Yojson.Safe.from_string body in
+      let result = Cs_api_io.get_graphql_errors json in
+      Alcotest.(check (list string)) name result expected_errors )
+
+let body_ok = {|{"data":{"createTrace":null}}|}
+
+let body_error =
+  {|{ 
+    "errors": [
+      {  
+        "message": "No project with this ID was found",
+        "locations": [{"line":3,"column":9}],
+        "path": ["createTrace"]
+      }
+    ],
+    "data": {"createTrace":null}
+  }|}
+
+let body_no_error_message =
+  {|{
+    "errors": [
+      {
+        "locations": [{"line":3,"column":9}],
+        "path": ["createTrace"]
+      }
+    ],
+    "data": {"createTrace":null}
+  }|}
+
+let body_multiple_errors =
+  {|{
+    "errors": [
+      {"message": "No project with this ID was found"},
+      {"message": "No project with this name was found"}
+    ],
+    "data": {"createTrace":null}
+  }|}
+
+let tests =
+  [ test ~name:"test_ok" ~body:body_ok ~expected_errors:[]
+  ; test ~name:"test_error" ~body:body_error
+      ~expected_errors:["No project with this ID was found"]
+  ; test ~name:"test_no_error_message" ~body:body_no_error_message
+      ~expected_errors:["Unexpected response from the server"]
+  ; test ~name:"test_multiple_errors" ~body:body_multiple_errors
+      ~expected_errors:
+        [ "No project with this ID was found"
+        ; "No project with this name was found" ] ]