Embed errors in the AST instead of raising

[panglesd]

Currently, when ppx_mysql encounters an error, it uses the raise_errorf function to raise a located error.

The exception is caught by ppxlib, which in this case:

• Catch the error,
• stops the rewriting process
• add the error (as a [%%%ocaml.error ...] extension node) to the last valid ast
• Use the resulting AST

The interruption of the rewriting is quite bad for the user experience! The implication for the users are:

• Since ppx_mysql runs at the "context-free" phase, the "last valid AST" is before the context-free phase. So, no other derivers/extenders get run, which generates a lot of noise in the errors (such as "uninterpreted extensions" or "unbound identifiers")
• Only one (meaningful) error from your PPX is reported at a time.

Example

For instance:

let invalid1 = [%mysql invalid ""]

let invalid2 = [%mysql invalid ""]

let valid = [%mysql select_one ""]

would report several errors:

• I don't understand query action 'invalid' for invalid1 (the right error)
• 'Uninterpreted extension 'mysql'forinvalid1, invalid2andvalid`.

The right error for invalid2 is not shown, and the "uninterpreted extension" errors add a lot of noise!

You can find more information about error reporting in PPXs in this section of the ppxlib manual.

❓ Would you be willing to accept contributions to this issue? I'm considering assigning its resolution as part of an outreachy internship: see more information here.

[panglesd]

Note that we decided that we will change the ppxlib behaviour regarding the handling of exceptions, to match the current use of raise_errorf in PPXs.
Catching an exception will no longer stop the rewriting process. So, the example I gave in the original issue is not relevant any more.

However, embedding errors still have advantages: It allows reporting multiple errors, while still outputting valid AST for the part that were successful. In the case of this PPX, an example where embedding errors is better could written as:

let insert_user =
  [%mysql
    execute "INSERT INTO users (id, phone) VALUES {(%invalid1{id}, %invalid2{phone})}"]

which, when raising instead of embedding errors, would not list both errors (both invalid1 and invalid2 raise "Unknown type specification). Moreover, it could still be able to generate a function of the right arity! (_ -> id:_ -> phone:_ -> _)