Skip Menu |

This queue is for tickets about the Moo CPAN distribution.

Report information
The Basics
Id: 99733
Status: rejected
Priority: 0/
Queue: Moo

People
Owner: Nobody in particular
Requestors: ZDM [...] cpan.org
Cc:
AdminCc:

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



Subject: Attribute name in the isa error message
Is it possible to include attribute name into "isa" error message? Sometime, if class has many attrs, it's not simple to know which attribute doesn't pass validation.
CC: undisclosed-recipients:;
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 00:01:25 +0200
To: bug-Moo [...] rt.cpan.org
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
"Dmytro Zagashev via RT" <bug-Moo@rt.cpan.org> writes: Show quoted text
> Is it possible to include attribute name into "isa" error message? > > Sometime, if class has many attrs, it's not simple to know which > attribute doesn't pass validation.
Moo has done this since version 1.000000 (released 2012-07-18): $ perl -Moo -E 'has foo => is => ro => isa => sub { die "bad\n" };' \ -E 'say Moo->VERSION; Class->new(foo=> 42)' 1.006001 isa check for "foo" failed: bad Which version are you using? -- "I use RMS as a guide in the same way that a boat captain would use a lighthouse. It's good to know where it is, but you generally don't want to find yourself in the same spot." - Tollef Fog Heen
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 01:54:42 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
Latest. I use global __DIE__ handler. Maybe this is the reason? On 23.10.2014 1:01, (Dagfinn Ilmari Mannsåker) via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > "Dmytro Zagashev via RT" <bug-Moo@rt.cpan.org> writes: >
>> Is it possible to include attribute name into "isa" error message? >> >> Sometime, if class has many attrs, it's not simple to know which >> attribute doesn't pass validation.
> Moo has done this since version 1.000000 (released 2012-07-18): > > $ perl -Moo -E 'has foo => is => ro => isa => sub { die "bad\n" };' \ > -E 'say Moo->VERSION; Class->new(foo=> 42)' > 1.006001 > isa check for "foo" failed: bad > > Which version are you using? >
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 02:12:52 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
It will be useful if Moo localize $SIG{__DIE__} in Method::Generate::Accessor::_wrap_attr_exception. This will guarantee, that prefix will be added to error message in any case. On 23.10.2014 1:01, (Dagfinn Ilmari Mannsåker) via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > "Dmytro Zagashev via RT" <bug-Moo@rt.cpan.org> writes: >
>> Is it possible to include attribute name into "isa" error message? >> >> Sometime, if class has many attrs, it's not simple to know which >> attribute doesn't pass validation.
> Moo has done this since version 1.000000 (released 2012-07-18): > > $ perl -Moo -E 'has foo => is => ro => isa => sub { die "bad\n" };' \ > -E 'say Moo->VERSION; Class->new(foo=> 42)' > 1.006001 > isa check for "foo" failed: bad > > Which version are you using? >
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Wed, 22 Oct 2014 17:12:53 -0700
To: "dzagashev [...] gmail.com via RT" <bug-Moo [...] rt.cpan.org>
From: Karen Etheridge <ether [...] cpan.org>
On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote: Show quoted text
> Queue: Moo > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > It will be useful if Moo localize $SIG{__DIE__} in > Method::Generate::Accessor::_wrap_attr_exception. > > This will guarantee, that prefix will be added to error message in any case.
Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify the exception before passing it upwards. If you are seeing these exceptions unmodified, that suggests your handler is not checking $^S, which is a problem in your code. See 'perldoc -f die' and 'perldoc perlvar'.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 03:53:01 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
I check $^S in my die handler, but this is not the case. The problem is that if global die handler used - code in your eval (after isa call) never will be executed and has no chance to prepend attribute information to error message. Using of: local $SIG{__DIE__} = undef; at the beginning of eval will solve the problem and don't affect the overall workflow. On 23.10.2014 3:13, Karen Etheridge via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote:
>> Queue: Moo >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >> >> It will be useful if Moo localize $SIG{__DIE__} in >> Method::Generate::Accessor::_wrap_attr_exception. >> >> This will guarantee, that prefix will be added to error message in any case.
> Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify > the exception before passing it upwards. If you are seeing these > exceptions unmodified, that suggests your handler is not checking $^S, > which is a problem in your code. See 'perldoc -f die' and 'perldoc > perlvar'. > >
CC: undisclosed-recipients:;
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 12:05:43 +0200
To: bug-Moo [...] rt.cpan.org
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
Seeing as we always rethrow the error (thus triggering any $SIG{__DIE__} handler then), this makes sense. "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: Show quoted text
> Queue: Moo > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > I check $^S in my die handler, but this is not the case. > > The problem is that if global die handler used - code in your eval > (after isa call) never will be executed and has no chance to prepend > attribute information to error message. > > Using of: > > local $SIG{__DIE__} = undef; > > at the beginning of eval will solve the problem and don't affect the > overall workflow. > > On 23.10.2014 3:13, Karen Etheridge via RT wrote:
>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >> >> On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote:
>>> Queue: Moo >>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>> >>> It will be useful if Moo localize $SIG{__DIE__} in >>> Method::Generate::Accessor::_wrap_attr_exception. >>> >>> This will guarantee, that prefix will be added to error message in any case.
>> Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify >> the exception before passing it upwards. If you are seeing these >> exceptions unmodified, that suggests your handler is not checking $^S, >> which is a problem in your code. See 'perldoc -f die' and 'perldoc >> perlvar'. >> >>
> > > >
-- "I use RMS as a guide in the same way that a boat captain would use a lighthouse. It's good to know where it is, but you generally don't want to find yourself in the same spot." - Tollef Fog Heen
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 13:26:02 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
Global handler can, for example, log error message and exit process. And in current case global handler receive die message without att into. Localizing __DIE__ is more versatile approach, and guarantee, that attr info will be added in any case. On 23.10.2014 13:06, (Dagfinn Ilmari Mannsåker) via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > Seeing as we always rethrow the error (thus triggering any $SIG{__DIE__} > handler then), this makes sense. > > "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: >
>> Queue: Moo >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >> >> I check $^S in my die handler, but this is not the case. >> >> The problem is that if global die handler used - code in your eval >> (after isa call) never will be executed and has no chance to prepend >> attribute information to error message. >> >> Using of: >> >> local $SIG{__DIE__} = undef; >> >> at the beginning of eval will solve the problem and don't affect the >> overall workflow. >> >> On 23.10.2014 3:13, Karen Etheridge via RT wrote:
>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>> >>> On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote:
>>>> Queue: Moo >>>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>> >>>> It will be useful if Moo localize $SIG{__DIE__} in >>>> Method::Generate::Accessor::_wrap_attr_exception. >>>> >>>> This will guarantee, that prefix will be added to error message in any case.
>>> Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify >>> the exception before passing it upwards. If you are seeing these >>> exceptions unmodified, that suggests your handler is not checking $^S, >>> which is a problem in your code. See 'perldoc -f die' and 'perldoc >>> perlvar'. >>> >>>
>> >> >>
CC: undisclosed-recipients:;
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 13:29:47 +0200
To: bug-Moo [...] rt.cpan.org
From: ilmari [...] ilmari.org (Dagfinn Ilmari Mannsåker)
I've implemented this in a branch: https://github.com/moose/Moo/pull/11 As you can see from the tests, the global $SIG{__DIE__} handler still gets called, but only once, with the final, wrapped exception. "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: Show quoted text
> Queue: Moo > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > Global handler can, for example, log error message and exit process. > And in current case global handler receive die message without att into. > Localizing __DIE__ is more versatile approach, and guarantee, that attr > info will be added in any case. > > On 23.10.2014 13:06, (Dagfinn Ilmari Mannsåker) via RT wrote:
>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >> >> Seeing as we always rethrow the error (thus triggering any $SIG{__DIE__} >> handler then), this makes sense. >> >> "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: >>
>>> Queue: Moo >>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>> >>> I check $^S in my die handler, but this is not the case. >>> >>> The problem is that if global die handler used - code in your eval >>> (after isa call) never will be executed and has no chance to prepend >>> attribute information to error message. >>> >>> Using of: >>> >>> local $SIG{__DIE__} = undef; >>> >>> at the beginning of eval will solve the problem and don't affect the >>> overall workflow. >>> >>> On 23.10.2014 3:13, Karen Etheridge via RT wrote:
>>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>> >>>> On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote:
>>>>> Queue: Moo >>>>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>>> >>>>> It will be useful if Moo localize $SIG{__DIE__} in >>>>> Method::Generate::Accessor::_wrap_attr_exception. >>>>> >>>>> This will guarantee, that prefix will be added to error message in any case.
>>>> Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify >>>> the exception before passing it upwards. If you are seeing these >>>> exceptions unmodified, that suggests your handler is not checking $^S, >>>> which is a problem in your code. See 'perldoc -f die' and 'perldoc >>>> perlvar'. >>>> >>>>
>>> >>> >>>
> > > >
-- "The surreality of the universe tends towards a maximum" -- Skud's Law "Never formulate a law or axiom that you're not prepared to live with the consequences of." -- Skud's Meta-Law
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Thu, 23 Oct 2014 14:34:15 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
That is what we need. On 23.10.2014 14:30, (Dagfinn Ilmari Mannsåker) via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > I've implemented this in a branch: > > https://github.com/moose/Moo/pull/11 > > As you can see from the tests, the global $SIG{__DIE__} handler still > gets called, but only once, with the final, wrapped exception. > > "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: >
>> Queue: Moo >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >> >> Global handler can, for example, log error message and exit process. >> And in current case global handler receive die message without att into. >> Localizing __DIE__ is more versatile approach, and guarantee, that attr >> info will be added in any case. >> >> On 23.10.2014 13:06, (Dagfinn Ilmari Mannsåker) via RT wrote:
>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>> >>> Seeing as we always rethrow the error (thus triggering any $SIG{__DIE__} >>> handler then), this makes sense. >>> >>> "dzagashev@gmail.com via RT" <bug-Moo@rt.cpan.org> writes: >>>
>>>> Queue: Moo >>>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>> >>>> I check $^S in my die handler, but this is not the case. >>>> >>>> The problem is that if global die handler used - code in your eval >>>> (after isa call) never will be executed and has no chance to prepend >>>> attribute information to error message. >>>> >>>> Using of: >>>> >>>> local $SIG{__DIE__} = undef; >>>> >>>> at the beginning of eval will solve the problem and don't affect the >>>> overall workflow. >>>> >>>> On 23.10.2014 3:13, Karen Etheridge via RT wrote:
>>>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>>> >>>>> On Wed, Oct 22, 2014 at 07:13:12PM -0400, dzagashev@gmail.com via RT wrote:
>>>>>> Queue: Moo >>>>>> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>>>>> >>>>>> It will be useful if Moo localize $SIG{__DIE__} in >>>>>> Method::Generate::Accessor::_wrap_attr_exception. >>>>>> >>>>>> This will guarantee, that prefix will be added to error message in any case.
>>>>> Moo doesn't use a __DIE__ handler -- it uses an eval to catch and modify >>>>> the exception before passing it upwards. If you are seeing these >>>>> exceptions unmodified, that suggests your handler is not checking $^S, >>>>> which is a problem in your code. See 'perldoc -f die' and 'perldoc >>>>> perlvar'. >>>>> >>>>>
>>>> >>>>
>> >> >>
There are other uses of __DIE__ handlers that would be harmed by this change. Having a __DIE__ handler that exits on any exception is pretty broken. I don't see a good reason Moo should change its behavior here for a particular type of __DIE__ handler.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Fri, 24 Oct 2014 19:06:39 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
This is more logical, that if Moo want to modify error message - he should perform this modifications before message will be passed to any other handlers. On 24.10.2014 18:42, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > There are other uses of __DIE__ handlers that would be harmed by this change. Having a __DIE__ handler that exits on any exception is pretty broken. I don't see a good reason Moo should change its behavior here for a particular type of __DIE__ handler.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Fri, 24 Oct 2014 19:17:33 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
Usually __DIE__ handler used specifically for errors messages processing, and it should retrieve fully qualified message. On 24.10.2014 18:42, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > There are other uses of __DIE__ handlers that would be harmed by this change. Having a __DIE__ handler that exits on any exception is pretty broken. I don't see a good reason Moo should change its behavior here for a particular type of __DIE__ handler.
Moo doesn't generate the error messages. __DIE__ handlers always have to account for things like re-throwing or errors being caught.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Sat, 25 Oct 2014 01:31:48 +0300
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
I'm not saying that Moo generates errors messages. __DIE__ handler handles errors and expects, that errors messages are already fully qualified. Or we should remember, that Moo can throw error twice, and for the first time we should ignore this? What the problem with local $SIG{__DIE__} in your case? Can you describe it more specifically? On 24.10.2014 22:42, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > Moo doesn't generate the error messages. > > __DIE__ handlers always have to account for things like re-throwing or errors being caught.
On Fri Oct 24 18:32:11 2014, dzagashev@gmail.com wrote: Show quoted text
> I'm not saying that Moo generates errors messages. > > __DIE__ handler handles errors and expects, that errors messages are > already fully qualified. > > Or we should remember, that Moo can throw error twice, and for the > first > time we should ignore this? > > What the problem with local $SIG{__DIE__} in your case? > > Can you describe it more specifically? > > On 24.10.2014 22:42, Graham Knop via RT wrote:
> > <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > > > Moo doesn't generate the error messages. > > > > __DIE__ handlers always have to account for things like re-throwing > > or errors being caught.
I have code that attaches stack traces to all exceptions when possible. The only thing that really works for this is a __DIE__ hook. And it is much less useful if it doesn't have access to the stack at the original location the error was triggered.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Tue, 28 Oct 2014 17:09:33 +0200
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
My handler doing exactly the same. We can call global handler manually and pass modified message, call stack will be preserved: eval { my $orig_die = $SIG{__DIE__}; local $SIG{__DIE__} = sub { $orig_die->( 'ATTRIBUTE_NAME_INFO' . $_[0] ) if $orig_die; }; ...; }; On 28.10.2014 6:48, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > On Fri Oct 24 18:32:11 2014, dzagashev@gmail.com wrote:
>> I'm not saying that Moo generates errors messages. >> >> __DIE__ handler handles errors and expects, that errors messages are >> already fully qualified. >> >> Or we should remember, that Moo can throw error twice, and for the >> first >> time we should ignore this? >> >> What the problem with local $SIG{__DIE__} in your case? >> >> Can you describe it more specifically? >> >> On 24.10.2014 22:42, Graham Knop via RT wrote:
>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > >>> >>> Moo doesn't generate the error messages. >>> >>> __DIE__ handlers always have to account for things like re-throwing >>> or errors being caught.
> I have code that attaches stack traces to all exceptions when possible. The only thing that really works for this is a __DIE__ hook. And it is much less useful if it doesn't have access to the stack at the original location the error was triggered.
Not all exceptions that occur within an isa or coerce are exceptions that will be thrown outward. They could be used for many different purposes. There was already a previous bug report because we had been overriding __DIE__ and it was triggering in places it shouldn't have been.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Tue, 28 Oct 2014 18:09:15 +0200
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
Code, that I promote, simply duplicate standard perl functionality. If there is parent die handler - it always will be called by perl before exiting eval block. Or it can be called manually from localized handler. Conditions are the same. I can't see any difference, which can affect workflow. On 28.10.2014 17:54, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > Not all exceptions that occur within an isa or coerce are exceptions that will be thrown outward. They could be used for many different purposes. > > There was already a previous bug report because we had been overriding __DIE__ and it was triggering in places it shouldn't have been.
Code inside the isa check may use exceptions for things other than to indicate an isa check failure. Adding information about the attribute to an exception like this is an error. The only exception that should have the attribute information added is one that reaches the isa check itself. This can't be done with a __DIE__ handler.
Subject: Re: [rt.cpan.org #99733] Attribute name in the isa error message
Date: Tue, 28 Oct 2014 19:19:39 +0200
To: bug-Moo [...] rt.cpan.org
From: "dzagashev [...] gmail.com" <dzagashev [...] gmail.com>
So, there is no working solution? On 28.10.2014 18:31, Graham Knop via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=99733 > > > Code inside the isa check may use exceptions for things other than to indicate an isa check failure. Adding information about the attribute to an exception like this is an error. The only exception that should have the attribute information added is one that reaches the isa check itself. This can't be done with a __DIE__ handler.
I don't see any full solution for this. __DIE__ handlers by nature have lots of pitfalls and we won't be able to satisfy all use cases. Trying to wrap or bypass existing __DIE__ handlers in this case has more issues than I am comfortable with.