[Dancer-users] User-friendly errors on YAML parsing failures (was: Re: dancer croaking with a strange error in config.yml)

[damien krotkine]

On 14 November 2011 17:11, David Precious <davidp at preshweb.co.uk> wrote:
> On Monday 14 November 2011 15:33:19 Mr. Puneet Kishor wrote:
>> Solved... (it was an invisible character, but see below) --
> [...]
>> Opened the file again in vi and `:set list`, and bingo! there was a
>> character that looked like ^I, a result of pressing 'tab'
>>
>> Seems like YAML doesn't like tabs. I deleted that character, and now
>> everything is well.
>
> Ah, good stuff, glad to hear you got it sorted.
>
> I'm beginning to wonder if we should add a little magic to Dancer to catch
> YAML parsing errors, and, at the very least, output a message making it
> clearer that the problem is not with Dancer but that YAML.pm couldn't parse
> the YAML and provide a link to the YAML spec, and perhaps also, look for any
> tabs in the file (before any non-whitespace chars) and, if found, output a "It
> looks like you've got tabs in the file; don't do that" message.
>
> I agree to some degree with previous messages on the subject of YAML parsing
> failures that it's not Dancer's problem, but if we can make things easier for
> the user at little cost, I think we should.
>
> So, what I propose is, if the parsing fails, log a message something like:
>
>  "It wasn't possible to parse your config.yml file.  This file needs to be
> valid YAML in order to be parsed by YAML.pm - please see the YAML
> specifications for details.  In particular, YAML is sensitive to indentation -
> use a consistent number of spaces, do not use tabs."
>
> Also, if the parsing failed, open config.yml and skim over it, and report any
> lines which included tabs, with the line number.

I agree that something should be done here regarding config parsing :
we try to be explicit about failures and issues in Dancer, but we fail
at doing that when there is an issue in the configuration file.

I don't think there is a need to skim the file for tabs or space
though. I would tend to more lazyness :

try to parse the config file. If it fails, try to parse the error
message sent by YAML, and extract the line / column from the message.
It's usually reported accurately. Then 2 cases : if Carp::Verbose is
false (general case), perform a croak (not a confess), with a human
message saying " failed to parse config file '$filename', at line 42,
column 12. Take extra care to spaces and tabs, blabla".
If Carp::Verbose is true, then do a confess instead, appending the
full YAML message.

Or something like that. I don't think it's good practice to reparse
the file, as we want to rely on YAML to perform checking and raise
errors.

>
> This would be a fairly small amount of code, and would only come into play if
> the parsing of config.yml failed (in which case, the app is unlikely to start
> anyway, so it's not like it's extra complexity / expense in the majority of
> cases).
>
>
> If other devs agree that makes sense, I'll rock up something like this,
> initially for Dancer then see about porting it forwards to Dancer2.

I agree :)