Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Better errors with codes and hoa explain #51

Open
Hywan opened this issue Feb 14, 2017 · 12 comments
Open

Better errors with codes and hoa explain #51

Hywan opened this issue Feb 14, 2017 · 12 comments

Comments

@Hywan
Copy link
Member

Hywan commented Feb 14, 2017

Hello fellow @hoaproject/hoackers and users!

As a direct or indirect user of Hoa, I want better errors than a simple exception backtrace. I want more context about errors.

Introduction

Rust and Elm errors are exemplaries. For instance in Rust:

error: `y` does not live long enough
  |
4 |     x = &y;
  |          - borrow occurs here
5 | }
  | ^ `y` dropped here while still borrowed
...
8 | }
  | - borrowed value needs to live until here

Or in Elm:

Cannot find variable `List.nap`.

6|    div [] (List.nap viewUser users)

`List` does not expose `nap`. Maybe you want one of the following?

    List.map
    List.any
    List.map2
    List.map3

All Rust errors are all listed online. Another interesting blog post from Elm: Compiler errors for humans.

We must provide a better mechanism to understand errors thrown by Hoa libraries. This RFC takes inspiration from the Rust and Elm languages.

Goals

Majority of the time spent on a new project consists of dealing with errors. This RFC will:

  • Improve the user experience by providing a description for each error, invalid examples, valid examples, and links to more resources,
  • Help us to visualise the amount of errors our libraries produce,
  • Help us to discuss about the meaning of errors (should we remove some of them, keep them, if yes why? Does it make sense?),
  • Being serious regarding errors,
  • Having error-based testing (™ 😉).

Context

So far, Hoa does not trigger any error. Only exceptions are thrown. Each exception is uniquely indexed per class (so 0, 1, 2, 3 for class C, 0, 1, 2, 3, 4, 5, 6 for class D etc). We might revise this indexing to have a similar mechanism to Rust's.

Index is stored in the $code attribute of the Exception class. The code is typed as mixed so we can store an integer, or a string in it.

Error format

By errors, we mean exceptions. An error code is defined by a specific format. In Rust, an error format has the following form: E[0-9]{4}. In our case, sure we will not reach the error number E9999, but we can apply a different format, like [A-Z]{3}[0-9]{3}, where the first letters represents the error prefix, i.e. the library identifier, and followed by the error index. For instance: ACL012 for Hoa\Acl, or RUL007 for Hoa\Ruler, or COM011 for Hoa\Compiler. Picking the first three letter of the library name is not enought to avoid conflicts, like Hoa\Event and Hoa\Eventsource that will collide (EVE in both case). So it can be totally arbitrary, we can take EVT for Hoa\Event and ESR for Hoa\Eventsource. This is a possibility. Or we can stick with E[0-9]{4} but we must set a lock on an error index in one library when creating a new error in another library to avoid having the same index twice (imagine I create 1 new error in Hoa\Acl and 1 new in Hoa\Bench, we need to “reserve” the new error index for one library to ensure the index is unique). Having the [A-Z]{3}[0-9]{3} format is simpler to manage for a library context.

We can use the $code attribute of the Exception class to store the [0-9]{3} part. The [A-Z]{3} might be a meta data attached to the library. Or the $code attribute can store the whole error code.

Explain

Given an error code, we can get more explanations about what leads to it, how to avoid it etc. The new hoa explain <error_code> command will show a comprehensive description about this particular error.

The description would include this:

  1. What the error means, in english, no code,
  2. An example of a bad code generating such an error (aka invalid example),
  3. An example of a good code (valid example),
  4. More links to online documentation (from hoa-project.net or php.net).

The format of the description will be markdown.

Example:

ACL012 Bad usage of the x argument of the f method

This error suggests that the value of x was not expected. f expects x to be an array but with some particular key/value pairs, bla bla.

Invalid example

For example, this following example is not invalid:

$acl = new Hoa\Acl\Acl();
$x   = …; // incorrect
$acl->f($x);

Valid example

This following example is valid:

$acl = new Hoa\Acl\Acl();
$x   = …; // incorrect
$acl->f($x);

Find more online at ….

The “Invalid example” and “Valid example” Sections must be strictly named like this. They act as “anchors” for error-based testing, see bellow. All sections can contain zero or more code blocks.

Error list

It will be easy to collect all errors, and publish them online. This will be a good resource for everyone, and it will publicly show we are serious regarding errors.

Error-based testing

Obviously, these error descriptions must be testable. We can extract the code from the examples, compile them to integration test cases, and then run them. All the example code blocks in the “Invalid example” Section are expected to fail. All the example code blocks in the “Valid example” Section are expected to succeed.

What command to run to test the errors? Maybe hoa test:run but it will need to scan, parse, compile, and execute error examples each time, a cache would be nice, not discussed here. Engineering issue.

Too much errors to manage?

If you think we have too much errors to manage, then this would be because we have too much errors at all. In this case, we must reduce the amount of errors a library can produce. If the number goes too high, it means we might be doing something wrong. This is a good probe.

Location of error descriptions

Just like we have Bin, Test, and Documentation at the root of each library, we can add Error, accessible through hoa://Library/<Name>/Error, but from my point of view, this must be considered as a documentation, so I would store all the error descriptions in Documentation/Error/, just like that:

Acl/
    Documentation/
        Error/
            Prefix.txt // contains `ACL` (the error prefix).
            ACL001.md
            ACL002.md
            ACL003.md
            …

Print exception message

When printing the exception message, instead of printing the exception index as usual, we print:

sprintf(
    '%s%03d',
    file_get_contents('hoa://Library/Acl/Documentation/Error/Prefix.txt'),
    $exception->getCode()
)

or, if we store the whole error code in Exception::$code, simply printing $exception->getCode(), not changes required.

Outro

What do you think? To be frank, majority of the time I spend on a new project is not coding but dealing with stupid errors. This mechanism will help a lot any new comers.

Thoughts?

@Hywan Hywan added this to the Roadmap for 2017 milestone Feb 14, 2017
@Hywan Hywan changed the title Better errors with code and hoa explain Better errors with codes and hoa explain Feb 15, 2017
@Pierozi
Copy link
Member

Pierozi commented Mar 6, 2017

I like the idea of have a normalized makdown template, related to error code.
But, in PHP world, we like to abuse of ClassName for Exception, instead of using detailed error code.

And i've commonly end up to use error code as severity index, like E_NOTICE, E_USER_ERROR...
or any personal constant definition for this purpose.

Can you give us a detail about C and D class and how we deal with it in PHP ?
Does that mean each throw new Exception must have code specified, or we will have ExceptionNotFound call parent exception with the not found code error spread ?

@Hywan
Copy link
Member Author

Hywan commented Mar 7, 2017

we like to abuse of ClassName for Exception, instead of using detailed error code.

It does not conflict. We can have nice exception types, and nice codes too. Exception types (sub-classes of Exception) are useful for catch block, and anything related to types, and the code is related to the information we can provide/link to this exception.

And i've commonly end up to use error code as severity index, like E_NOTICE, E_USER_ERROR...
or any personal constant definition for this purpose.

Hmm yes. I understand the need to have a severity attribute on exception, but I would not use E_* constants. And I won't never use the code attribute to store this information neither.

Can you give us a detail about C and D class and how we deal with it in PHP ?

Simply indexing exceptions like 1, 2, 4 for C, and 3, 5, 6, 7 for D for instance. Just a big counter for the whole library.

Does that mean each throw new Exception must have code specified, or we will have ExceptionNotFound call parent exception with the not found code error spread?

Good question. What happens if an exception has no code? Well, we won't tell the user to use hoa explain to find more information. But if a previous exception has a code, then we can tell the user: “There is no code attached to this exception, but this exception is due to another one, and its has a code! Maybe hoa explain <thiscode> can provide some information”. Thoughts?

@shulard
Copy link
Contributor

shulard commented Mar 7, 2017

It seems a good idea to use Exception hierarchy to retrieve the last valid code that can be explained. The source exception is often the source of the problem.

@Hywan
Copy link
Member Author

Hywan commented Mar 7, 2017

Be careful: We will not be able to use class hierarchy. If a class E extends Exception has no code, then its parents will have no code either. However, my proposal is to use exception precedence (an exception “embedding” another exceptions).

@shulard
Copy link
Contributor

shulard commented Mar 7, 2017

That's exactly what I was trying to say, precedence is the right word here (sorry for my english 😄).

@Pierozi
Copy link
Member

Pierozi commented Mar 7, 2017

Ok we are on same line, but, look at Hoa Exception today.
Gimme How many Exception have error code, versus total number of Exception.

This is why i'm asking, do we need to refactor our Exception by strictly specify a error code ?

as an example, if a method who throws an exception expects to receive an array of strings but receives a number instead, we should throw Exception($msg, $code) instead of ArrayOfStringException().

Can we share code between library ?
the array of string example is pretty common.

@shulard
Copy link
Contributor

shulard commented Mar 7, 2017

@Pierozi for me the code can still use ArrayOfStringException() but that exception must define its own error code. I think that updating the code to use a "basic" exception everywhere but with a code will be more complicated to understand.

@Hywan
Copy link
Member Author

Hywan commented Mar 8, 2017

The code is given in the exception constructor, as we always do:

throw new MyException($message, $code, $arguments, $previous);

@shulard
Copy link
Contributor

shulard commented Jul 5, 2017

Yesterday evening, we talked about this RFC during the Hoa Virtual meeting.

In Hoa, each exception already exists for a specific need and already have a solid meaning. Relying on exception types instead of creating code may be more relevant.

Now, with Kitab, we can add all the documentation above the class declaration with examples. This examples will be tested to be sure the documentation is always valid.

The hoa explain must exists to help users retrieving exception details. We can use the class documentation as description and add more details inside command output (places where the exception can be thrown, solution suggestions, link to online resources...).

@Grummfy
Copy link
Member

Grummfy commented Jul 6, 2017

If you look at angular js 1, all error come with a link inside the js console to the website of angular. And the website of angular display an explanation on this error. For me like shulard say, Hoa is very specific in his exception, so just with the exception name we could redirect to a website with explanation, overall with kitab.

@Hywan
Copy link
Member Author

Hywan commented Jul 7, 2017

@shulard @Grummfy Actually, you're right and I think hoa explain suddenly becomes useless. I quote you:

places where the exception can be thrown, solution suggestions, link to online resources…

All that stuff can be added directly into the documentation.

This is OK to cancel an RFC if we have a single tool that addresses several problems or needs. This is actually even better!

@Hywan
Copy link
Member Author

Hywan commented Jul 7, 2017

However, if people does not have Kitab installed or an Internet access, we can still have the hoa explain T command to print explanations about the exception T, i.e. to extract the documentation directly.

Bhoat pushed a commit that referenced this issue Feb 5, 2018
Attached to #51.

The PhpCsFixer\AbstractLinesBeforeNamespaceFixer::fixLinesBeforeNamespace signature as changed in PHP-CS-Fixer v2.8.1 :

* The offending commit : PHP-CS-Fixer/PHP-CS-Fixer@d3a7101#diff-41cf271d3466dbb2548c606fee86f147L30

Since Hoa rely on the host installed php-cs-fixer binary, we have an edge case to check here...
Bhoat pushed a commit that referenced this issue Feb 5, 2018
Since php-cs-fixer is run using the globally installed binary, we must handle edge case / breaking change between version.
To be able to detect those incompatibilities, we must be able to check php-cs-fixer verbose output...
See #51.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

4 participants