Improvements to the way we currently handle internal Reek errors in detectors

[troessner]

Related to #1069:

996 will help a lot making Reek more resilient but as already outlined in #1069 we kind of forgot that the majority of all errors the user makes in either the configuration or the source code comments that would have caused Reek to crash before will now result in this:

      !!!
      Source lib/reek/ast/node.rb can not be processed by Reek.
      This is most likely a Reek bug.
      It would be great if you could report this back to the Reek team
      by opening up a corresponding issue at https://github.com/troessner/reek/issues
      Make sure to include the source in question, the Reek version
      and the original exception below.

      Original exception:
      #<Psych::SyntaxError: (<unknown>): did not find expected ',' or '}' while parsing a flow mapping at line 1 column 22>
      !!!

Which is highly confusing for the user and might lead to a lot of false-positives issues in the long run (like: "Reek told me to report this here, what now?").

Of course the best solution to this problem would be validate all configuration before running (or in case of comments while Reek is running and then print sth on STDERR or similar) which is something we are actively working on. But this might take a while since the problem is not trivial.

So I was wondering what we could do in the meantime and I have 2 suggestions:

1.) Improve the error message above

Instead of

      This is most likely a Reek bug.
      It would be great if you could report this back to the Reek team
      by opening up a corresponding issue at https://github.com/troessner/reek/issues
      Make sure to include the source in question, the Reek version
      and the original exception below.

I'd suggest to print:

This is most likely either a bug in your Reek configuration (config file or 
source code comments) or a Reek bug.

Please double check your Reek configuration taking the original exception
below into account -  you might have mispelled a smell detector for instance.
(In the future Reek will handle configuration errors more gracefully, something
we are working on already).

If you feel that this is not a problem with your Reek configuration but with
Reek itself it would be great if you could report this back to the Reek
team by opening up a corresponding issue at https://github.com/troessner/reek/issues

Make sure to include the source in question, the Reek version
and the original exception below.

2.) Fix the printing of the original exception

That was another thing we missed in our review :) As you can see above, we're only printing out the very last line. This should be a full stacktrace to make it easier on the user and on us.

WDYT?

[mvz]

Point 2 is definitely needed.

I think we should and can differentiate the message in our error handler to show known user errors differently. So instead of one message with a 'could be this, could be that' content, we show a more specific message if we know it's due to the configuration.

[troessner]

I think we should and can differentiate the message in our error handler to show known user errors differently.

I think this would be very hacky and brittle at best at the moment because we don't know what we, uhm, don't know :)

So instead of one message with a 'could be this, could be that' content, we show a more specific message if we know it's due to the configuration.

While I agree in theory, it might be that in practice this is not necessary anymore. We're working on validating the full configuration right now and on validating comments at runtime. As soon as this is done we don't need to differentiate at all because for every crash from then on can assume that's it's bad source / Reek-internal bug.