[Mono-dev] New error handling framework for mono

[Rodrigo Kumpera]

On Thu, Sep 3, 2009 at 12:36 PM, Paolo Molaro <lupus at ximian.com> wrote:

> I'm not happy with both the MONO_ERROR_STORE_FULL_MESSAGE and
> MONO_ERROR_DONT_COPY_STRINGS flags you introduced.
> For the message this could be handled automatically: if it doesn't fit
> into the buffer you can malloc it at set time, the code that initializes
> the
> MonoError has no way to know if the message will fit or not.
> Similarly, the initializing code can't know if any of the strings need
> to be strduped because it's far away from the code that sets it.
> This should be decided at the callsite, either passing a flag or calling
> a separate function that does the dup.
>

I introduced mono_error_dup_strings which let's the caller control if the
supplied
strings are to be released.


> A small note on overlong messages. I think we should have a policy like
> this:
> *) error messages should be short sentences
> *) any non-essential description of the error and suggestions for
> workarounds should go into either or both the logging mechanism and the
> documentation (wiki etc.)
>

Makes sense, one thing we could improve in the logged messages is to provide
some sort of unique code describing it. This helps a lot troubleshooting
users
problems and is very google friendly.



> > But what should be done when we get an allocation failure on the
> > mono_exception_set_*
> > function? Should we just convert such error into an OOM, abort due to a
> > double-fault or
> > silently ignore those arguments (and print something on console).
>
> It is tricky. In some cases we want to communicate an OOM exception to
> the managed code. In some others we can't: we need to give the caller
> code a chance to handle the error and not crash.
> Using a flag here can be helpful: something like MONO_ERROR_INCOMPLETE
> to signal that the full info was not stored for whatever reason.
> That should about strike a good balance between being robust and keeping
> track of as much info as we can. Aborting here is not an option, IMHO.
>

I added a MONO_ERROR_INCOMPLETE used for that.

> +void
> > +mono_error_set_corlib_exception (MonoError *oerror, const char
> *name_space, const char *name)
> > +{
> > +     MonoErrorInternal *error = (MonoErrorInternal*)oerror;
> > +
> > +     if (error->flags & MONO_ERROR_DONT_COPY_STRINGS) {
> > +             error->exception_name_space = name_space;
> > +             error->exception_name = name;
> > +     } else {
> > +             error->exception_name_space = g_strdup (name_space);
> > +             error->exception_name = g_strdup (name);
> > +     }
>
> This should set error_code as well (maybe have it passed as an argument).
>
>
I removed those functions from the API as they add no value at all. The way
to go is to
use one of the provided error-setting functions. Using those setter
functions won't make
difference for reporting the error.
If it's not enough, then it's better to just add another entry in the error
codes array and a
new error-setting function.

Attached is a revised patch.

Cheers,
Rodrigo
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090908/80b6a25e/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mono-error-patch-2.diff
Type: text/x-diff
Size: 17122 bytes
Desc: not available
Url : http://lists.ximian.com/pipermail/mono-devel-list/attachments/20090908/80b6a25e/attachment-0001.bin