Skip to content

Commit

Permalink
error streaming: fail nicer in case of watchman failure
Browse files Browse the repository at this point in the history
Summary:
Context: For error streaming, the client will attempt to open and tail the errors.bin file. The header of that file contains the watchman clock corresponding to the start of the typecheck, more precisely to the watchman notification for the file changes that triggered the typecheck producing these errors.

Upon reading that header, the client will synchronously ask watchman "has there been any file changes since that clock?". If so, it will close the errors.bin file and wait for a new one to be produced by hh. This is to avoid bad UX when a user runs `hh` just after saving a file. If we didn't do that, we might immediately return with a message saying "files have changed, please re-run hh".

This diff deals with the case where that watchman query fails. Prior to this diff, the user would see an exception. With this diff, we proceed with tailing the errors file instead. This might mean than we end up displaying the message "files have changed, please re-run hh", but that's relatively unlikely, and even so that's a better UX than loudly failing.

We do log those watchman errors anyway, see previous diff D66300063

Reviewed By: patriciamckenzie

Differential Revision: D66457845

fbshipit-source-id: 0cd74023c37ee271cf0d22106f2072b443c75188
  • Loading branch information
Catherine Gasnier authored and facebook-github-bot committed Nov 25, 2024
1 parent 9d0ab75 commit 9f83204
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions hphp/hack/src/client/clientCheckStatus.ml
Original file line number Diff line number Diff line change
Expand Up @@ -630,9 +630,12 @@ let rec keep_trying_to_open
match since_result with
| Error e ->
Hh_logger.log "Errors-file: watchman failure:\n%s\n" e;
progress_callback None;
Printf.eprintf "Watchman failure.\n%s\n%!" e;
raise Exit_status.(Exit_with Exit_status.Watchman_failed)
(* The errors file is present, was started at clock `clock` but the watchman query failed.
We'll assume there has not been any file change since that clock.,
If there has, hh_server will ultimately tell us with a restart sentinel, and we'll
ask the user to re-run hh. This is sub-optimal UX, but still better UX than
loudly failing now. *)
Lwt.return (pid, fd)
| Ok relative_raw_updates ->
let raw_updates =
relative_raw_updates
Expand Down

0 comments on commit 9f83204

Please sign in to comment.