Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Debian dpkg (http://www.linux-archive.org/debian-dpkg/)
-   -   Don't obscure errors with custom errors only (http://www.linux-archive.org/debian-dpkg/646331-dont-obscure-errors-custom-errors-only.html)

Thomas Adam 03-19-2012 10:28 AM

Don't obscure errors with custom errors only
 
When coming out of eval blocks and reporting on errors, make sure $@ is
included as part of the textual output so that the real underlying error is
reported.
---
I was recently bitten by this:

dpkg-source: error: source package format `3.0 (native)' is not supported
(Perl module Dpkg::Source::Package::V3::native is required)

Of course, that file is being required just fine, the problem turned out
to be missing File::Temp, but without including $@, I would never have
know this, due to a custom error message completely hiding this detail.

scripts/Dpkg/Source/Package.pm | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/scripts/Dpkg/Source/Package.pm b/scripts/Dpkg/Source/Package.pm
index 693eda5..92905ea 100644
--- a/scripts/Dpkg/Source/Package.pm
+++ b/scripts/Dpkg/Source/Package.pm
@@ -233,7 +233,8 @@ sub upgrade_object_type {
$self->{'fields'}{'Format'} .= " ($variant)" if defined $variant;
}
if ($@) {
- error(_g("source package format `%s' is not supported (Perl module %s is required)"), $format, $module);
+ error(_g("source package format `%s' is not supported (Perl module %s is required): %s"),
+ $format, $module, $@);
}
bless $self, $module;
} else {
--
1.7.9.1


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20120319112826.GA6600@debian.dev.smoothwall.net">h ttp://lists.debian.org/20120319112826.GA6600@debian.dev.smoothwall.net

Jonathan Nieder 03-19-2012 02:28 PM

Don't obscure errors with custom errors only
 
Hi Thomas,

Thomas Adam wrote:

> When coming out of eval blocks and reporting on errors, make sure $@ is
> included as part of the textual output so that the real underlying error is
> reported.
> ---
> I was recently bitten by this:
>
> dpkg-source: error: source package format `3.0 (native)' is not supported
> (Perl module Dpkg::Source::Package::V3::native is required)
>
> Of course, that file is being required just fine, the problem turned out
> to be missing File::Temp, but without including $@, I would never have
> know this, due to a custom error message completely hiding this detail.

Small nit: in this example, I think the text after the three dashes
makes a much better commit message than the text before them.

[...]
> --- a/scripts/Dpkg/Source/Package.pm
> +++ b/scripts/Dpkg/Source/Package.pm
> @@ -233,7 +233,8 @@ sub upgrade_object_type {
> $self->{'fields'}{'Format'} .= " ($variant)" if defined $variant;
> }
> if ($@) {
> - error(_g("source package format `%s' is not supported (Perl module %s is required)"), $format, $module);
> + error(_g("source package format `%s' is not supported (Perl module %s is required): %s"),
> + $format, $module, $@);

If the Dpkg::Source::Package::V3::foo module is missing, will $@
mention so? I am asking because I wonder if

_g("source package format '%s' is not supported: %s')

might not be a shorter way to convey the same information.

Thanks,
Jonathan


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20120319152835.GD11180@burratino">http://lists.debian.org/20120319152835.GD11180@burratino

Thomas Adam 03-19-2012 02:33 PM

Don't obscure errors with custom errors only
 
Hi,

On 19 March 2012 15:28, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi Thomas,
>
> Thomas Adam wrote:
>
>> When coming out of eval blocks and reporting on errors, make sure $@ is
>> included as part of the textual output so that the real underlying error is
>> reported.
>> ---
>> *I was recently bitten by this:
>>
>> *dpkg-source: error: source package format `3.0 (native)' is not supported
>> *(Perl module Dpkg::Source::Package::V3::native is required)
>>
>> *Of course, that file is being required just fine, the problem turned out
>> *to be missing File::Temp, but without including $@, I would never have
>> *know this, due to a custom error message completely hiding this detail.
>
> Small nit: in this example, I think the text after the three dashes
> makes a much better commit message than the text before them.

I can re-roll if you prefer?

> [...]
>> --- a/scripts/Dpkg/Source/Package.pm
>> +++ b/scripts/Dpkg/Source/Package.pm
>> @@ -233,7 +233,8 @@ sub upgrade_object_type {
>> * * * * * * *$self->{'fields'}{'Format'} .= " ($variant)" if defined $variant;
>> * * * * *}
>> * * * * *if ($@) {
>> - * * * * error(_g("source package format `%s' is not supported (Perl module %s is required)"), $format, $module);
>> + * * * * error(_g("source package format `%s' is not supported (Perl module %s is required): %s"),
>> + * * * * * * * * * * $format, $module, $@);
>
> If the Dpkg::Source::Package::V3::foo module is missing, will $@
> mention so? *I am asking because I wonder if
>
> * * * *_g("source package format '%s' is not supported: %s')
>
> might not be a shorter way to convey the same information.

It should do, but it depends entirely on the intent of the original
error message, which is why I left that intact and merely appended the
extra content from $@.

Regards,

--
Thomas Adam
Senior Developer

thomas.adam@smoothwall.net

Smoothwall Ltd
1 John Charles Way, Leeds, LS12 6QA United Kingdom
Telephone: *USA: 1 800 959 3760 *Europe: +44 (0) 8701 999500
www.smoothwall.net

Smoothwall Limited is registered in England, Company Number: 4298247. *This
email and any attachments transmitted with it are confidential to the
intended recipient(s) and may not be communicated to any other person or
published by any means without the permission of Smoothwall Limited. *Any
opinions stated in this message are solely those of the author.


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: CAOyM7r37-YqgPU+_JboJOSBdYpT37huKqgbVqaXoP=GCNpS8Ow@mail.gma il.com">http://lists.debian.org/CAOyM7r37-YqgPU+_JboJOSBdYpT37huKqgbVqaXoP=GCNpS8Ow@mail.gma il.com

Guillem Jover 05-05-2012 03:16 AM

Don't obscure errors with custom errors only
 
Hi Thomas!

On Mon, 2012-03-19 at 15:33:28 +0000, Thomas Adam wrote:
> On 19 March 2012 15:28, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Thomas Adam wrote:
> >> When coming out of eval blocks and reporting on errors, make sure $@ is
> >> included as part of the textual output so that the real underlying error is
> >> reported.
> >> ---
> >> *I was recently bitten by this:
> >>
> >> *dpkg-source: error: source package format `3.0 (native)' is not supported
> >> *(Perl module Dpkg::Source::Package::V3::native is required)
> >>
> >> *Of course, that file is being required just fine, the problem turned out
> >> *to be missing File::Temp, but without including $@, I would never have
> >> *know this, due to a custom error message completely hiding this detail.
> >
> > Small nit: in this example, I think the text after the three dashes
> > makes a much better commit message than the text before them.
>
> I can re-roll if you prefer?

Given the note below, that'd be perfect, yes.

> > [...]
> >> --- a/scripts/Dpkg/Source/Package.pm
> >> +++ b/scripts/Dpkg/Source/Package.pm
> >> @@ -233,7 +233,8 @@ sub upgrade_object_type {
> >> * * * * * * *$self->{'fields'}{'Format'} .= " ($variant)" if defined $variant;
> >> * * * * *}
> >> * * * * *if ($@) {
> >> - * * * * error(_g("source package format `%s' is not supported (Perl module %s is required)"), $format, $module);
> >> + * * * * error(_g("source package format `%s' is not supported (Perl module %s is required): %s"),
> >> + * * * * * * * * * * $format, $module, $@);
> >
> > If the Dpkg::Source::Package::V3::foo module is missing, will $@
> > mention so? *I am asking because I wonder if
> >
> > * * * *_g("source package format '%s' is not supported: %s')
> >
> > might not be a shorter way to convey the same information.
>
> It should do, but it depends entirely on the intent of the original
> error message, which is why I left that intact and merely appended the
> extra content from $@.

Although the current message might seem more clear, if it ends up giving
confusing information it does not serve its purpose well, so could you
rework it with the one suggested by Jonathan (notice the use of single
quotes, though), please?

thanks,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20120505031639.GB11317@gaara.hadrons.org">http://lists.debian.org/20120505031639.GB11317@gaara.hadrons.org

Guillem Jover 05-18-2012 02:35 AM

Don't obscure errors with custom errors only
 
On Sat, 2012-05-05 at 05:16:39 +0200, Guillem Jover wrote:
> On Mon, 2012-03-19 at 15:33:28 +0000, Thomas Adam wrote:
> > I can re-roll if you prefer?

Actually, on second thought, being this a one-liner it's easier for me
to fix it up, which I've done locally, and will be included on my next
push.

thanks,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20120518023528.GA20964@gaara.hadrons.org">http://lists.debian.org/20120518023528.GA20964@gaara.hadrons.org


All times are GMT. The time now is 04:10 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.