Skip Menu |
 

This queue is for tickets about the Imager CPAN distribution.

Report information
The Basics
Id: 24193
Status: open
Priority: 80/
Queue: Imager

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

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



Subject: add an option to make errors fatal
Download (untitled) / with headers
text/plain 520b
add some mechanism to make errors fatal, either within a given lexical scope or a package Under perl 5.10 this could use the "user pragma" support described in perlpragma. Under older versions we could caller to get line numbers, though I'm not sure that that would work with lexical scopes. usage: use Imager '+fatal'; # the following would die my $font = Imager::Font->new(file=>'doesnotexist.ttf'); perhaps have it include context information: # would die with a backtrace use Imager '+fatalcontext';
Subject: discussing throwing exceptions and identifying corrupt image errors
When Imager throws an exception in my apps, I'm primarily interested to know one thing: Does it represent a coding bug, or bad data, in the form of a corrupt image that got passed in? To address this, I wrote a little routine that checks 'errstr', to match patterns that I believe are caused by corrupt images and are "not my fault". Based on the result, I might tell the user "try again" or apologize and generate logging that will notify developers. This seems like a common enough situation that it seems worth baking into Imager. My initial idea is to consider upgrading 'errstr' to an exception object that uses overloading to stringify to an string as it does not. Imager would classify exceptions "corrupt image" exceptions or general exceptions as they happen. A second idea would be to add a new method: Imager->throw; $imager->throw; Or more explicitly: Imager->throw_exception; $imager->throw_exception; That would insure complete backcompat regarding 'errstr', and might also be part of the solution to "make errors fatal", perhaps by pairing it with an "auto_throw" option: Imager->new( auto_throw_exceptions => 1 ); Someone who wanted this behavior throughout a large project could sub-class Imager and set "auto_throw_exceptions" as a default. (Like the original idea, I would suggest that "throw" object of type Imager::Exception or Imager::Exception::Corrupt based on the type of error). What do you think? Below the error checking I used on another project. =head2 check_imager_errstr check_imager_errstr(Imager->errstr); check_imager_errstr($imager->errstr); If an input is defined, assume it's an Imager exception. Throw 'Image_Corrupt' or a general exception depending on the content. =cut sub check_imager_errors { my $error = shift; if ($error) { if ($error =~ m/Unsupported marker type|not possible to guess|offset too small|Invalid JPEG file str ucture/) { Image_Corrupt->throw(error=>$_); } else { croak "Imager failure: ".$error } } else { # no error, no action. } }
Subject: attached: exception handling proposal
Download (untitled) / with headers
text/plain 141b
Attached is a draft of Imager::Exception which implements the ideas discussed above as a plugin. I'm interested in your feedback. Thanks!
Subject: Exception.pm
Download Exception.pm
text/x-perl 1.2k
=head2 SYNPOSIS use Imager; use Imager::Exception; my $imager = Imager->new( file => $tmp_filename ) or Imager->throw_exception; # ... $imager->some_call or $imager->throw_exception. =head2 throw_exception Imager->throw_exception; $imager->throw_exception; Intended to be used when an imager call fails. Throws L<Imager::Exception::Corrupt> if the failure appears to be a corrupt image. Throws L<Imager::Exception::Base> in all other cases. A default error message is provided if errstr is empty. =cut package Imager::Exception; use Exception::Class ( 'Imager::Exception::Base', 'Imager::Exception::Corrupt' => { isa => 'Imager::Exception::Base' }, ); sub Imager::throw_exception { my $class_or_object = shift; my $error = $class_or_object->errstr; if (defined $error) { if ($error =~ m/Unsupported marker type|not possible to guess|offset too small|Invalid JPEG file str ucture/) { Imager::Exception::Corrupt->throw(error => $error); } else { Imager::Exception::Base->throw(error => $error); } } else { Imager::Exception::Base->throw(error => 'throw_exception was called, but errstr was not defined'); } } 1;
Subject: Re: [rt.cpan.org #24193] discussing throwing exceptions and identifying corrupt image errors
Date: Thu, 24 Jan 2013 11:52:56 +1100
To: MARKSTOS via RT <bug-Imager [...] rt.cpan.org>
From: Tony Cook <tony [...] develop-help.com>
Download (untitled) / with headers
text/plain 2.9k
On Wed, Jan 23, 2013 at 10:59:19AM -0500, MARKSTOS via RT wrote: Show quoted text
> Queue: Imager > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=24193 > > > When Imager throws an exception in my apps, I'm primarily interested to > know one thing: Does it represent a coding bug, or bad data, in the form of > a corrupt image that got passed in? > > To address this, I wrote a little routine that checks 'errstr', to match > patterns that I believe are caused by corrupt images and are "not my > fault". > > Based on the result, I might tell the user "try again" or apologize and > generate logging that will notify developers. > > This seems like a common enough situation that it seems worth baking into > Imager. > > My initial idea is to consider upgrading 'errstr' to an exception object > that uses overloading to stringify to an string as it does not. > > Imager would classify exceptions "corrupt image" exceptions or general > exceptions as they happen. > > A second idea would be to add a new method: > > Imager->throw; > $imager->throw; > > Or more explicitly: > > Imager->throw_exception; > $imager->throw_exception; > > That would insure complete backcompat regarding 'errstr', and might also be > part of the solution to "make errors fatal", perhaps by pairing it with an > "auto_throw" option: > > Imager->new( auto_throw_exceptions => 1 ); > > Someone who wanted this behavior throughout a large project could sub-class > Imager and set "auto_throw_exceptions" as a default.
Maybe I'd add an error() method that returns an object with the error type (usage, corrupt image, I/O) and the components that make up the error message. Something like: # example reading a truncated GIF image { type => "CORRUPT", # or USAGE, IO (?), others? errors => [ [ 0, "IO", "Failed to read from file" ], [ 0, "CORRUPT", "Unable to get image descriptor" ] ] } (as an object with appropriate methods) Of course, doing this is a lot of work, I'd need an alternate set of i_push_error*() functions in the C parts of Imager that accept an error category as well as the code and message text. Show quoted text
> (Like the original idea, I would suggest that "throw" object of type > Imager::Exception or Imager::Exception::Corrupt based on the type of > error). > > What do you think?
My original idea was something along the lines of autodie, so *any* failure would cause an exception, complicated by some methods internally (harmlessly) producing errors which don't need to be returned to the caller (and hence shouldn't produce an exception.) Your idea is good in that it adds error categories but that's only loosely related to to "throw an exception on error". Basing the error category on the error message (as your sample does) is evil and broken and won't be implemented. How is your "auto_throw_exception" option intended to work with different categories - does it onlt throw on USAGE errors, or all errors? Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.