fix exception handling in get-def command

Summary:
if i'm reading it correctly, if the server raises an exception during
`get-def`, it writes to the server log and returns `None` which is not marshaled
over the channel so the client blocks until it times out.

instead, it should return an error to the user.

Reviewed By: avikchaudhuri

Differential Revision: D5088814

fbshipit-source-id: cf1ceca05ac74a2005bf72c4ee1c40b452814d7f

src/commands/getDefCommand.ml

@@ -73,19 +73,26 @@ let main option_values root json pretty strip_root path args () =
   ServerProt.cmd_to_channel oc
     (ServerProt.GET_DEF (file, line, column));
   (* command result will be a position structure with full file path *)
-  let loc:Loc.t = Timeout.input_value ic in
-  (* format output *)
-  if json || pretty
-  then (
-    (* TODO: this format is deprecated but can't be backwards-compatible.
-       should be replaced with just `Reason.json_of_loc loc`. *)
-    let open Hh_json in
-    let json =
-      JSON_Object (Errors.deprecated_json_props_of_loc ~strip_root loc) in
-    print_endline (json_to_string ~pretty json)
-  ) else
-    if option_values.from = "vim" || option_values.from = "emacs"
-    then print_endline (Errors.Vim_emacs_output.string_of_loc ~strip_root loc)
-    else print_endline (range_string_of_loc ~strip_root loc)
+  let response: ServerProt.get_def_response = Timeout.input_value ic in
+  match response with
+  | Ok loc ->
+    (* format output *)
+    if json || pretty
+    then (
+      (* TODO: this format is deprecated but can't be backwards-compatible.
+         should be replaced with just `Reason.json_of_loc loc`. *)
+      let open Hh_json in
+      let json =
+        JSON_Object (Errors.deprecated_json_props_of_loc ~strip_root loc) in
+      print_endline (json_to_string ~pretty json)
+    ) else
+      if option_values.from = "vim" || option_values.from = "emacs"
+      then print_endline (Errors.Vim_emacs_output.string_of_loc ~strip_root loc)
+      else print_endline (range_string_of_loc ~strip_root loc)
+  | Error exn_msg ->
+      Utils_js.prerr_endlinef
+        "Could not get definition for %s:%d:%d\n%s"
+        (ServerProt.file_input_get_filename file) line column
+        exn_msg
 
 let command = CommandSpec.command spec main

src/server/server.ml

@@ -404,19 +404,15 @@ let collate_errors =
         | profiling, Some cx, _, _ -> profiling, cx
         | _  -> failwith "Couldn't parse file"
       in
-      Some (GetDef_js.getdef_get_result
+      Ok (GetDef_js.getdef_get_result
         profiling
         command_context
         ~options
         cx
         state
       )
     with exn ->
-      Hh_logger.warn
-        "Could not get definition for %s:%d:%d\n%s"
-        filename line col
-        (Printexc.to_string exn);
-      None
+      Error (Printexc.to_string exn)
     in
     GetDef_js.getdef_unset_hooks ();
     result
@@ -634,8 +630,8 @@ let collate_errors =
         (gen_flow_files ~options !env files: ServerProt.gen_flow_file_response)
           |> marshal
     | ServerProt.GET_DEF (fn, line, char) ->
-        (get_def ~options client_logging_context (fn, line, char): Loc.t option)
-          |> marshal_option
+        (get_def ~options client_logging_context (fn, line, char): ServerProt.get_def_response)
+          |> marshal
     | ServerProt.GET_IMPORTS module_names ->
         get_imports ~options module_names
           |> marshal

src/server/serverProt.ml

@@ -75,6 +75,7 @@ type dump_types_response = (
   (Loc.t * string * string * string option * Reason.t list) list,
   Loc.t * string
 ) result
+type get_def_response = (Loc.t, string) result
 type infer_type_response = (
   Loc.t * string option * string option * Reason.t list,
   Loc.t * string