UNO implementations & error conditions: canned error handling

[Thorsten Behrens]

Hi *,

there are a few recurring patterns for error handling in UNO API
implementations, three of which I'll address here:

 1. input validation/precondition checks. Violating one of those
    usually leads to an IllegalArgumentException being thrown.
 2. Runtime errors, usually signalled by a RuntimeException (this is
    comprised of a whole bunch of error conditions, ranging from logic
    errors/invariant violations to resource exhaustion)
 3. exception-less error reporting, usually via boolean return values

For all cases, one might want to see an assertion in debug builds, if
only to make sure the caller handles the error condition correctly.

So, I'd like to propose a bunch of macros, quite similar to the
OSL_ASSERT/OSL_ENSURE family, that, in the case of an error, show an
assertion in debug builds, and always throw a corresponding exception:

---%<-----------------------------------------------------------------

#define VALIDATE_AND_THROW(c, m, ifc, arg_num)  if( !(c) ) { \
           OSL_ENSURE(false, m); \
           throw ::com::sun::star::lang::IllegalArgumentException( \
           ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
           ::rtl::OUString::createFromAscii(",\n"m), \
           ifc, \
           arg_num ); }

#define ENSURE_AND_THROW(c, m, ifc) if( !(c) ) { \
          OSL_ENSURE(false, m); \
          throw ::com::sun::star::uno::RuntimeException( \
          ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
          ::rtl::OUString::createFromAscii(",\n"m), \
          ifc ); }

#define ENSURE_AND_RETURN(c, m)  if( !(c) ) { \
          OSL_ENSURE(false, m); \
          return false; }

---%<-----------------------------------------------------------------

Is this useful, and if yes, where to place it? It needs UNO, so osl is
too low, and it currently employs boost, which afaik rules out udk...

Feedback greatly appreciated,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Stephan Bergmann]

Thorsten Behrens wrote:

> Hi *,
>
> there are a few recurring patterns for error handling in UNO API
> implementations, three of which I'll address here:
>
>  1. input validation/precondition checks. Violating one of those
>     usually leads to an IllegalArgumentException being thrown.
>  2. Runtime errors, usually signalled by a RuntimeException (this is
>     comprised of a whole bunch of error conditions, ranging from logic
>     errors/invariant violations to resource exhaustion)
>  3. exception-less error reporting, usually via boolean return values
>
> For all cases, one might want to see an assertion in debug builds, if
> only to make sure the caller handles the error condition correctly.
>
> So, I'd like to propose a bunch of macros, quite similar to the
> OSL_ASSERT/OSL_ENSURE family, that, in the case of an error, show an
> assertion in debug builds, and always throw a corresponding exception:
>
> ---%<-----------------------------------------------------------------
>
> #define VALIDATE_AND_THROW(c, m, ifc, arg_num)  if( !(c) ) { \
>            OSL_ENSURE(false, m); \
>            throw ::com::sun::star::lang::IllegalArgumentException( \
>            ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>            ::rtl::OUString::createFromAscii(",\n"m), \
>            ifc, \
>            arg_num ); }
>
> #define ENSURE_AND_THROW(c, m, ifc) if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           throw ::com::sun::star::uno::RuntimeException( \
>           ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>           ::rtl::OUString::createFromAscii(",\n"m), \
>           ifc ); }
>
> #define ENSURE_AND_RETURN(c, m)  if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           return false; }
>
> ---%<-----------------------------------------------------------------
>
> Is this useful, and if yes, where to place it? It needs UNO, so osl is
> too low, and it currently employs boost, which afaik rules out udk...

Yes, the interface of URE is kept clear of boost for now, so porting and
udk are ruled out.

Personally I'm undecided whether some macros like these are really
needed.  At the very least, I would rename them from "_AND_" to "_OR_."

-Stephan

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Thorsten Behrens]

Stephan Bergmann <stephan.bergmann@...> writes:

> Personally I'm undecided whether some macros like these are really
> needed.
>
How do you handle the mentioned cases? Never assert at the place of
the error?

> At the very least, I would rename them from "_AND_" to "_OR_."
>
Fine with me.
 
Cheers,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Frank Schönheit - Sun Microsystems Germany]

Hi Thorsten,

> #define VALIDATE_AND_THROW(c, m, ifc, arg_num)  if( !(c) ) { \
>            OSL_ENSURE(false, m); \

this should be "OSL_ENSURE(c, m)", IMO - just in case somebody later
changes OSL_ENSURE to also display the violated condition.

>            throw ::com::sun::star::lang::IllegalArgumentException( \
>            ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \

Not sure this is a good idea - putting the current function into the
exception text. Exceptions messages might happen to be read by end users
(e.g. when displayed in StarBasic), and the function name - which is a
multiple-line complex beast when obtained via the BOOST macro - is
certainly not for end users.

Also, this would flood the release builds with a lot of strings of only
secondary interest.

I'd prefer putting the CURRENT_FUNCTION into the assertion instead.

>            ::rtl::OUString::createFromAscii(",\n"m), \
>            ifc, \
>            arg_num ); }
>
> #define ENSURE_AND_THROW(c, m, ifc) if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           throw ::com::sun::star::uno::RuntimeException( \
>           ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>           ::rtl::OUString::createFromAscii(",\n"m), \
>           ifc ); }

Of course, some kind of resource-fed versions of those would also be
nice :), with a similar reason as above: Exception messages are
potentially read by end users, and sometimes it's debatable whether they
should be localized. But that is probably another can of worms. Also,
this then would really rule out the UDK.

> #define ENSURE_AND_RETURN(c, m)  if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           return false; }

parametrizing the return type might be a good idea, too.

>
> ---%<-----------------------------------------------------------------
>
> Is this useful, and if yes,

Yes.

> where to place it? It needs UNO, so osl is
> too low, and it currently employs boost, which afaik rules out udk...

Well, boost/current_function.hpp is really small. Nobody hinders us from
duplicating those about 10 lines (less, if we count currently supported
compilers only) of code in UDK, right?


Whether the macros are really *needed* - well, this might be debatable.
They're useful, at least, so I'm for adding them. To osl/diagnose_ex.h.
Or something like this.

(Yes, they *belong* into the UDK. We should definitely *not* introduce
yet another place of such diagnostics facilities. We already have
osl/diagnose.h, tools/debug.hxx (and, speaking strictly, the less known
tools/diagnose_ex.h), and those are already too much.)

Ciao
Frank

--
- Frank Schönheit, Software Engineer         frank.schoenheit@... -
- Sun Microsystems                      http://www.sun.com/staroffice -
- OpenOffice.org Base                       http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Stephan Bergmann]

Thorsten Behrens wrote:
> Stephan Bergmann <stephan.bergmann@...> writes:
>
>> Personally I'm undecided whether some macros like these are really
>> needed.
>>
> How do you handle the mentioned cases? Never assert at the place of
> the error?

Yes, I never assert when I throw an exception.  (Not that I would
necessarily discourage the other way.  I am just unsure whether
introducing those macros best serves that other way---they hide control
flow, hide which types of exceptions are thrown, and are macros.)

-Stephan

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Thorsten Behrens]

Hi Frank,

you wrote:

> > #define VALIDATE_AND_THROW(c, m, ifc, arg_num)  if( !(c) ) { \
> >            OSL_ENSURE(false, m); \
>
> this should be "OSL_ENSURE(c, m)", IMO - just in case somebody later
> changes OSL_ENSURE to also display the violated condition.
>
ok.

> >            throw ::com::sun::star::lang::IllegalArgumentException( \
> >            ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>
> Not sure this is a good idea - putting the current function into the
> exception text. Exceptions messages might happen to be read by end users
> (e.g. when displayed in StarBasic), and the function name - which is a
> multiple-line complex beast when obtained via the BOOST macro - is
> certainly not for end users.
>
> Also, this would flood the release builds with a lot of strings of only
> secondary interest.
>
> I'd prefer putting the CURRENT_FUNCTION into the assertion instead.
>

Hm - the intention was to make error reporting easier here. With the
assertion, one can quite easily get to the faulty code place. With an
exception, it's a bit harder - BOOST_CURRENT_FUNCTION would make that
unambiguously clear. But yeah, the size argument is clearly valid.

> Of course, some kind of resource-fed versions of those would also be
> nice :), with a similar reason as above: Exception messages are
> potentially read by end users, and sometimes it's debatable whether they
> should be localized.
>
Nope, don't think so. Lingu franca of programming is English - or do
you also want to translate IDL documentation?

> >           OSL_ENSURE(false, m); \
> >           return false; }
>
> parametrizing the return type might be a good idea, too.
>
One has to strike a balance. I thought about it as well, but the cases
I would have needed a different return type (mostly NULL) have been
seldom enough that I wouldn't impose the additional clutter to the
macro...

> Well, boost/current_function.hpp is really small. Nobody hinders us from
> duplicating those about 10 lines (less, if we count currently supported
> compilers only) of code in UDK, right?
>
Nope. Would prolly also benefit the other assertions - I'd change them
to include the function name then, as well.

> (Yes, they *belong* into the UDK. We should definitely *not* introduce
> yet another place of such diagnostics facilities. We already have
> osl/diagnose.h, tools/debug.hxx (and, speaking strictly, the less known
> tools/diagnose_ex.h), and those are already too much.)
>
So, you suggest using stuff from udkapi in osl, relying on the fact
that anyone making sensible use of them prolly implements UNO API (and
thusly already is past udkapi in the build hierarchy)?

Cheers,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Frank Schönheit - Sun Microsystems Germany]

Hi Thorsten,

>> Of course, some kind of resource-fed versions of those would also be
>> nice :), with a similar reason as above: Exception messages are
>> potentially read by end users, and sometimes it's debatable whether they
>> should be localized.
>>
> Nope, don't think so. Lingu franca of programming is English - or do
> you also want to translate IDL documentation?

I did say this is a can of worms, didn't I? :) Let's not discuss it in
this thread here ...

>>>           OSL_ENSURE(false, m); \
>>>           return false; }
>> parametrizing the return type might be a good idea, too.
>>
> One has to strike a balance. I thought about it as well, but the cases
> I would have needed a different return type (mostly NULL) have been
> seldom enough that I wouldn't impose the additional clutter to the
> macro...

Okay.

>> Well, boost/current_function.hpp is really small. Nobody hinders us from
>> duplicating those about 10 lines (less, if we count currently supported
>> compilers only) of code in UDK, right?
>>
> Nope. Would prolly also benefit the other assertions - I'd change them
> to include the function name then, as well.

+1

>> (Yes, they *belong* into the UDK. We should definitely *not* introduce
>> yet another place of such diagnostics facilities. We already have
>> osl/diagnose.h, tools/debug.hxx (and, speaking strictly, the less known
>> tools/diagnose_ex.h), and those are already too much.)
>>
> So, you suggest using stuff from udkapi in osl, relying on the fact
> that anyone making sensible use of them prolly implements UNO API (and
> thusly already is past udkapi in the build hierarchy)?

Ehm, well, I overlooked your argument "osl is too deep for UNO" in the
first place. Okay, this perhaps wouldn't be a good idea (and vetoed by
Stephan, anyway :). Sigh. I really really really really dislike the fact
of introducing yet another place for diagnostics. So much that I'm
tempted to say we should not do this at all :-\

Okay, what's in the UDK, and able to use UNO? Let's place a diagnose.h
there, which also includes osl/diagnose.h.

Ciao
Frank

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Thorsten Behrens]

Hi Frank,

you wrote:

> Okay, what's in the UDK, and able to use UNO? Let's place a diagnose.h
> there, which also includes osl/diagnose.h.
>
well, unotools? ;-)

Anyway, I'm open for suggestions here - and would then think that
tools/diagnose_ex.h (why the heck is that .h?) content should also be
moved there. I'd hope to get rid of the tools assertions some day, in
favor of consolidated stuff on UDK level.

Cheers,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Frank Schönheit - Sun Microsystems Germany]

Hi Thorsten,

>> Okay, what's in the UDK, and able to use UNO? Let's place a diagnose.h
>> there, which also includes osl/diagnose.h.
> well, unotools? ;-)

not part of the UDK. Stehan, alternatives?

> Anyway, I'm open for suggestions here - and would then think that
> tools/diagnose_ex.h (why the heck is that .h?)

Ehm - because ... ehm ...

> content should also be moved there.

+1

> I'd hope to get rid of the tools assertions some day, in
> favor of consolidated stuff on UDK level.

+10

Ciao
Frank

--
- Frank Schönheit, Software Engineer         frank.schoenheit@... -
- Sun Microsystems                      http://www.sun.com/staroffice -
- OpenOffice.org Base                       http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Niklas Nebel]

Frank Schönheit - Sun Microsystems Germany wrote:
>> I'd hope to get rid of the tools assertions some day, in
>> favor of consolidated stuff on UDK level.
>
> +10

Right, it would be very nice to have a small set of macros/helpers that
are consistently used, so you can read source from any module and see
what's going on. Adding more new macros might actually move us away from
that goal.

Niklas

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Frank Schönheit - Sun Microsystems Germany]

Hi Niklas,

>>> I'd hope to get rid of the tools assertions some day, in
>>> favor of consolidated stuff on UDK level.
>> +10
>
> Right, it would be very nice to have a small set of macros/helpers that
> are consistently used, so you can read source from any module and see
> what's going on. Adding more new macros might actually move us away from
> that goal.

Well ... if people start adding those macros to their own modules for
their own convenience, this also moves us aways from that goal. We can
either have those macros in every module, being different each time, or
in one global place, or not at all. What do you prefer?

Ciao
Frank

--
- Frank Schönheit, Software Engineer         frank.schoenheit@... -
- Sun Microsystems                      http://www.sun.com/staroffice -
- OpenOffice.org Base                       http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[bjoern.milcke]

Hi Thorsten,

> [...]
>
> ---%<-----------------------------------------------------------------
>
> #define VALIDATE_AND_THROW(c, m, ifc, arg_num)  if( !(c) ) { \
>            OSL_ENSURE(false, m); \
>            throw ::com::sun::star::lang::IllegalArgumentException( \
>            ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>            ::rtl::OUString::createFromAscii(",\n"m), \
>            ifc, \
>            arg_num ); }
>
> #define ENSURE_AND_THROW(c, m, ifc) if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           throw ::com::sun::star::uno::RuntimeException( \
>           ::rtl::OUString::createFromAscii(BOOST_CURRENT_FUNCTION) + \
>           ::rtl::OUString::createFromAscii(",\n"m), \
>           ifc ); }
>
> #define ENSURE_AND_RETURN(c, m)  if( !(c) ) { \
>           OSL_ENSURE(false, m); \
>           return false; }
>
> ---%<-----------------------------------------------------------------

I am just wondering if the naming is clear enough.

VALIDATE_AND_THROW and ENSURE_AND_THROW are the same, except that the
former throws an IllegalArgumentException whereas the latter throws a
RuntimException. And both validate and ensure. I think when reading the
place where such a macro is used, I wouldn't exactly know what happens.
Or, when wanting to use it I wouldn't probably remember which was which.

     // I admit: foo should be an argument here, so you should see that
     VALIDATE_AND_THROW( foo==1, "foo should be 1", this, 2 );
     ENSURE_AND_THROW( foo==1, "foo should be 1", this );

What about (after changing also AND to OR):

     ENSURE_ARG_OR_THROW
     ENSURE_OR_THROW

Just my 2 ct's (1ct for each macro)

-Bjoern

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Thorsten Behrens]

Bjoern Milcke <Bjoern.Milcke@...> writes:

> I am just wondering if the naming is clear enough.
>
> VALIDATE_AND_THROW and ENSURE_AND_THROW are the same, except that the
> former throws an IllegalArgumentException whereas the latter throws a
> RuntimException. And both validate and ensure. I think when reading
> the place where such a macro is used, I wouldn't exactly know what
> happens. Or, when wanting to use it I wouldn't probably remember which
> was which.
>
What would you propose instead?

Cheers,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[bjoern.milcke]

Hi Thorsten,

> Bjoern Milcke <Bjoern.Milcke@...> writes:
>
>> I am just wondering if the naming is clear enough.
>>
>> VALIDATE_AND_THROW and ENSURE_AND_THROW are the same, except that the
>> former throws an IllegalArgumentException whereas the latter throws a
>> RuntimException. And both validate and ensure. I think when reading
>> the place where such a macro is used, I wouldn't exactly know what
>> happens. Or, when wanting to use it I wouldn't probably remember which
>> was which.
>>
> What would you propose instead?

Er, what you cut away:

     ENSURE_ARG_OR_THROW
     ENSURE_OR_THROW
     ENSURE_OR_RETURN

-Bjoern

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...

[Thorsten Behrens]

Bjoern Milcke <Bjoern.Milcke@...> writes:

> > What would you propose instead?
>
> Er, what you cut away:
>
>      ENSURE_ARG_OR_THROW
>      ENSURE_OR_THROW
>      ENSURE_OR_RETURN
>
Ah, must have skipped that, mentally ;-)

Sounds fine!

Cheers,

-- Thorsten

---------------------------------------------------------------------
To unsubscribe, e-mail: interface-discuss-unsubscribe@...
For additional commands, e-mail: interface-discuss-help@...