Better Build Error if Toplevel Fails

[patricoferris]

From #1265 it would seem if some tutorials are broken then the build process still succeeds, this makes sense as some of the tutorials purposefully fail in order to explain compiler errors (see https://ocaml.org/learn/tutorials/common_error_messages.html).

However, it would also be great to have the build fail (or at least a test case) if a toplevel codeblock we expected to work didn't so issues like 1265 are caught at build time and not live on the site!

This issue contains quite a lot of work and perhaps a good starting point is looking at better error reporting during the build phase of the site (somewhere in this file would probably be a good place to start https://github.com/ocaml/ocaml.org/blob/master/script/code_top.ml) but bare in mind that we also want the compiler error message tutorial to still fail, but not break the build.

[TildaDares]

Can I try this?

[gs0510]

@TildaDares go for it!

[TildaDares]

@patricoferris What is the loading procedure example #1265 is referencing?

[patricoferris]

Hi @TildaDares,

The loading procedure is referencing things related to the OCaml toplevel.

In particular the very first code block here: https://github.com/ocaml/ocaml.org/blob/master/site/learn/tutorials/introduction_to_gtk.md#introduction-to-gtk

The site runs these code blocks as if they were actual code using the script I linked to above. But if you go to the website you will see that it did indeed run the code, but it is broken so all of the code blocks have errors. Does this help?

[patricoferris]

No worries, this is a complicated issue (that's the point 😄).

So this compiler error is telling you that it cannot found this module that you want. The best way to work out how to compile and run this script is to see how the Makefile currently does. If you search files for code_top you can find https://github.com/ocaml/ocaml.org/blob/eca0b931985ca4cf5c646224c2292e53a0d73604/Makefile.common#L90-L93.

This compiles the code_top script (you can try it yourself make script/code_top). With a little extra digging to see where code_top is used we find md_preprocess.ml. Looking at how it consumes markdown files we see: https://github.com/ocaml/ocaml.org/blob/master/script/md_preprocess.ml#L89-L98 where the first line consumes from stdin.

So to run the code we can do make script/md_preprocess and then something like

cat site/learn/tutorials/a_first_hour_with_ocaml.md | script/md_preprocess

cat prints the markdown to stdout and the pipe operator | takes stdout and pipes it to stdin (where md_preprocess wants the file to come from). The result is then printed to stdout. Have a look at the code blocks that said ocamltop they should now be HTML and have been run.

Hopefully that's helped a little bit :))

[TildaDares]

@patricoferris https://github.com/ocaml/ocaml.org/blob/master/script/code_top.ml#L67 is the line that executes the code so I thought maybe I could use a try..with block around it like this:

try
      let lexbuf = Lexing.from_string phrase in
      let phrase = !Toploop.parse_toplevel_phrase lexbuf in
      (try
        ignore(Toploop.execute_phrase true Format.str_formatter phrase);
      with |
        exn ->
        eprintf "An error has occured:\n
                stdout: %S\n
                stderr: %S\n"
                  out err
       );
      let exec_output = Format.flush_str_formatter () in
      let out, err = get_stdout_stderr_and_restore () in
      Normal(exec_output, out, err)

Now the issue says to catch it at build time. The hack I came up with only prints the error out when you do this cat site/learn/tutorials/a_first_hour_with_ocaml.md | script/md_preprocess. I tried to find documentation on the Makefile warn target but I didn't see anything that would help. Another issue with this is with the out and err values. Those values are defined after the try..with block so I can't access them. I've tried moving the block after the try..with inside the try block but that didn't work. I could also use the Printexc module but the error message is extremely vague. And another thing I noticed is that I rarely get the ***** STARTING OCAML TOPLEVEL ****** when running make. Isn't that supposed to happen every time?

[patricoferris]

Hi @TildaDares,

Sometimes things are cached by make if targets and dependencies don't change (I think) so it's always worth doing a make clean && make if you were expecting something different :))

Secondly, we can do a little digging to learn more about Toploop.execute_phrase. So it appears it returns false if something went wrong. Note the difference between (a) the code being evaluated not working and (b) the actual Toploop.execute_phrase breaking. We're interested in (a).

So we need to inspect that boolean. Maybe have a go with:

let success =  Toploop.execute_phrase true Format.str_formatter phrase in 
if success then print_endline "Woohoo!" else failwith "Somthing is broken :(";

And running:

cat site/learn/tutorials/introduction_to_gtk.md | script/md_preprocess

[TildaDares]

@patricoferris Thanks for the explanation. I already tried the solution you suggested using utop and it works. But on some errors like unbound value it doesn't return false. It just returns the error.

[TildaDares]

@patricoferris It only works for exceptions

[patricoferris]

Ah okay, awesome work getting this far especially since this is your first foray into OCaml!

The actual failure modes seem a little undocumented, but try...with blocks are very like pattern-match statements. In fact you can pattern match on the exception itself i.e.

try 
  eval "GtkButton.ToggleButton.create;;"
with 
  | Failure m -> (* this would catch the 1/0 case *)
  | Env.Error _ -> (* this catches this case *)

Note I think you need to run utop-full so it has access to global things like Env in this case.

[TildaDares]

@patricoferris So this is what I ended up with

let lexbuf = Lexing.from_string phrase in
      let phrase = !Toploop.parse_toplevel_phrase lexbuf in
      (try
      ignore(Toploop.execute_phrase true Format.str_formatter phrase);
      with
      | exn ->
        eprintf "There was an error\n"
      );
      let exec_output = Format.flush_str_formatter () in
      let out, err = get_stdout_stderr_and_restore () in
      Normal(exec_output, out, err)

I'm still working on printing descriptive errors. Now the issue is with printing these errors during the build process i.e. when running make. I had a look into the file responsible for preprocessing the .md files during the build. I tried searching for ways to pass a variable from a target file to the makefile I linked and from what I've read so far I don't think this is possible.

[patricoferris]

Okay great. I think if you pattern-match on the exceptions you can give more information because you are "destructuring" the exception so you can know more about it.

Whilst doing that we could also completely fail the build if a particular error occurs. I don't think there is any case where want Toplevel.execute_phrase to throw an Env.Error _ exception, this might be a good candidate to just fail the build on, you may want to experiment with that to make sure it's true.

[TildaDares]

@patricoferris I tried this

try
      ignore(Toploop.execute_phrase true Format.str_formatter phrase);
      with
      | exn -> exit 1

This exits the script with a Fatal Error: End_of_File raised but doesn't fail the build. I started reading up on how to fail a build when it encounters an error but I don't really understand it https://www.google.com/url?sa=t&source=web&rct=j&url=https://stackoverflow.com/questions/35701016/make-stop-compilation-on-error&ved=2ahUKEwjAttOV8ZPwAhUpahUIHbSjAOQQrAIoAHoECAYQAQ&usg=AOvVaw2cF-n6B7pCJTKbrOT3UxnW&cshid=1619164679797

[TildaDares]

@patricoferris I tried this and it worked.

(try
      ignore(Toploop.execute_phrase true Format.str_formatter phrase);
      with
      | exn -> failwith "This program failed with errors"
      );

This is what it prints out

This is the full output

Would you like me to make a PR?