[csw-maintainers] pkgcheck phase is failing

[Maciej (Matchek) Bliziński]

2013/1/7 Yann Rouillard <yann at pleiades.fr.eu.org>:
>
> 2013/1/7 Maciej (Matchek) Bliziński <maciej at opencsw.org>
>
>> 2013/1/7 Yann Rouillard <yann at pleiades.fr.eu.org>:
>> > I think that makes a lot of sense.
>> >
>> > I suppose we should define some exception class whose instances would at
>> > least contain the error message and the information message.
>> > Any check could use this exception or a sub-class when they need to
>> > raise an
>> > error.
>>
>> Each module can define its exceptions; I was following this rule most
>> of the time so far. But for simplicity, we should not change the type
>> of exception we're raising, only display an error message and
>> propagate the exception as-is. This way we keep the code simpler and
>> not bury ourselves in an exception handling jungle.
>
>
> You think it will be so complicated if we just use a parent class for all
> exceptions (or at least these kind of exceptions) ?

No, for custom exceptions we should use specific classes. For example,
in the opencsw.py file

https://sourceforge.net/apps/trac/gar/browser/csw/mgar/gar/v2/lib/python/opencsw.py

...we're using PackageError, and it's good that way.

I looked at the code that runs ldd:

https://sourceforge.net/apps/trac/gar/browser/csw/mgar/gar/v2/lib/python/inspective_package.py#L289

...and I see that it raises the non-specific package.Error. I think
that this exception is specific to running ldd. So we could either
declare a new exception in package.py, or create a new exception in
inspective_package.py:

class LddExecutionError(package.Error):
  """Problem while running ldd."""

>> > Now the question is just: where is the central place to catch these
>> > exceptions and display the message ?
>>
>> I would like to avoid having this. If anything goes wrong, I want to
>> crash early and loud.
>
>
> I find cleaner that the function just raises exceptions and that the caller
> decides how to handle the message.

Sure. Is that different from what I was saying?

> I find this more flexible, it allows us to easily make change to the way we
> handle these errors if this is done in a one place.

Generally, the place where you should handle an exception depends on
the exception, no?

> About the need to crash early, what is exactly the risk ?

What's the risk of not crashing early when there's a problem?

Eric Raymond wrote it best:
http://www.faqs.org/docs/artu/ch01s06.html#id2878538

"It's best when software can cope with unexpected conditions by
adapting to them, but the worst kinds of bugs are those in which the
repair doesn't succeed and the problem quietly causes corruption that
doesn't show up until much later."

> After your
> comment, I thought that for example an intermediate function might catch
> exceptions too broadly and prevents the exception from being properly
> handled. I am not sure if this is a big problem. We could easily search for
> this kind of exception catch.
>
> At least, I think we should create some common function used by each module
> to print the error message so we keep the advantage of not duplicating the
> code used to display the error.

We have that already, the code to display the error is simply:

logging.fatal("...")

Or you can roll the message right into the exception itself.

>> > I didn't check yet but I am sure you already have an idea.
>> >
>> > For the stack trace, we could display it only in debug mode.
>>
>> I'm thinking that if we don't display the stack trace, we won't be
>> able to ask for it right away (“can you show me the stack trace?” --
>> “what stack trace?”). Right now people can immediately copy/paste the
>> stack trace and quickly give us idea about what's going on. Without
>> the stack trace, we'll still have to go and ask people to re-run in
>> debug mode, then copy and paste. Why add an additional middle step?
>
>
> Well I understood that the idea was not freak people.

Let me illustrate. Good stack trace (does not freak people out that much):

Traceback (most recent call last):
  File "./example.py", line 13, in <module>
    main()
  File "./example.py", line 10, in main
    raise IntentionalError("Yes, I meant to do throw this exception, "
__main__.IntentionalError: Yes, I meant to do throw this exception,
and it means that foo is probably barred.

Bad stack trace (freaks people out):

Traceback (most recent call last):
  File "./example_bad.py", line 12, in <module>
    main()
  File "./example_bad.py", line 10, in main
    raise IOError()
IOError

> I think that a stack trace can give the impression that the error was not
> properly handled and can prevent people from really looking at the output
> and see the informative message.

If we had millions of casual users, I would agree. But our user base
is rather technically savvy, dealing with breakages day to day, so we
should be able to educate them that the stack trace is something
useful to show to the developer.

> If the stack trace is really long, you might even miss the informative
> message.

I think that shouldn't happen, the error message from inside the
exception gets displayed at the very bottom of the trace.

> We can try to lay out the message so we don't miss the informative part, but
> we would need to generate the traceback ourself to be able to display it
> before the informative message.

If anything, we should display it below, not above, no?

What's wrong with the default stack trace, how own stack trace will be better?

> That would advocate even more to use some
> common code to display the error.

Right now we say simply: raise WhatWentWrong("and why"). How do you
improve on that? It doesn't look to me that it can be shorter than
that.

Maciej