Skip to content

Commit 78608b1

Browse files
dlreevesmeta-codesync[bot]
authored andcommitted
Add hh --log-errors
Summary: Added a new `--log-errors` mode to the Hack client that allows typechecking files and logging their errors to either as an event or a file. The implementation uses a fire-and-forget pattern where the client receives an immediate response before the actual typechecking work completes on the server, improving responsiveness. The new mode accepts `--log-errors` to enable logging and optionally `--log-to-file <path>` to write errors to a file instead of logging an event. To support the fire-and-forget behavior, I added a `handle_after_send` mechanism in `ServerCommandTypes` that allows specific RPC commands to send a response immediately and then continue processing. The `LOG_ERRORS` command uses this to return unit to the client right away, then performs the typechecking and logging asynchronously. The implementation reuses `ServerStatusSingle.go` for typechecking individual files and respects error filtering and warning configuration just like the existing `--single` mode. Error serialization is handled in `ServerLogErrors` which produces telemetry with full diagnostic information. Reviewed By: vassilmladenov Differential Revision: D86486434 fbshipit-source-id: 0b1c38bc28d5f2d69b6bacb7d5a996b36dcd1f43
1 parent 838d640 commit 78608b1

11 files changed

Lines changed: 232 additions & 25 deletions

File tree

hphp/hack/src/client/clientArgs.ml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ let parse_check_args cmd ~from_default : ClientEnv.client_check_env =
178178

179179
let set_from x () = from := x in
180180
let single_files = ref [] in
181+
let only_log_errors = ref false in
182+
let log_to_file = ref None in
181183
let set_mode ?(validate = true) x =
182184
if validate && Option.is_some !mode then
183185
raise (Arg.Bad "only a single mode should be specified")
@@ -188,6 +190,7 @@ let parse_check_args cmd ~from_default : ClientEnv.client_check_env =
188190
end
189191
in
190192
let add_single x = single_files := x :: !single_files in
193+
let set_log_to_file x = log_to_file := Some x in
191194
let add_multi f =
192195
let files =
193196
Sys_utils.read_file f
@@ -213,6 +216,9 @@ let parse_check_args cmd ~from_default : ClientEnv.client_check_env =
213216
(MODE_STATUS_SINGLE
214217
{ filenames = single_files; show_tast; preexisting_warnings }))
215218
in
219+
let set_mode_only_log_errors config =
220+
if !only_log_errors then set_mode (MODE_LOG_ERRORS config)
221+
in
216222
let find_my_tests_max_distance = ref 1 in
217223
(* parse args *)
218224
let usage =
@@ -684,6 +690,14 @@ rewrite to the function names to something like `foo_1` and `foo_2`.
684690
Arg.String add_multi,
685691
"<path> Return errors for files read from the given file (one per line)"
686692
);
693+
( "--log-errors",
694+
Arg.Unit (fun () -> only_log_errors := true),
695+
" (mode) type check the given list of files and log their errors (use --log-to-file to specify output file)"
696+
);
697+
( "--log-to-file",
698+
Arg.String set_log_to_file,
699+
"<path> Write logged errors to specified file (use with --log-errors)"
700+
);
687701
( "--show-tast",
688702
Arg.Unit (fun () -> show_tast := true),
689703
" in combination with `--single`, output the TASTs of the file along with TAST hashes."
@@ -816,12 +830,15 @@ rewrite to the function names to something like `foo_1` and `foo_2`.
816830
);
817831

818832
set_mode_from_single_files !show_tast !preexisting_warnings;
833+
set_mode_only_log_errors
834+
{ log_file = !log_to_file; preexisting_warnings = !preexisting_warnings };
819835
let mode = Option.value !mode ~default:MODE_STATUS in
820836
(* fixups *)
821837
let (root, paths) =
822838
match (mode, args) with
823839
| (MODE_LINT, _)
824-
| (MODE_FILE_LEVEL_DEPENDENCIES, _) ->
840+
| (MODE_FILE_LEVEL_DEPENDENCIES, _)
841+
| (MODE_LOG_ERRORS _, _) ->
825842
(Wwwroot.interpret_command_line_root_parameter [], args)
826843
| (_, _) -> (Wwwroot.interpret_command_line_root_parameter args, [])
827844
in

hphp/hack/src/client/clientCheck.ml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,20 @@ let main_internal
388388
~max_errors:args.max_errors
389389
in
390390
Lwt.return (exit_status, telemetry)
391+
| ClientEnv.MODE_LOG_ERRORS { log_file; preexisting_warnings; _ } ->
392+
let files = filter_real_paths ~allow_directories:false args.paths in
393+
let error_filter =
394+
Filter_errors.Filter.make
395+
~default_all:local_config.warnings_default_all
396+
~generated_files:(ServerConfig.warnings_generated_files config)
397+
args.warning_switches
398+
in
399+
let%lwt ((), telemetry) =
400+
rpc args
401+
@@ ServerCommandTypes.LOG_ERRORS
402+
{ files; log_file; error_filter; preexisting_warnings }
403+
in
404+
Lwt.return (Exit_status.No_error, telemetry)
391405
| ClientEnv.MODE_LIST_FILES ->
392406
let%lwt (infol, telemetry) =
393407
rpc args @@ ServerCommandTypes.LIST_FILES_WITH_ERRORS

hphp/hack/src/client/clientEnv.ml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ type status_single = {
1919
preexisting_warnings: bool;
2020
}
2121

22+
type log_errors_params = {
23+
log_file: string option;
24+
preexisting_warnings: bool;
25+
}
26+
2227
type client_mode =
2328
| MODE_CST_SEARCH of string list option
2429
| MODE_DUMP_SYMBOL_INFO of string
@@ -63,6 +68,7 @@ type client_mode =
6368
| MODE_STATS
6469
| MODE_STATUS
6570
| MODE_STATUS_SINGLE of status_single
71+
| MODE_LOG_ERRORS of log_errors_params
6672
| MODE_TYPE_AT_POS of string
6773
| MODE_TYPE_AT_POS_BATCH of string list
6874
| MODE_TYPE_ERROR_AT_POS of string

hphp/hack/src/client_and_server/serverCommandTypes.ml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,13 @@ type _ t =
357357
-> ((Errors.finalized_error list * int)
358358
* Tast.program Tast_with_dynamic.t Relative_path.Map.t option)
359359
t
360+
| LOG_ERRORS : {
361+
files: string list;
362+
log_file: string option;
363+
error_filter: Filter_errors.Filter.t;
364+
preexisting_warnings: bool;
365+
}
366+
-> unit t
360367
| INFER_TYPE :
361368
file_input * File_content.Position.t
362369
-> InferAtPosService.result t
@@ -490,6 +497,7 @@ let rpc_command_needs_full_check : type a. a t -> bool =
490497
| NO_PRECHECKED_FILES -> true
491498
| STATS -> false
492499
| STATUS_SINGLE _ -> false
500+
| LOG_ERRORS _ -> false
493501
| INFER_TYPE _ -> false
494502
| INFER_TYPE_BATCH _ -> false
495503
| INFER_TYPE_ERROR _ -> false
@@ -516,3 +524,9 @@ let rpc_command_needs_full_check : type a. a t -> bool =
516524

517525
let use_priority_pipe (command : 'result t) : bool =
518526
not (rpc_command_needs_full_check command)
527+
528+
let handle_after_send : type a. a t -> (a * unit t) option =
529+
fun cmd ->
530+
match cmd with
531+
| LOG_ERRORS _ -> Some ((), cmd)
532+
| _ -> None

hphp/hack/src/client_and_server/serverCommandTypesUtils.ml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ open ServerCommandTypes
44
let debug_describe_t : type a. a t -> string = function
55
| STATUS _ -> "STATUS"
66
| STATUS_SINGLE _ -> "STATUS_SINGLE"
7+
| LOG_ERRORS _ -> "LOG_ERRORS"
78
| INFER_TYPE _ -> "INFER_TYPE"
89
| INFER_TYPE_BATCH _ -> "INFER_TYPE_BATCH"
910
| INFER_TYPE_ERROR _ -> "INFER_TYPE_ERROR"

hphp/hack/src/server/dune

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
serverInvalidateUnits
6868
serverIsSubtype
6969
serverLint
70+
serverLogErrors
7071
serverMethodJumps
7172
serverMethodJumpsBatch
7273
serverRage

hphp/hack/src/server/serverCommand.ml

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ let actually_handle genv client msg full_recheck_needed ~is_stale env =
5353
~key:Connection_tracker.Server_done_full_recheck
5454
~long_delay_okay:true;
5555

56-
let (_metadata, cmd) = msg in
56+
let (metadata, cmd) = msg in
5757
ClientProvider.ping client;
5858
let t_start = Unix.gettimeofday () in
5959
ClientProvider.track
@@ -62,30 +62,40 @@ let actually_handle genv client msg full_recheck_needed ~is_stale env =
6262
~time:t_start;
6363
Sys_utils.start_gc_profiling ();
6464
Full_fidelity_parser_profiling.start_profiling ();
65-
66-
let (new_env, response) =
67-
try ServerRpc.handle ~is_stale genv env cmd with
68-
| exn ->
69-
let e = Exception.wrap exn in
70-
raise (Nonfatal_rpc_exception (e, env))
65+
let handle_request cmd =
66+
let result =
67+
try ServerRpc.handle ~is_stale genv env metadata cmd with
68+
| exn ->
69+
let e = Exception.wrap exn in
70+
raise (Nonfatal_rpc_exception (e, env))
71+
in
72+
73+
let parsed_files = Full_fidelity_parser_profiling.stop_profiling () in
74+
ClientProvider.track
75+
client
76+
~key:Connection_tracker.Server_end_handle
77+
~log:true;
78+
let (major_gc_time, minor_gc_time) = Sys_utils.get_gc_time () in
79+
HackEventLogger.handled_command
80+
(ServerCommandTypesUtils.debug_describe_t cmd)
81+
~start_t:t_start
82+
~major_gc_time
83+
~minor_gc_time
84+
~parsed_files;
85+
result
7186
in
7287

73-
let parsed_files = Full_fidelity_parser_profiling.stop_profiling () in
74-
ClientProvider.track
75-
client
76-
~key:Connection_tracker.Server_end_handle
77-
~log:true;
78-
let (major_gc_time, minor_gc_time) = Sys_utils.get_gc_time () in
79-
HackEventLogger.handled_command
80-
(ServerCommandTypesUtils.debug_describe_t cmd)
81-
~start_t:t_start
82-
~major_gc_time
83-
~minor_gc_time
84-
~parsed_files;
85-
86-
ClientProvider.send_response_to_client client response;
87-
ClientProvider.shutdown_client client;
88-
new_env
88+
match ServerCommandTypes.handle_after_send cmd with
89+
| None ->
90+
let (new_env, response) = handle_request cmd in
91+
ClientProvider.send_response_to_client client response;
92+
ClientProvider.shutdown_client client;
93+
new_env
94+
| Some (response, post_send_cmd) ->
95+
ClientProvider.send_response_to_client client response;
96+
ClientProvider.shutdown_client client;
97+
let (new_env, ()) = handle_request post_send_cmd in
98+
new_env
8999

90100
let handle
91101
(genv : ServerEnv.genv)
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
(*
2+
* Copyright (c) 2018, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the "hack" directory of this source tree.
7+
*
8+
*)
9+
open Hh_prelude
10+
open Option.Monad_infix
11+
12+
let go
13+
(workers : MultiWorker.worker list option)
14+
(env : ServerEnv.env)
15+
(files : string list)
16+
(error_filter : Filter_errors.Filter.t)
17+
(preexisting_warnings : bool) : Telemetry.t =
18+
let file_names =
19+
List.map files ~f:(fun filename -> ServerCommandTypes.FileName filename)
20+
in
21+
22+
let ctx = Provider_utils.ctx_from_server_env env in
23+
24+
let tast_error_filter =
25+
{
26+
Tast_provider.ErrorFilter.error_filter;
27+
warnings_saved_state =
28+
ServerEnv.(env.init_env.mergebase_warning_hashes)
29+
>>= Option.some_if (not preexisting_warnings);
30+
}
31+
in
32+
33+
let (errors, _tasts) =
34+
ServerStatusSingle.go
35+
workers
36+
file_names
37+
ctx
38+
~return_expanded_tast:false
39+
~error_filter:tast_error_filter
40+
in
41+
42+
(* Define error JSON serialization function.
43+
This differs from `hh --json` to align with the information sent to VSCode
44+
(similar structure, though not identical). The `hh --json` format includes
45+
unnecessary extra fields that aren't needed for this use case. *)
46+
let error_to_json : Errors.error -> Hh_json.json =
47+
fun err ->
48+
let {
49+
User_error.severity;
50+
code = _;
51+
claim = (pos, claim_msg);
52+
reasons;
53+
explanation = _;
54+
custom_msgs;
55+
quickfixes = _;
56+
is_fixmed = _;
57+
function_pos = _;
58+
} =
59+
User_error.to_absolute err
60+
in
61+
let msg_to_json msg =
62+
Hh_json.string_ @@ Markdown_lite.render ~add_bold:false msg
63+
in
64+
let reason_to_json (pos, msg) =
65+
Hh_json.JSON_Object
66+
[("location", Pos.multiline_json pos); ("message", msg_to_json msg)]
67+
in
68+
Hh_json.JSON_Object
69+
[
70+
( "severity",
71+
Hh_json.string_ @@ User_error.Severity.to_all_caps_string severity );
72+
("range", Pos.multiline_json_no_filename pos);
73+
("message", msg_to_json claim_msg);
74+
("relatedInformation", Hh_json.array_ reason_to_json reasons);
75+
("customErrors", Hh_json.array_ msg_to_json custom_msgs);
76+
( "lineAgnosticHash",
77+
Hh_json.string_
78+
@@ Printf.sprintf "%x" (User_error.hash_error_for_saved_state err) );
79+
]
80+
in
81+
let errors = Errors.drop_fixmed_errors_in_files errors in
82+
let file_to_errors = Errors.as_map errors in
83+
let file_to_error_json =
84+
Relative_path.Map.map ~f:(List.map ~f:error_to_json) file_to_errors
85+
in
86+
let compute_file_telemetry fn =
87+
let relpath = Relative_path.create_detect_prefix fn in
88+
Telemetry.create ()
89+
|> Telemetry.string_
90+
~key:"filename"
91+
~value:(Relative_path.to_absolute relpath)
92+
|> Telemetry.json_
93+
~key:"diagnostics"
94+
~value:
95+
(Hh_json.JSON_Array
96+
(Relative_path.Map.find_opt file_to_error_json relpath
97+
|> Option.value ~default:[]))
98+
in
99+
Telemetry.create ()
100+
|> Telemetry.object_list
101+
~key:"errors"
102+
~value:(List.map files ~f:compute_file_telemetry)
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
(*
2+
* Copyright (c) 2018, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the "hack" directory of this source tree.
7+
*
8+
*)
9+
10+
(** Type check files and generate Telemetry with error diagnostics *)
11+
val go :
12+
MultiWorker.worker list option ->
13+
ServerEnv.env ->
14+
string list ->
15+
Filter_errors.Filter.t ->
16+
bool ->
17+
Telemetry.t

hphp/hack/src/server/serverRpc.ml

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,10 @@ let handle :
4242
ServerEnv.genv ->
4343
ServerEnv.env ->
4444
is_stale:bool ->
45+
ServerCommandTypes.cmd_metadata ->
4546
a ServerCommandTypes.t ->
4647
ServerEnv.env * a =
47-
fun genv env ~is_stale -> function
48+
fun genv env ~is_stale metadata -> function
4849
| ServerCommandTypes.STATUS { max_errors; error_filter } ->
4950
log_check_response env;
5051
let (error_list, dropped_count) =
@@ -118,6 +119,29 @@ let handle :
118119
None
119120
in
120121
(env, (errors, tasts))
122+
| ServerCommandTypes.LOG_ERRORS
123+
{ files; log_file; error_filter; preexisting_warnings } ->
124+
let telemetry =
125+
ServerLogErrors.go
126+
genv.ServerEnv.workers
127+
env
128+
files
129+
error_filter
130+
preexisting_warnings
131+
in
132+
let () =
133+
match log_file with
134+
| Some path ->
135+
let oc = Out_channel.create ~binary:false ~append:true path in
136+
telemetry |> Telemetry.to_json |> Hh_json.json_to_output oc;
137+
Out_channel.newline oc;
138+
Out_channel.close oc
139+
| None ->
140+
HackEventLogger.LogFileErrors.log
141+
telemetry
142+
~from:metadata.ServerCommandTypes.from
143+
in
144+
(env, ())
121145
| ServerCommandTypes.INFER_TYPE (file_input, pos) ->
122146
let path =
123147
match file_input with

0 commit comments

Comments
 (0)