Closed sourceforge-issue-exporter closed 5 years ago
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?
Original comment by: shankarunni
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.
Original comment by: nfortescue
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
Original comment by: nobody
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
Original comment by: patn
Original comment by: shankarunni
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?
Original comment by: shankarunni
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
Original comment by: patn
Original comment by: patn
Original comment by: patn
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).
Reported by: shankarunni
Original Ticket: "beanshell/bugs/91":https://sourceforge.net/p/beanshell/bugs/91