Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 03/10/2013 10:02, IKEDA Soji a écrit :
> > > > Exception handling seems not to be resolved. the fatal_err calls were
> > > > usefull because they specified explicitely that Sympa should not keep on
> > > > running under certain circumstances. This is a clear distinction with
> > > > the return undef we used as exception handling.
> > > > I am under the impression that all return undef could be replaced by
> > > > croak calls, providing we intercept exceptions at the right place and we
> > > > use the Carp option allowing to produce stak traces.
> > > > But, we need to distinguish the very few circumstances in which sympa
> > > > should stop running, so that the daemons will stop gracefully.
> > > > Inspecting the code, it looks like all fatal_err calls were made in the
> > > > main daemons, so I think it should be easy.
> > > My concern on exception:
> > >
> > > - It tends to skip clean-up processes inside "try" and other blocks.
> > >    Not only skipping each logic, it is likly to cause leaks of
> > >    rosources such as memory.  The function corresponding to "finally"
> > >    may probably ease such situation, while it can worsen the next
> > >    problem.
> > If the "finally" resolves the memory leaks, I violently support this option.
finally does just provides a code place holder, it doesn't magically fix 
any resource liberation issue.

> > > - It is a bit hard to read.  The idiom using eval {} and if () is
> > >    obvious (additionally, by this idiom, programmers must re-throw
> > >    uncaught exceptions by themselves).  Syntax sugers provided by
> > >    several modules are not always acceptable by average Perl
> > >    programmers.
> > If we set up clear rules about how exceptions are handled in Sympa on
> > the Sympa web site, it should not be too much trouble.
>
> D'accord, I'll try to figure out the rule.
>
> IMO to emulate "try-catch" by Perl5, objects will be passed to
> croak, instead of string.  Here is a template:
>
>      # "try" block
>      eval {
>
>          # *1
>
>          # Throw exception if error occurred
>          if (...) {
>              # *3
>              croak Sympa::Exception::Foo->new(
>                  File => __FILE__, Line => __LINE__, Message => 'error 
> occurred'
>              );
>          }
>
>          # *2
>
>      };
>
>      # "catch" block
>      if (ref $@ eq 'Sympa::Exception::Foo') {
>
>          # process exception
>
>      } elsif (ref $@ eq 'Sympa::Exception::Bar') {
>
>          # process another exception
>
>      }
Testing if an object belongs to a given class should use the ->isa() 
method, not rely on string comparaison. Otherwise you'll miss subclasses 
instances.
if ($@) {
    if (blessed $@) {
             if ($@->isa('Sympa::Exception::Foo') {
        } elsif ($@->isa('Sympa::Exception::Bar') {
        }
    } else {
    }
}


>      # uncuaght exceptions
>      # We have responsibility to re-throw exceptions we won't catch.
>      elsif (ref $@) {
>          croak $@;
>      } elsif ($@) {
>          croak "$@\n";
>      }
>
> Last two elsif blocks are somewhat eyesore, but they are exactly
> necessary not to miss exceptions raised by others (maybe in lower
> level).
Throwing uncaught exception is a choice, it is not mandatory. The same 
way you can perfectly ignore result of any function call. And the 
explicit concatenation of \n character to a string-based exception is 
cosmetic only, meaning the two block could be merged in to one only:
} else {
    croak $@;
}

> My first concern: For example, a filehandle is opened at (*1) then
> closed at (*2).  When exception is thrown, that filehandle can
> leak.  So we must also close it at (*3).  It is not easy to compile
> this clean-up process into single place such as "finally" block.
>
> # Here, I ignored the alternative to use CPAN modules.
Using CPAN modules has a cost about your target user base.

A CPAN module which just provides syntactic sugar (try/catch keywords, 
instead of eval/if, for instance), is definitively not worth the cost.

One that allows to propagate objects instead of strings would eventually 
be more interesting.

However, there is still room for improvement with current error messages 
(for instance, having more useful and readable ones), before changing 
the way they are transmitted.

> Anyway, my second concern: is the rule above easy enough to keep?
> (this is a simple question, not rhetorical)
>
>
> > > I'm not an opponent of exception, contrary, I prefer to this
> > > concept.  But by now I'm undecided.
> > There is something still unclear to me: How exceptions are escalated. If
> > I understand correctly, we plan to croak or carp everywhere we used
> > "return undef" and eval{} these exceptions at the very top level (i.e.,
> > in each Sympa daemon loops). In the end, will we have only the original
> > message or all the croak messages we will meet between the original
> > error and the final eval{} interception?
>
> "try-catch" structure described above may appear in several levels,
> because clean-up processes such as filehandle can be accomplished
> only in each level.
>
> Each "try-catch" catches particular exceptions and processes it ---
> logs messages, recovers errors etc.  Uncaught exceptions will be
> escalated (re-thrown) to upper level.  Once an exception has been
> caught, it no longer will propagate to upper level.
>
> Finally at uppermost level, Perl catches exceptions never caught
> and dies.
>
> There is one more comment on implementation.
>
> Error messages are contained in exception objects.  Also,
> information of call stack should be collected when each exception
> object is instantiated.  These information may be shown as error
> message and traceback.
> However, if exception is simple scalar (thrown by external modules or
> Perl), traceback cannot be shown.  Because information of call stack
> had been lost.
I'd rather say: traceback in external code is lost, if you don't 
intercept them when entering your own code.

I'm still convinced than we have other priorities than switching error 
handling to exceptions all over the place right now. And a limited usage 
of them would probably help figuring out how they work exaclty.
--
Guillaume Rousse
INRIA, Direction des systèmes d'information
Domaine de Voluceau
Rocquencourt - BP 105
78153 Le Chesnay
Tel: 01 39 63 58 31

Attachment: smime.p7s
Description: Signature cryptographique S/MIME