[GRASS-dev] GRASS scripting Python API error/fatal handling

GRASS GIS GRASS GIS developer
1

Dear developers,
i am confused about the implementation of grass.error() and
grass.fatal() in the grass.scripting library.

From my understanding should the grass.error() function simply print

an error and grass.fatal() should either exit or call an exception so
that the GUI or a module developer can catch a fatal error. The
grass.set_raise_on_error() function allow to throw an exception in
case an error occurs that is not fatal to the processing. But it is
not possible to catch a fatal error because simply exit is called.
Can grass.fatal() be catched when raise_on_error is true, since it
calls internally grass.error()? If yes raise_on_error must be set by
the GUI to catch fatal error. BUT all errors that may not be fatal for
module processing will raise an exception too?

That gives me the impression that grass.fatal() should never be called
in a Python library function since otherwise the GUI will crash?

Howto implement library functions that face an error but should allow
i.e. the GUI to keep on processing? Printing an error and raise a
ScriptError exception should do it i guess. But what to do in cases
the same library functions are used in modules where the error should
be handled as fatal? Raising an exception will in this case confuse
the user, hence calling fatal would be more appropriate. IMHO Python
modules should show the same behavior as the C and bash modules and
avoid to print implementation language specific errors.

Hence i would like to add a "raise_on_fatal_error" flag to catch
grass.fatal() errors by exception and to avoid the raise of a script
error by calling grass.error().

What do you think? Any suggestions are welcome. :slight_smile:

Best regards
Soeren

2

Sören Gebbert wrote:

Dear developers,
i am confused about the implementation of grass.error() and
grass.fatal() in the grass.scripting library.

>From my understanding should the grass.error() function simply print
an error and grass.fatal() should either exit or call an exception so
that the GUI or a module developer can catch a fatal error. The
grass.set_raise_on_error() function allow to throw an exception in
case an error occurs that is not fatal to the processing. But it is
not possible to catch a fatal error because simply exit is called.
Can grass.fatal() be catched when raise_on_error is true, since it
calls internally grass.error()? If yes raise_on_error must be set by
the GUI to catch fatal error. BUT all errors that may not be fatal for
module processing will raise an exception too?

If raise_on_error is set, there is no difference between error() and
fatal(); both will generate a ScriptError which can be caught; fatal()
will never execute the sys.exit() call.

If raise_on_error is not set, error() will print an error message and
continue, while fatal() will print an error message then call
sys.exit() (which raises SystemExit, which can be caught but normally
shouldn't be).

I'm unsure whether placing the raise_on_error behaviour within error()
(as opposed to fatal()) was a good idea. But I don't know whether any
code depends upon the current behaviour.

That gives me the impression that grass.fatal() should never be called
in a Python library function since otherwise the GUI will crash?

Howto implement library functions that face an error but should allow
i.e. the GUI to keep on processing? Printing an error and raise a
ScriptError exception should do it i guess. But what to do in cases
the same library functions are used in modules where the error should
be handled as fatal? Raising an exception will in this case confuse
the user, hence calling fatal would be more appropriate.

In the absence of try/except, the only real difference between raising
ScriptError and SystemExit is that the latter won't result in a
backtrace being printed.

IMHO Python modules should show the same behavior as the C and bash
modules and avoid to print implementation language specific errors.

Python has exceptions, C doesn't. If C had exceptions, we would use
them instead of the "fatal error" mechanism, as exceptions are easier
to catch (catching fatal errors requires the use of setjmp() and
longjmp(), which can be messy, and nesting handlers is even worse) and
provide the same default behaviour (i.e. a program which doesn't go
out of its way to handle the error will terminate).

Hence i would like to add a "raise_on_fatal_error" flag to catch
grass.fatal() errors by exception and to avoid the raise of a script
error by calling grass.error().

Can you elaborate?

--
Glynn Clements <glynn@gclements.plus.com>

3

Hi Glynn,
thanks for your answer and sorry for the delay.

[snip]

If raise_on_error is set, there is no difference between error() and
fatal(); both will generate a ScriptError which can be caught; fatal()
will never execute the sys.exit() call.

If raise_on_error is not set, error() will print an error message and
continue, while fatal() will print an error message then call
sys.exit() (which raises SystemExit, which can be caught but normally
shouldn't be).

I'm unsure whether placing the raise_on_error behaviour within error()
(as opposed to fatal()) was a good idea. But I don't know whether any
code depends upon the current behaviour.

This behavior confuses me. So we need to force error() to raise an
exception to be able to catch a fatal() exception?
I would expect that catching a fatal() exception is independent from
catching an error() exception. IMHO the idea of having error() is that
an algorithm can keep on processing but inform the user that something
may be wrong in the result. The GUI or other software components force
such algorithms to raise on error() and break if they want to catch
only fatal().

[snip]

Hence i would like to add a "raise_on_fatal_error" flag to catch
grass.fatal() errors by exception and to avoid the raise of a script
error by calling grass.error().

Can you elaborate?

Well, i think it should be named "raise_on_fatal" rather than
"raise_on_fatal_error".

I would like to have the ability to catch only the exception that was
raised by fatal() when "raise_on_fatal" is set. In this case
algorithms that call error() will be able to continue computation
since error() will not raise an exception. The flag "raise_on_error"
should still be available, but the exception that are raised by
error() or fatal() should be different. I would suggest that a
ScriptError is raised by error() and a ScriptFatalError is raised by
fatal().

Best regards
Soeren

--
Glynn Clements <glynn@gclements.plus.com>

4

Sören Gebbert wrote:

> I'm unsure whether placing the raise_on_error behaviour within error()
> (as opposed to fatal()) was a good idea. But I don't know whether any
> code depends upon the current behaviour.

This behavior confuses me. So we need to force error() to raise an
exception to be able to catch a fatal() exception?
I would expect that catching a fatal() exception is independent from
catching an error() exception. IMHO the idea of having error() is that
an algorithm can keep on processing but inform the user that something
may be wrong in the result. The GUI or other software components force
such algorithms to raise on error() and break if they want to catch
only fatal().

The original behaviour was that error() simply called "g.message -e",
while fatal() also called sys.exit().

>> Hence i would like to add a "raise_on_fatal_error" flag to catch
>> grass.fatal() errors by exception and to avoid the raise of a script
>> error by calling grass.error().
>
> Can you elaborate?

Well, i think it should be named "raise_on_fatal" rather than
"raise_on_fatal_error".

I would like to have the ability to catch only the exception that was
raised by fatal() when "raise_on_fatal" is set. In this case
algorithms that call error() will be able to continue computation
since error() will not raise an exception. The flag "raise_on_error"
should still be available, but the exception that are raised by
error() or fatal() should be different. I would suggest that a
ScriptError is raised by error() and a ScriptFatalError is raised by
fatal().

Personally, I would favour simply restoring the original behaviour of
error(), i.e. print an error message then return. fatal() would always
be fatal in the sense of not returning, but you'd have a choice of
whether it throws SystemExit or e.g. ScriptError.

If you're not planning on catching the exception, SystemExit is
preferable as a program terminating due to an uncaught SystemExit
exception doesn't print a backtrace.

If you are planning on catching the exception, a normal exception
(i.e. a subclass of StandardError) should be used. SystemExit isn't
supposed to be "permanently" caught. Rather, it allows context
managers ("with" statement), try-finally, and try-except-raise to be
used to clean up on the way out.

For simple clean-up operations, atexit.register() is sufficient.

--
Glynn Clements <glynn@gclements.plus.com>

5

Hi Glynn,

Personally, I would favour simply restoring the original behaviour of
error(), i.e. print an error message then return. fatal() would always
be fatal in the sense of not returning, but you'd have a choice of
whether it throws SystemExit or e.g. ScriptError.

That's the behavior i would support.

If you're not planning on catching the exception, SystemExit is
preferable as a program terminating due to an uncaught SystemExit
exception doesn't print a backtrace.

Yes.

If you are planning on catching the exception, a normal exception
(i.e. a subclass of StandardError) should be used.

I think we can keep ScriptError for this case.

For simple clean-up operations, atexit.register() is sufficient.

While implementing this, can we use the "raise_on_error" flag to
enable the raise of a ScriptError exception in fatal()?
It would be only a simple modification of the current code i guess.

Best regards
Soeren

--
Glynn Clements <glynn@gclements.plus.com>

6

Sören Gebbert wrote:

> For simple clean-up operations, atexit.register() is sufficient.

While implementing this, can we use the "raise_on_error" flag to
enable the raise of a ScriptError exception in fatal()?
It would be only a simple modification of the current code i guess.

Yes. The only thing which needs to be done is to move the conditional
from error() to fatal(), and check any code which might be using
set_raise_on_error() in case it relies upon the existing behaviour.

Most scripts should continue to use atexit.register() for clean-up and
not bother with exception handling.

--
Glynn Clements <glynn@gclements.plus.com>

7

Hi,
sorry fir the late response.

···

2012/10/1 Glynn Clements <glynn@gclements.plus.com>

Sören Gebbert wrote:

For simple clean-up operations, atexit.register() is sufficient.

While implementing this, can we use the “raise_on_error” flag to
enable the raise of a ScriptError exception in fatal()?
It would be only a simple modification of the current code i guess.

Yes. The only thing which needs to be done is to move the conditional
from error() to fatal(), and check any code which might be using
set_raise_on_error() in case it relies upon the existing behaviour.

Done in r53396.

Best regards
Soeren

Most scripts should continue to use atexit.register() for clean-up and
not bother with exception handling.

–
Glynn Clements <glynn@gclements.plus.com>