Bug #91199 for App-CSV: [PATCH] Better error message when a named field isn't found

Thu Dec 05 14:19:14 2013 tsibley [...] cpan.org - Ticket created

Subject:

Better error message when a named field isn't found

I typo'd a field name and got this Moose validation error: Use of uninitialized value $in in numeric le (<=) at /Users/trsibley/.perlbrew/libs/5.18@hackery/lib/perl5/App/CSV.pm line 116, <$fh> line 1. Attribute (columns) does not pass the type constraint because: Validation failed for 'ArrayRef[Int]' with value ARRAY(0x7fe50cb00ae8) at accessor App::CSV::columns (defined at /Users/trsibley/.perlbrew/libs/5.18@hackery/lib/perl5/App/CSV.pm line 26) line 4, <$fh> line 1. App::CSV::columns('App::CSV=HASH(0x7fe50caffd80)', 'ARRAY(0x7fe50cb00ae8)') called at /Users/trsibley/.perlbrew/libs/5.18@hackery/lib/perl5/App/CSV.pm line 209 App::CSV::init('App::CSV=HASH(0x7fe50caffd80)') called at /Users/trsibley/.perlbrew/libs/5.18@hackery/lib/perl5/App/CSV.pm line 244 App::CSV::run('App::CSV=HASH(0x7fe50caffd80)') called at /Users/trsibley/.perlbrew/libs/5.18@hackery/bin/csv line 11 It's rather unfriendly and useless. I took a look at the source and saw that because I specified an unknown field name, it was passed through as undef instead of being converted to a column number. How about this patch? --- a/App/CSV.pm 2013-12-05 11:18:05.000000000 -0800 +++ b/App/CSV.pm 2013-12-05 11:17:51.000000000 -0800 @@ -139,6 +139,10 @@ # if there is at least one named field, we need to read the header line from input and translate into column number if (@named_fields) { my $header_map = $self->_get_header_map; + if (my @missing_fields = grep { not defined $header_map->{$_} } @named_fields) { + die "The following named fields aren't in the input header: ", + join(", ", @missing_fields), "\n"; + } @normalized_fields = map { __normalize_column(/^\d+/ ? $_ : $header_map->{$_} ) } @all_fields; } else {

Thu Dec 05 14:24:25 2013 GAAL [...] cpan.org - Correspondence added

On Thu Dec 05 14:19:14 2013, TSIBLEY wrote: Show quoted text

> I typo'd a field name and got this Moose validation error: >

[...] Show quoted text

> It's rather unfriendly and useless. I took a look at the source and > saw that because I specified an unknown field name, it was passed > through as undef instead of being converted to a column number. > > How about this patch?

Great idea! Mind including a test? Better yet please send me a pull request on github? I will get to this sooner or later even if you don't, but my primary dev env is a bit borked.

Thu Dec 05 14:24:25 2013 The RT System itself - Status changed from 'new' to 'open'

Thu Dec 05 14:46:46 2013 tsibley [...] cpan.org - Correspondence added

An updated patch, for even better i/o errors.

Subject:

error-reporting.patch

--- a/App/CSV.pm 2013-12-05 11:18:05.000000000 -0800 +++ b/App/CSV.pm 2013-12-05 11:45:18.000000000 -0800 @@ -139,6 +139,10 @@ # if there is at least one named field, we need to read the header line from input and translate into column number if (@named_fields) { my $header_map = $self->_get_header_map; + if (my @missing_fields = grep { not defined $header_map->{$_} } @named_fields) { + die "The following named fields aren't in the input header: ", + join(", ", @missing_fields), "\n"; + } @normalized_fields = map { __normalize_column(/^\d+/ ? $_ : $header_map->{$_} ) } @all_fields; } else { @@ -252,7 +256,7 @@ } if (!$self->_output_csv->print($self->_output_fh, $data)) { - warn $self->_output_csv->error_diag; + warn $self->format_error("Warning - Output error", diag => [$self->_input_csv->error_diag]), "\n"; next INPUT; } $self->_output_fh->print("\n"); @@ -262,11 +266,28 @@ # TODO: strict errors, according to command line, blah if (not defined $data) { last INPUT if $self->_input_csv->eof; - warn $self->_input_csv->error_diag; + warn $self->format_error("Warning - Input error", line => $., diag => [$self->_input_csv->error_diag]), "\n"; } } } } +sub format_error { + my $self = shift; + my $msg = shift; + my %args = @_; + + $msg .= ", line $args{line}" + if defined $args{line}; + + if ($args{diag}) { + my ($code, $err, $pos, $record) = @{ $args{diag} || [] }; + $msg .= ": " . join " - ", + $code, $err, "position $pos", + (defined $record ? "record $record" : ()); + } + return $msg; +} + 1; __END__

Thu Dec 05 14:46:59 2013 tsibley [...] cpan.org - Subject changed from 'Better error message when a named field isn't found' to '[PATCH] Better error message when a named field isn't found'

Fri Dec 06 16:55:24 2013 tom [...] zulutango.org - Correspondence added

Subject:	Re: [rt.cpan.org #91199] Better error message when a named field isn't found
Date:	Fri, 06 Dec 2013 16:54:51 -0500
To:	GAAL via RT <bug-App-CSV [...] rt.cpan.org>
From:	Thomas Sibley <tom [...] zulutango.org>

On Thu, Dec 05, 2013 at 02:24:25PM -0500, GAAL via RT wrote: Show quoted text

> Great idea! Mind including a test? Better yet please send me a pull > request on github? I will get to this sooner or later even if you > don't, but my primary dev env is a bit borked.

https://github.com/gaal/app-csv/pull/2 I also added a patch to Makefile.PL to report the repository so MetaCPAN will link to it.

Sat Dec 14 10:32:52 2013 GAAL [...] cpan.org - Correspondence added

On Fri Dec 06 16:55:24 2013, tom@zulutango.org wrote: Show quoted text

> On Thu, Dec 05, 2013 at 02:24:25PM -0500, GAAL via RT wrote:

> > Great idea! Mind including a test? Better yet please send me a pull > > request on github? I will get to this sooner or later even if you > > don't, but my primary dev env is a bit borked.

> > https://github.com/gaal/app-csv/pull/2 > > I also added a patch to Makefile.PL to report the repository so MetaCPAN > will link to it.

I've merged your patch and pushed 0.07 to PAUSE, which should index it soon. Thanks!

Sat Dec 14 10:32:54 2013 GAAL [...] cpan.org - Status changed from 'open' to 'resolved'

Sat Dec 14 10:32:54 2013 GAAL [...] cpan.org - Taken