#91 Line #s in EvalError missing from many p

closed-fixed
nobody
None
7
2003-05-30
2002-12-10
Shankar Unni
No

Line numbers are missing in many EvalErrors, because
too many places call the convenience constructor that
skips the Node that caused the error.

This makes it really hard to debug scripts. Ideally,
the convenience constructors should be removed, and
every caller should be made to specify an explicit
Node, or null if one is truly not available.

The other problem is that there are many places where
the source node is not passed down to inner routines,
so that even though an error is taking place in the
context of a node, that node is not present in the
exception. This is especially true of TargetExceptions
caused by null pointers in the script itself (calling
nullobj.someMember(), for instance).

Discussion

  • Shankar Unni
    Shankar Unni
    2002-12-10

    Logged In: YES
    user_id=167432

    By the way, Pat - many of these are trivially fixable, so we
    can easily do it ourselves. How can we contribute code changes?

     
  • Nick Fortescue
    Nick Fortescue
    2002-12-10

    Logged In: YES
    user_id=251678

    Pat, can I add a vote for this one to be fixed? It's been a pain
    for us. I can very quickly do an audit and submit a list of the
    problem areas.

     
  • Logged In: NO

    Thanks. I'm aware that this is very annoying and will get
    it fixed and try to find a way to ensure that it is
    maintained properly in the future.

    The problem as you can see with removing the default
    constructor is that there are places buried in utility
    classes that don't have the context available to them. It's
    an analogous problem that that of needing the interpreter
    reference everwhere we do evaluation (interetsting aside -
    this would be a great place for an aspect pattern using
    something like AspectJ). The easiest thing to do is simply
    enforce our rules and make sure we catch these and add the
    context to them...

    If someone already has a list of known problem areas that
    would be great. I can probably get those into the next 1.2
    bug fix release (working on it right now).

    More generally - how to keep this from happening again -
    Maybe we could make a new, checked exception type to replace
    the no-context target error... one that we have to catch in
    the ASTs and rethrow as the target error... then we could
    remove that constructor. Lemme think about it a bit.

    Pat

     
  • Pat Niemeyer
    Pat Niemeyer
    2002-12-10

    Logged In: YES
    user_id=18885

    Actually, after doing a quick check this doesn't look as bad
    as I'd thought. There aren't that many cases and some of
    those look easily corrected.

    Pat

     
  • Shankar Unni
    Shankar Unni
    2002-12-23

    • priority: 5 --> 7
     
  • Shankar Unni
    Shankar Unni
    2002-12-23

    Logged In: YES
    user_id=167432

    I'm jacking up the priority somewhat, as this is kind of
    important for good error reporting. Do you use the priority
    field?

     
  • Pat Niemeyer
    Pat Niemeyer
    2002-12-24

    Logged In: YES
    user_id=18885

    I have fixed this for the 1.3 release by removing the
    no-context (no node) constructors from EvalError and
    TargetError. This was more painful that I had thought, but
    it's done. I will check it into the head of CVS along with
    some other changes soon.

    Pat

     
  • Pat Niemeyer
    Pat Niemeyer
    2002-12-24

    • status: open --> open-fixed
     
  • Pat Niemeyer
    Pat Niemeyer
    2003-05-30

    • status: open-fixed --> closed-fixed