Re: [issue2551127] Detector errors (Value-, Key-, IndexError) should raise a traceback

Hi Ralf and Joseph:

In message <alpine.DEB.2.22.394.2104081709090.924398@digraph.polyomino.org.uk>,
Joseph Myers writes:
>Since the userauditor.py provided in all the templates shipped with 
>Roundup uses ValueError like this (for incorrect data provided by a 
>tracker user rather than a bug in the tracker), I expect that example is 
>very widely followed by people's own detectors.

My detectors also use ValueError(21 times) for cases where the user
supplied value is incorrect. E.G.

   raise ValueError('Timezone "%s" does not exist' % tz)

From the exception message, it looks like it is intended for the user.
I use this when the user supplied an invalid value.

They also use:

   roundupdb.DetectorError, mailgw.Unauthorized and MessageSendError

which are intended for the administrator.

Along with:

   Reject, RejectRaw

meant for the users. These are used when the values are valid but
something else (e.g. trying to close an issue with open dependencies,
setting up a loop of issues etc.)

The template detectors use:

  ValueError (lots)
  roundupdb.DetectorError
  InvalidOptionError


So I claim we should keep ValueError at least. I am not sure about the
KeyError or IndexError. From a user level perspective, I would expect
KeyError to be mapped to a ValueError assuming the user supplied the
key value. IndexError, I am not sure why that is being reformatted.

I don't remeber having any issues with a rogue ValueError or
IndexError causing issue or had issues debugging the cause of the
error.

That said, I assume this is the issue Ralf was trying to debug.  I can
see a couple of ways of making this easier.  Adding an option to
config.ini:

   detector_debug = Yes/No
or
   detector_debug = InvalidOptionError, DetectorError, ValueError

(maybe auditor_debug would be better) which would:

   enable tracebacks for all detector/auditor exceptions except
   Reject/RejectRaw
or
   enable tracebacks for only specified exceptions

If we can determine where the exception is thrown another possibility
presents itself. If it's not raised in a detector file you get a
traceback.

So if in detectors/foo.py I have:

    raise ValueError('this is an allowed error')

or if I have:
    try:
       somestuff_that_can_raise_value_error()
    except ValueError:

      check some stuff
      raise

they get formatted like Reject.

However if in detectors/foo.py I call:

    somestuff_that_can_raise_value_error()

and that raises a ValueError I get a traceback.

The ideas is if the user raised/re-raised the exception by writing
code in a detector, format it like Reject. If the exception originates
from code called by the detector and not caught you get the traceback
to help debug the issue.

Thoughts?

Have a great weekend all.

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.