[Tcl-bugs] [ tcl-Bugs-652765 ] errors disappear in trace callbacks

[SourceForge.net <no...@so...>]

Bugs item #652765, was opened at 2002-12-12 16:49
Message generated for change (Comment added) made by dkf
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=652765&group_id=10894

Category: 45. Traces
Group: obsolete: 8.4.1
Status: Open
Resolution: None
Priority: 5
Submitted By: Nobody/Anonymous (nobody)
Assigned to: Don Porter (dgp)
Summary: errors disappear in trace callbacks

Initial Comment:
Errors occuring in "trace add command" callbacks (i.e.,
rename and delete callbacks) are swallowed by Tcl
instead of being propagated to the application.

wi...@wj...

----------------------------------------------------------------------

>Comment By: Donal K. Fellows (dkf)
Date: 2004-08-26 11:26

Message:
Logged In: YES 
user_id=79902

Note that delete traces have the same properties; both kinds
are notifications to be done after the fact.

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2004-08-26 11:07

Message:
Logged In: YES 
user_id=32170

Totally agree with dkf's summary.  Only remaining question
is how do we carry out (2)?  (Note: this is a very similar
problem to that faced by the tclvfs extension, which uses
'vfs::filesystem internalerror $cmdprefix' to specify a
command prefix to call whenever there's an error).

One possibility would be to use the 'tclLog' proc which
appears to be built into Tcl.  I'm not really sure what
'tclLog' is documented to do, but it does exist!  (In fact
it might be a good idea of tclvfs to use it instead of its
own ad-hoc approach).




----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-08-26 10:02

Message:
Logged In: YES 
user_id=79902

Summing up:
 1) Do we propagate errors?
 2) If we propagate errors, do we do this directly or via
some kind of background mechanism?
 3) If we propagate errors directly, do we permit them to
abort the renaming or prevent other rename traces from
getting called?

IMO:
  1) Yes
  2) bgerror is good, but other can work
  3) the error should *not* abort the rename

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2004-08-26 09:26

Message:
Logged In: YES 
user_id=32170

Sorry, I disagree.  If errors are swallowed then we have a
well-defined, consistent behaviour: command renames and
deletes always succeed.  Therefore all trace callbacks can
assume that is true.

Of course this doesn't mean that a badly written callback
can't accidentally throw an error and therefore not succeed
and therefore leave _itself_ in an inconsistent state. But
that is an example of the programmer causing themselves a
problem.

However, if errors propagate, then the example I gave shows
how one trace callback can abort a 'rename' and therefore
cause problems to another trace callback which doesn't know
the rename was aborted.

This means that one programmer can shoot another programmer
in the foot.  That's the inconsistency I don't like. There's
a distinction between 'your program being in an inconsistent
state' and 'this piece of code is in an inconsistent state
not because it contains any bugs but because a totally
different piece of code contains bugs'.

It's also why I like the 'bgerror' solution because it
avoids this inconsistency. (But your points about the event
loop are good, and mean we effectively have to rule that
solution out).

Now, it may well be that allowing this inconsistency is the
best way forward, trading it off for better debugging (i.e.
receiving an error message), but it would be nice if we can
find a more consistent approach.




----------------------------------------------------------------------

Comment By: Will Duquette (duquette)
Date: 2004-08-26 00:48

Message:
Logged In: YES 
user_id=372859

Vince said: "So, as I've said, I'm all for not swallowing errors, but I'm
not very keen on inconsistent states either."

Precisely.  If a bug in a rename trace callback causes an error to be 
thrown, and it gets swallowed, then very likely your program is now in 
an inconsistent state.

That is, it's in an inconsistent state whether the error is swallowed or 
not--but if the error isn't swallowed, the developer is more likely to catch 
it and fix it before the software is released.


----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2004-08-25 15:35

Message:
Logged In: YES 
user_id=32170

I agree it is undesirable to swallow errors, but the
suggested approach can still lead to inconsistencies. 
Looking closely, the current variable tracing approach also
has a similar problem, for example:

set foo 3
trace add variable foo write {puts "1: foo is $foo" ; #}
trace add variable foo write {set foo 3 ; error "don't
change" ; #}
trace add variable foo write {puts "2: foo is $foo" ; #}

If I now try to 'set foo 2', the last trace fires and thinks
that foo is 2, but then the next trace actually sets foo to
3 and throws an error, aborting the change.

The upshot is that one of the traces was confused, and
believed the variable was set to 2 when it wasn't.

A similar, but more fundamental problem will occur with
rename traces, if we allow them to be aborted by an error. 
One trace can think something has been renamed, but then the
next trace aborts the rename.  Now we are in an inconsistent
state.

So, as I've said, I'm all for not swallowing errors, but I'm
not very keen on inconsistent states either.


----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-08-25 15:27

Message:
Logged In: YES 
user_id=79902

There are a few other places where errors are silently
gobbled and IMO they're all bugs.

----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2004-08-25 15:20

Message:
Logged In: YES 
user_id=80530


I think I have to disagree on part of that.

It's a property of Tcl commands that they
can be renamed.  A package that wants
to prevent the renaming of its commands
seems buggy to me.

Generally, though, I agree that it's best
to avoid swallowing and silently hiding errors
when we can.


----------------------------------------------------------------------

Comment By: Will Duquette (duquette)
Date: 2004-08-25 15:15

Message:
Logged In: YES 
user_id=372859

Vince,

Using bgerror really only works well if you're running an app with an 
event loop.  There are lots of Tcl applications that don't, and Snit (my 
particular interest) can be used in both.  So I can't just use bgerror to 
flag errors in Snit's rename trace callback.

You ask "which of these two is it?"  Am I trying to discover programming 
errors, or am I trying to signal conditions that a programmer might wish 
to catch?

The answer is, "both".  I like Tcl's error handling; I really dislike having 
code in which errors can occur silently.  For that reason, I'm 
philosophically against the current implementation, as it's the one place 
(that I'm aware of) in which errors are ignored without recourse.  
(Bgerror is not a real recourse, for the reason I stated above.)

But in any event, it's a false dichotomy.  If I have a module which--for 
whatever reason--contains commands that should not be renamed, it 
seems reasonable to use a rename trace to catch attempts to rename 
them, and throw an error if they do, just as some folks use variable 
traces to control what can be assigned to particular variables.  If errors 
are thrown in these cases, it's probably not a "condition which a 
programmer might wish to catch and workaround"; it's a logical error in 
the program that has to be fixed.

Will

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2004-08-25 10:29

Message:
Logged In: YES 
user_id=32170

Will,

If your purpose is just to discover programming errors, then
that's one thing (which could also be handled by dgp's
suggestion of calling bgerror with the problem -- in fact
calling bgerror might be better, because the programmer
would not be tempted to use 'catch' to hide the problem).

However, earlier you gave an example of using a trace in
tcllib to prevent renaming of an structure. That's certainly
not just to discover programming errors. It's to use the
error to signal a certain condition, which a programmer
might wish to catch and workaround.

So which of these two is it?


----------------------------------------------------------------------

Comment By: Will Duquette (duquette)
Date: 2004-08-24 01:24

Message:
Logged In: YES 
user_id=372859

By "propagate normally", I mean that if 

   rename SomeName SomeOtherName

causes a trace to throw an error, then the error propagates up and is
returned by "rename".  I think the concern about multiple traces is
misleading.  After all, why would a rename callback throw an error? 
I can think of two reasons: 

1. The callback threw an error explicitly, becausing renaming the 
    command will cause the program to misbehave in mysterious
    ways.

2. There's an error in the callback, meaning it didn't do what it was
    supposed to do, meaning that the program is likely to misbehave
    in mysterious ways.

In either case, there's probably no good way to recover from the
error; it needs to be fixed.  Better that the programmer find out
ASAP, while they are developing the software.  But as it is, 
such errors hide and can be monstrously difficult to debug.

Regarding using a more permanent token rather than the command
name--well, yes, obviously.  If you can.



----------------------------------------------------------------------

Comment By: Don Porter (dgp)
Date: 2004-08-23 18:14

Message:
Logged In: YES 
user_id=80530


Is the real problem here one of using
a tranisient, user-modifiable thing -- the
command's name -- rather than a more
permanent token of some kind as the
index against which to store data?

Simliar to having a Tcl_ObjCmdProc
that changes behavior based on
the value of objv[0] rather than storing
and behavior controlling value in
the ClientData ?

Not that I wouldn't like a sensible solution
to this problem, but the motivating example
here might be a subtle instance of
pilot error.

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2004-08-23 14:10

Message:
Logged In: YES 
user_id=32170

In Will's example, I would've said a more obvious use of
rename traces in struct::stack would be to rename the array
entries so that the code still works after rename.

I still don't quite understand _exactly_ what Will (or
anyone) means by 'if ... propagates normally'?

What happens if one rename trace succeeds but then the next
one throws an error?  In Will's example, let's say there
were two traces. Now, if the first trace succeeds (causing
some side effect of renaming appropriate associated
information), but a second trace fails, then the original
rename will not take place and the first trace will actually
have done the wrong thing. 

Can someone please define a consistent behaviour, if they
have one in mind that is different to what we have now?




----------------------------------------------------------------------

Comment By: Donal K. Fellows (dkf)
Date: 2004-08-23 11:42

Message:
Logged In: YES 
user_id=79902

One possibility I've used elsewhere would be to report
traces using the background error message mechanism. As long
as people have a [bgerror] command defined (and are running
the event loop), the error message will go somewhere where
it can be picked up...

----------------------------------------------------------------------

Comment By: Will Duquette (duquette)
Date: 2004-08-21 19:14

Message:
Logged In: YES 
user_id=372859

Here's an example of where the current code causes a problem: in object 
code (e.g., Tcllib's struct::stack) the object name is often used as an 
index into array in which the specific object's data is stored.  If the object 
is renamed, this breaks.  An obvious use of a rename trace is to throw 
an error if the object is renamed.  At present, you can't do that and 
make it stick without installing a modified version of the "rename" 
command.

Look at it another way--if errors in rename and delete callbacks  
propagated normally, the programmer could achieve the current behavior 
using "catch".  With the code as it is, the programmer has no choice.

This is not academic, either; Snit relies on the rename/destroy callbacks.

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2003-06-26 14:43

Message:
Logged In: YES 
user_id=32170

Here are the basic options:

(i) If rename/delete always delete the command/proc in 
question. This implies either all errors in traces must be 
ignored, or an error in a trace stops the execution of any 
other trace, but lets the delete still happen afterwards 
anyway. In any case, if the command in question has 
a 'Tcl_CmdDeleteProc' which itself wants to create an error 
message, that error would overwrite any error thrown by a 
trace.

(ii) If rename/delete are (like variable 'unset' traces) allowed 
to fail with an error, then things are conceptually much 
cleaner: the first trace which throws an error aborts the 
operation. However, this is an overall change in Tcl's 
documented behaviour, and would require quite careful coding 
to get right.


----------------------------------------------------------------------

Comment By: Hemang Lavana (hemanglavana)
Date: 2003-01-17 13:18

Message:
Logged In: YES 
user_id=81875

I think it should bail out at the first occurence of an
error in a callback proc. Not doing so will make it very
difficult to find/debug such errors. Moreover, execution
traces bail out as soon as an error is encountered and the
behavior should be the same for rename/delete traces as well.

Do we need a tip to make this change? I already have a patch
to fix this ... I just need to add few test cases to
trace.test and update the docs also.

----------------------------------------------------------------------

Comment By: Vince Darley (vincentdarley)
Date: 2003-01-17 11:32

Message:
Logged In: YES 
user_id=32170

This is true, and the code quite explicitly ignores all
errors. This is not documented however.  I quite open about
what the correct behaviour should be (as long as we then
document it), but there is one major problem with not
swallowing errors: rename and delete operations are defined
to succeed always, irrespective of any errors thrown. So,
what do we do if there are two traces, each of which throw
an error? We discard the first error message?  What if the
first trace throws an error and the second is ok?  Do we
still discard the error?  In some ways the current behaviour
is the most consistent.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=110894&aid=652765&group_id=10894