Skip Menu |

This queue is for tickets about the Type-Tiny CPAN distribution.

Report information
The Basics
Id: 109940
Status: rejected
Priority: 0/
Queue: Type-Tiny

People
Owner: perl [...] toby.ink
Requestors: NERDVANA [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 1.000005
Fixed in: (no value)



Subject: Make Type::Tiny assertions compatible with Carp::Always and/or Carp::Verbose
Currently, the ->assert_valid method can not be overridden with Carp::Always and ignores Carp::Verbose, making it difficult to get a stack trace. Most Moo classes report the error coming from "eval nnn line nnn" which is not helpful. I have not looked into the code, but it should just be a matter of calling 'carp' to generate the error, or doing less trickery with die.
Could you be more specific? Provide example code, show current behaviour, and desired behaviour? Note that Type::Tiny throws exception objects rather than just dying with a string. If you've got Devel::StackTrace installed, these will include a stack trace.
On Tue Jan 31 22:26:10 2017, TOBYINK wrote: Show quoted text
> Could you be more specific? Provide example code, show current > behaviour, and desired behaviour? > > Note that Type::Tiny throws exception objects rather than just dying > with a string. If you've got Devel::StackTrace installed, these will > include a stack trace.
Ok, consider this example: package Foo { use Moo; use Types::Standard ':all'; has attr1 => is => 'rw', isa => Int; }; Foo->new(attr1 => "String); When you run that, the output is currently: Value "String" did not pass type constraint "Int" (in $args->{"attr1"}) at (eval 76) line 51 "Int" is a subtype of "Num" "Num" is a subtype of "LaxNum" Value "String" did not pass type constraint "LaxNum" (in $args->{"attr1"}) "LaxNum" is defined as: (defined($_) && !ref($_) && Scalar::Util::looks_like_number($_)) I expect it to say "at foo.pl line 6" instead of "at (eval 76) line 51". I did just install Devel::StackTrace and the output was unchanged. While carp is a primitive tool, it is a tool that works, and is pretty standard for writing modules. It would be nice if the exception object you are using could match carp behavior.
Limitation of Moo? package Foo { use Moo; use Sub::Quote; has attr1 => is => 'rw', isa => quote_sub q{ die "blah" }; }; Foo->new(attr1 => "String"); Moose has a slightly nicer error: package Foo { use Moose; use Types::Standard qw/ Int /; has attr1 => is => 'rw', isa => Int; }; Foo->new(attr1 => "String");
Also, example with stack traces: package Foo { use Moo; use Types::Standard ':all'; has attr1 => is => 'rw', isa => Int; }; use Try::Tiny; try { local $Error::TypeTiny::StackTrace = 1; Foo->new(attr1 => "String"); } catch { my $e = shift; print $e->stack_trace->as_string; };
Well, so I dug into the code :-) It seems that the reason Moose gets the right line number is that Type::Tiny has specific support for it, rather than the other way around: https://github.com/tobyink/p5-type-tiny/blob/master/lib/Error/TypeTiny.pm#L58 Instead of also adding support for Moo, maybe you could skip any stack frame which comes from a file matching /^\(eval/ ? It would also be nice to match Carp's behavior by additionally skipping any package listed in "@{"${PKG}::CARP_NOT"}". And maybe the stack trace could get collected if $Carp::Verbose is a true value. and then printed by to_string on the same condition. I wasn't going to recommend this at first because I've previously had problems with Devel::StackTrace affecting reference counts, but I just read the changelog and it looks like he fixed the newer versions so they stringify all the refs, so that shouldn't be a problem anymore.
On Wed Feb 01 18:59:07 2017, NERDVANA wrote: Show quoted text
> It would also be nice to match Carp's behavior by additionally > skipping any package listed in "@{"${PKG}::CARP_NOT"}".
Actually Carp has a function "short_error_loc" that returns the index of the stack frame to start on. How do you feel about just calling that method to get full Carp behavior?
Sub::Quote doesn't include #line directives to try to include correct file/line number information, because when inlining code it will combine code together from multiple sources, some of which won't include their own line directives. Showing an '(eval ...)' location at least shows that the location is unknown rather than giving a confusing incorrect location. Moo does set @CARP_NOT pretty comprehensively now, so if you use croak or short_error_loc, it should find the correct location.
I don't have any desire to dig into Carp's internals any more than I already have, but would happily accept a patch that demonstrably improves error reporting, provided it includes decent test cases.
Carp::Always skips over blessed exceptions, and I'm not likely to stop using blessed exceptions. Just use Devel::Confess instead.