homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: traceback: add a new thin class storing a traceback without storing local variables
Type: enhancement Stage:
Components: asyncio, Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: adaptivelogic, eric.snow, gvanrossum, mahmoud, martin.panter, martius, ncoghlan, ned.deily, pitrou, python-dev, rbcollins, rhettinger, terry.reedy, vstinner, xonatius, yselivanov
Priority: normal Keywords: patch

Created on 2013年05月05日 21:31 by gvanrossum, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
traceback.patch adaptivelogic, 2013年07月11日 21:15 review
traceback2.patch adaptivelogic, 2013年07月12日 15:13 Better documentation, fixed edge cases review
linecache_1.patch rbcollins, 2015年01月14日 01:37
linecache_2.patch rbcollins, 2015年01月19日 02:09 review
frame_1.patch rbcollins, 2015年01月19日 02:09 review
tb_stats_1.patch rbcollins, 2015年01月19日 02:12
issue17911-1.patch rbcollins, 2015年01月20日 02:38 review
issue17911-2.patch rbcollins, 2015年01月26日 03:18 review
issue17911-3.patch rbcollins, 2015年01月26日 21:48 review
issue17911-4.patch rbcollins, 2015年02月03日 23:28 review
qualname_in_exceptions.patch xonatius, 2015年02月07日 08:23 Prints qualnames in function call exceptions. review
issue17911-5.patch rbcollins, 2015年03月03日 22:24 review
issue17911-6.patch rbcollins, 2015年03月03日 22:56 review
issue-19711-8.patch rbcollins, 2015年03月05日 02:20 review
Messages (77)
msg188467 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013年05月05日 21:31
For Tulip I may have to extract tracebacks and store the extracted data, in order to avoid cycles created by the frames referenced in the traceback. I'll be extracting the traceback every time an exception is raised, and in most cases I won't be using the extracted info, because usually the exception is caught (or logged) by user code. But it's very important to ensure that if the user code doesn't catch or log it, I can log a traceback, and I won't know that this is the case until a destructor is called, which may be quite a bit later. (Reference: http://code.google.com/p/tulip/source/browse/tulip/futures.py#38)
Unfortunately it looks like the public APIs do a lot more work than needed. Ideally, there'd be something similar to _extract_tb_or_stack_iter() that doesn't call linecache.getline() -- it should just return triples of (filename, lineno, functionname), and enough structure to tell apart the __cause__, __context__, and __traceback__ (the first two possibly repeated). Given this info it would be simple enough to format and log the actual traceback, and storing just this would take less space and time than computing the lines of the fully-formatted traceback.
msg188530 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年05月06日 13:06
I think instead we may want to add a finalize() or close() method on frame objects which would clear all local variables (as well as dereference the globals dict, perhaps), after having optionally run a generator's close() method (if the frame belongs to a generator).
If I'm not mistaken, it should allow breaking reference cycles, and remove the need for complex traceback processing, which Twisted currently also does: http://twistedmatrix.com/trac/browser/trunk/twisted/python/failure.py#L89
Note that generator cleanup through the frame has a patch in issue17807.
msg188548 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013年05月06日 14:39
I'm not snubbing my nose at something that breaks these types of cycles more easily, but I still think that the abstractions offered by the traceback module could be improved.
msg188554 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013年05月06日 14:46
On Mon, May 6, 2013 at 6:06 AM, Antoine Pitrou <report@bugs.python.org> wrote:
> Note that generator cleanup through the frame has a patch in issue17807.
Sadly that doesn't help with my issue. I applied path gen3.patch and
ran various versions of the code I have. There's still something
holding on to the Task or the exception.
msg188556 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年05月06日 14:53
> On Mon, May 6, 2013 at 6:06 AM, Antoine Pitrou
> <report@bugs.python.org> wrote:
> > Note that generator cleanup through the frame has a patch in
> > issue17807.
> 
> Sadly that doesn't help with my issue.
It won't. It's just a building block for the change I've proposed here.
msg188616 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2013年05月07日 01:42
Antoine - I like your idea, but I think it's a separate issue. I agree with Guido that exposing some lower level non-formatting APIs in the traceback module would be helpful.
I see Guido's suggestion here as similar to the change we just made in the dis module to expose a dis.get_instructions iterator. That change makes it much easier to work with the disassembler output programmatically, whereas the module was previously too focused on displaying text with a specific format.
My current thoughts: define a "TracebackSummary" with the following fields:
 exc_type
 exc_repr
 stack_summary
 cause
 context
stack_summary would be a list of (filename, lineno, functionname) triples as Guido suggested (probably a named tuple)
cause and context would be either None or a reference to an appropriate TracebackSummary object
Basically, the summary should contain all of the information needed to create the formatted traceback output, without actually doing any formatting (aside from calling repr() on the exception)
msg188620 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013年05月07日 03:39
That sounds great, except I'd expect the exception type and str(), since a formatted traceback uses the str() of the exception, not its repr().
Personally I don't think the tuples need to be named, but I won't hold up progress either. :-)
msg188648 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年05月07日 13:13
You may want two pieces of stack: the piece of stack that is above and including the catch point, and the piece of stack that is below it. The former is what gets traditionally printed in tracebacks, but the latter can be useful as well (see issue1553375 for a related feature request).
msg188719 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年05月08日 11:55
Ok, I've spinned off the frame clearing suggestion to issue17934.
msg192672 - (view) Author: Björn Sandberg Lynch (adaptivelogic) Date: 2013年07月08日 17:04
I've been looking into this as an easy piece to bite off. If I understand Guido correctly, he'd like to defer or suppress the linecache call when getting the tb summary. The problem with deferring is that you need to access f_globals for the loader to work correctly when the module source is a non-file import source. If we keep a reference to f_globals for each line in the traceback, we can defer this to later (ideally we'd just keep the __loader__ value, but that would require changing the linecache interface as well).
My inclination would be to have another keyword argument to _extract_tb_or_stack_iter (terse=False or verbose=True - either style works). In terse mode, no source lines would be available, and the formatted output would be the same as if the source wasn't available at all. This would work, although the traceback module is structured so that I'd need to pass it through quite a few wrapped iterator calls.
I'm not sure how free a hand I have when it comes to refactoring the internal implementation. I'm not fond of the extractor callbacks - I'd prefer a generator-based approach on the lines of:
def _tb_iter(tb, limit):
 i = 0
 while tb is not None:
 if limit is not None and limit < i:
 break
 yield tb.tb_frame, tb.tb_lineno
 tb = tb.tb_next
 i += 1
def _extract_frame_iter(frames, terse=False):
 ...
 for frame, lineno in frames:
 ...
msg192690 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013年07月08日 20:07
In terms of how much freedom you have about changing the internal, I'd check how long ago they were changed. "Internal" APIs that have been stable for many versions tend to have illicit external uses -- but internal APIs that were introduced recently (e.g. in 3.2) are usually safe to use -- nobody is going to make too much of a stink if you break their code.
As for saving f_globals, if you're going to save an extra pointer anyways, why not just save the frame pointer?
msg192728 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年07月09日 08:32
> If we keep a reference to f_globals for each line in the traceback, we 
> can defer this to later (ideally we'd just keep the __loader__ value,
> but that would require changing the linecache interface as well).
Or you could add a *new* linecache interface.
Keeping __loader__ sounds much less hackish than keeping f_globals to me.
msg192909 - (view) Author: Björn Sandberg Lynch (adaptivelogic) Date: 2013年07月11日 21:15
After thinking about it, I decided to defer instead of suppress line fetching. Suppressing would still give a traceback later, but not the same as the original.
extract_exception is where the meat is. There's a slight quirk in how context works - if cause is set to something else than the actual exception, context still gets set even though it won't be printed. This is to enable later analysis. However, if you explicitly 'raise Exception from None' neither of them will be set - the formatting code would have issues otherwise.
I'm not too happy about the code duplication between the format_exception method and the formatting that works on exceptions directly, but there are plenty of tests to ensure that the output is identical.
Feel free to dispute my naming choices. I'm admittedly influenced by twisted.
msg193272 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年07月18日 09:42
I think ExceptionSummary could be the visible API, instead of adding an indirection through a separate global function.
Also, I don't think having the "exc_traceback" list of tuples is future-proof. What if we want to add some information to each traceback entry, or refactor it to take __loader__ into account?
Instead, ExceptionSummary could expose the desired operations (e.g. iterate over traceback lines and the associated source code lines) without being constrained by some implementation details.
msg193294 - (view) Author: Björn Sandberg Lynch (adaptivelogic) Date: 2013年07月18日 13:35
I was trying to stay with the established pattern of the existing methods. There are two unrelated issues to solve here - deferring linecache access, and the extract_exception functionality.
When it comes to deferral, we could wrap each line in a different class than tuple, but this constitutes a public API change (and this is a *very* widely used library). Even changing to a namedtuple would cause a lot of grief, since we'd break lots of doctests, and subclassing tuple is a lot of effort for little benefit. If we only do it for the deferred usage, it would "only" be inconsistent. I suppose we could have a completely separate way of doing things for ExceptionSummary, but again, inconsistent, and I think if one extract_xxx method provides the functionality, users would expect it to be available in the others.
For ExceptionSummary, the same summary instance is used more than once if you have a cause set, so object creation had to be separated from the graph population. Either this is done in a constructor function or we would need some obscure tricks with the class kwargs. This way, there's a separation of concerns - the class wraps *one* exception, and the function creates a bunch and links them together as needed.
msg193296 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年07月18日 13:45
> When it comes to deferral, we could wrap each line in a different
> class than tuple, but this constitutes a public API change (and this
> is a *very* widely used library). Even changing to a namedtuple
> would cause a lot of grief, since we'd break lots of doctests, and
> subclassing tuple is a lot of effort for little benefit. If we only
> do it for the deferred usage, it would "only" be inconsistent. I
> suppose we could have a completely separate way of doing things for
> ExceptionSummary, but again, inconsistent, and I think if one
> extract_xxx method provides the functionality, users would expect it
> to be available in the others.
YMMV, but I think we should go for inconsistency here. The other APIs
in the traceback module are low-level and clearly limited by the type
of objects they return.
This feature request is a good opportunity to design something a little
more future-proof. I'd love to know what other developers/contributors
think here.
msg193333 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2013年07月18日 23:06
For dis, we introduced a new rich bytecode introspection API. Ditto for
inspect.Signature. I think it makes sense to treat this as a completely new
traceback introspection API and ignore the low level details of the legacy
API.
msg220381 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014年06月12日 20:11
> I think it makes sense to treat this as a completely new
> traceback introspection API and ignore the low level details
> of the legacy API.
That would likely be the cleanest approach.
msg220390 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014年06月12日 21:48
While trying to fix a similar issue for the asyncio project, I wrote the following code:
https://bitbucket.org/haypo/misc/src/ce48d7b3ea1d223691e496e41aca8f5784671cd5/python/suppress_locals.py?at=default
I was not aware that a similar approach (attached traceback2.patch) was already implemented.
Asyncio issues:
https://code.google.com/p/tulip/issues/detail?id=155
https://code.google.com/p/tulip/issues/detail?id=42
See also the Python issue #20032.
msg231743 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2014年11月27日 02:47
Belatedly looking at this again: one thing I noticed is that the summary currently loses info if __cause__ is set.
It would be better to follow the PEP 409/415 model and report the "__suppress_context__" flag as part of the summary, rather than dropping the context information.
That allows the decision on whether or not to show the context information to be deferred to display time, rather than discarding it eagerly.
msg231792 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2014年11月28日 00:56
I'll put something together.
msg233895 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月12日 22:53
w.r.t. a new linecache interface, it looks like we need two attributes from f_globals: __name__ and __loader__, so that we can eventually call __loader__.get_source(__name__).
One small change (to let me focus on traceback) would be to add another kw argument to the existing calls that take module_globals, called e.g. get_source, which would be a simple callable (source = get_source()). For the deferred linecache situation, we'd then create partial(f_globals.__loader__.get_source, f_globals.__name__) and keep that around until we need the source. We could even abstract that out into a function in linecache to keep the knowledge in one place.
Another way to tackle this would be to add a new function to linecache that lazily seeds the cache: it would stash the get_source method and the name against the filename, and when getline[s] is called actually invoke it.
I think the second way is a bit nicer myself. What do folk think?
msg233995 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月14日 01:37
Ok, here's a draft patch for linecache. Next up, poking around the new TB API.
msg234297 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月19日 01:54
One thing I noticed while working on this, traceback today stats each included file once per frame per call into e.g. format_list. This might actually account for quite some of the overhead. I'll include an optimisation for this in my new API. http://paste.openstack.org/show/158714/
http://paste.openstack.org/show/158715/ 
msg234298 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月19日 02:09
This is a WIP patch, including it to just share progress.
msg234299 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月19日 02:12
And this improves the scaling of the cache stating overhead, but we may well want to fix this more fundamentally - e.g. by linking the cache ownership into the import system somehow, so that when a file is reimported, the cached source is automatically evicted, and format_stack etc have no need to stat at all.
I tested this with the following script in timeit:
import traceback
def recurse(count):
 if count> 0:
 return recurse(count - 1)
 return traceback.format_stack()
def doit():
 len(recurse(500))
msg234319 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月19日 17:56
I've split out the stat question to http://bugs.python.org/issue23273 - we can optimise it slightly in this patch, but I think its scope creep here, and will be unclear, to dive after a full fix in this issue.
msg234338 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月20日 02:38
Stack and Frame looking good, next update will be next Monday, when I finish off my TracebackException class.
msg234712 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 03:18
Right, and a usable API. 
I believe that this will meet Guido's use case:
tb = TracebackException(*sys.exc_info, lookup_lines=False)
....
some time later
....
if should_show_tb:
 lines = list(tb.format())
I'm not 100% sold on the public API being a generator, but rather than forcing it one way or the other, I'll let reviewers tell me what they think :)
Performance wise, this is better, with the following times:
format_stack -> 5.19ms
new API to extract and not format -> 3.06ms
new API to extract, not lookup lines and not format -> 2.32ms
Formatting is then 2-3ms per 500 line traceback actually formatted, which seems terribly slow, but its still better than the 8+ms trunk takes (see my earlier tests). I'll look at tuning the time to render an actual trace later, since I don't like paying high costs in unittest ;) - but AIUI this should be enough to help asyncio as is.
Updated test script I used to isolate times with timeit:
import traceback
def recurse(count, lookup_lines=True):
 if count> 0:
 return recurse(count - 1, lookup_lines=lookup_lines)
 if lookup_lines:
 return traceback.Stack.extract(traceback.walk_stack(None), lookup_lines=True)
 else:
 return traceback.Stack.extract(traceback.walk_stack(None), lookup_lines=False)
def doit():
 len(recurse(500))
def doit_lazy():
 len(recurse(500, False))
msg234740 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年01月26日 13:12
I put some detailed comments in the review, but my main large scale concern is with the "traceback.Frame" name. Making "frame object" an ambiguous phrase seems like a bad idea to me, so I've suggested making it "FrameSummary" (or "FrameInfo", but on reflection I prefer the more explicit term). That would then suggest also making it "traceback.StackSummary" for consistency.
I think having the top level API be an iterable would be OK, but may be a little surprising. Is there a major advantage to avoiding building the whole traceback in memory?
msg234741 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年01月26日 13:38
> tb = TracebackException(*sys.exc_info, lookup_lines=False)
You don't need an exception tuple in Python 3, the exception instance is enough.
msg234765 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 18:36
Thanks for the review - shall action it all as it seems all good improvements.
Two key notes: the use of an exception triple is useful to ease backports of this: a primary goal for me is being able to use the locals stuff in unittest for existing production code bases like OpenStack: there's a balance to be struck between new shiny features like __traceback__ and backporting. I'll consider the idea of value-or-triple; if all the consuming code is going to be using the triple interface I'm not sure the value one would, drumroll please, have value.
The locals stuff is as you note not complete or exposed yet. I wanted it in the design to be able to vet that it will actually be feasible without a later API break. I'm not -quite- there but I do also agree that it doesn't need to be in the output layer yet (in fact there's a different ticket for that anyway).
msg234769 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年01月26日 18:59
Le 26/01/2015 19:36, Robert Collins a écrit :
> 
> Two key notes: the use of an exception triple is useful to ease
backports of this
Then I'd recommend making it exception-or-tuple (rather than a
*exc_info), so that both situations are satisfied.
msg234777 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 20:44
The generator thing: the code was refactoring not so long ago to be generator based internally with lists as a UI shim. I don't think there is a major advantage either way. Less buffering on one hand. Less convenient in some cases on the other. Simple use like ''.join(foo.format()) is going to be unaffected.
msg234781 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 21:48
And for profit, review changes applied (minus the small number I disagreed with). I've clarified in the code why the exc_info tuple break out is still used (compat with the legacy API is the strongest argument).
I haven't fleshed out the locals thing properly yet, I'm going to do one more iteration on that then ask for a final review before committing. (Unless we're in freeze ATM ?)
msg234782 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 22:04
actually, strike that, I'm happy with this pending a final +1 from another reviewer. Finishing the locals stuff is a separate patch, and will look like a single new parameter to StackSummary.extract, similarly on TracebackException.__init__ and then change the format logic to show it.
@Antoine - I'm not super happy with inspecting the first arg to do type-or-value stuff, if you feel its a super issue we can do it, but I'd rather wait and add it once we've got some feedback.
msg234788 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年01月26日 22:46
On the triple-or-value front, the main case I see for considering the "you don't need exception state triples any more" argument already lost is the fact exc_info still returns a triple, and __exit__ methods still accept them as unpacked arguments. The C level exception handling APIs are also all still exception state triple based. In this view, __traceback__ is primarily about accessing *chained* tracebacks, while the top-level APIs still focus on exception state triples.
This perspective also has the virtue of confining use of the Python 3 only exception traceback attribute to code that is taking advantage of a Python 3 only feature (i.e. exception chaining). For chaining independent exception state handling, working with exception state triples then remains the "one obvious way to do it", a way which is conveniently also source compatible between Python 2 and 3.
msg234795 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年01月26日 23:22
> In this view, __traceback__ is primarily about accessing *chained*
> tracebacks
Please let's not make up sophistic arguments when the only motivation for triples is backwards compatibility of *existing* APIs.
Exception triples are an unnecessary bother in Python 3. This new API should accept exception instances primarily. Accepting triples as well is ok but it shouldn't be made the only possibility.
msg234796 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月26日 23:30
So its fairly simple IMO: it will be more code, not less, to support the non-triple API, *and* it can be added later, unless we're proposing not to support the triple API at all (which hasn't been proposed AFAICT).
To me thats a fairly strong argument for adding non-triple support later.
msg234797 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年01月26日 23:34
Le 27/01/2015 00:30, Robert Collins a écrit :
> 
> Robert Collins added the comment:
> 
> So its fairly simple IMO: it will be more code, not less, to support
the non-triple API, *and* it can be added later, unless we're proposing
not to support the triple API at all (which hasn't been proposed AFAICT).
I am proposing it. Passing a single exception instance is the natural
way of expressing such an API. Crippling Python 3 APIs for the purpose
of backwards compatibility is not ok.
Nobody wants to write some_func(exc.__class__, exc, exc.__traceback__)
when they could simply write some_func(exc).
msg234804 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年01月27日 01:05
Why do you consider it crippling?
msg234812 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年01月27日 08:33
It's tedious and complicated compared to the natural way.
msg234820 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年01月27日 13:23
Most of the time when I'm working heavily with exceptions it's something related to contextlib, so I'm likely getting my exception details from either sys.exc_info() or the arguments to __exit__. That means I start out with an exception triple, and the only time I need to look at type(exc) or exc.__traceback__ is when I'm following exception chains.
However, Antoine's right that if you got your exception from an *except clause* rather than one of the more indirect APIs, then you're just going to have the exception, rather than an exception triple.
So I think that's where the argument for accepting "exception-or-triple" comes from: handling an exception directly is for use with except clauses, while handling triples is convenient for use with sys.exc_info(), __exit__ methods and for general backwards compatibility.
msg235365 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月03日 23:28
This iteration provides two constructors for TracebackException, one for exception objects, one for exc_info tuples. So it should be easy to use.
The __init__ takes the exc_info tuple because thats less code (much easier to destructure rather than restructure), but in the docs and docstrings I point to the classmethods, which includes from_exception - that takes a single exception object.
msg235428 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年02月05日 10:49
+1 for having the base __init__ API be the exception triple and then a TracebackException.from_exception() class method as an alternate constructor.
However, the redundancy between TracebackException.__init__() and TracebackException.from_exc_tuple() feels very strange to me. It leaves the __init__ working-but-undocumented and isn't a pattern I can recall seeing anywhere else in the standard library.
Would it be possible to just add the from_exception() class method and leave the documented (and only) API for building from a triple being the normal constructor call?
msg235444 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月05日 18:45
I can certainly do that; I was aiming to make it fair I guess, given Antoines strong feelings on this matter. As long as I'm not piggy-in-the-middle, I don't have a care in this regard :)
msg235446 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年02月05日 19:09
I just posted some review comments.
I also realized that it posted some older comments that I hadn't submitted before... some of them may be obsolete, sorry :-/
msg235455 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年02月05日 22:18
I think in this case, the fact that it's easier to decompose an exception
into the corresponding triple rather than vice-versa, together with the
fact that other exception state manipulation APIs like exc_info() and
__exit__() methods work with triples, means it makes sense to accept the
triple as the base constuctor API, and then have the convenience method to
destructure a caught exception.
msg235456 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年02月05日 22:19
The solution of having two constructor classmethods actually seems nice
and symmetric to me.
msg235472 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年02月06日 08:34
It's genuinely user hostile, as the "from_exc_tuple" version is entirely
redundant, and folks using pydoc to discover the API won't know they
"shouldn't" use the constructor directly.
That means both forms will get used, but only one will be documented. When
people ask why a second spelling for the lower level API was added, "to
make it look like it exists at the same level as the higher level
convenience API" isn't going to be a satisfying explanation.
msg235480 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年02月06日 12:43
Le 06/02/2015 09:34, Nick Coghlan a écrit :
> 
> It's genuinely user hostile, as the "from_exc_tuple" version is entirely
> redundant, and folks using pydoc to discover the API won't know they
> "shouldn't" use the constructor directly.
Using pydoc isn't generally a successful way of discovering an API. It
shows you too many details that don't matter. Besides our docstrings are
much terser and less informative than the Sphinx doc.
pydoc is useful mostly as a Python-specific "man" command: you know
something exists, just want to remember the details.
msg235483 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年02月06日 13:31
dir() will get me TracebackException as a name, help(traceback.TracebackException) will get me its signature. IDEs with autocomplete and signature tooltips will do the same.
There is nothing in those usage sequences to say "don't use __init__, use this redundant class method with the same signature instead".
I agree providing a "directly from an exception" constructor is essential for cases where you're working with a caught exception at the Python level. I don't agree with the idea of duplicating the required low level API under a different name so it doesn't *look* like a lower level API in the documentation.
msg235517 - (view) Author: Daniil Bondarev (xonatius) * Date: 2015年02月07日 08:23
As suggested adding patch from http://bugs.python.org/issue2786 which prints qualnames in function call exceptions.
e.g:
>>> class A:
... def __init__(self):
... pass
>>> A(1)
Traceback (most recent call last):
 File "<stdin>", line 1, in <module>
TypeError: A.__init__() takes 1 positional argument but 2 were given
msg235569 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月08日 21:59
Sorry xonatius I wasn't clear: AFAICT your patch is going to require changes to the traceback tests, and this issue is changing the implementation substantially: I was suggesting that you make sure your patch applies on top of this issue, not that you merge them :)
msg235570 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月08日 22:01
Nick, Antoine - I'm now stuck between you. Options going forward:
 - I can JFDI realising you won't both be happy :)
 - you two can reach consensus!
I could cripple __init__ by switching to __new__, but I think thats massive overcomplication and not needed.
I can of course add more text to the pydoc prose saying 'do not use __init__' if that would address Nicks concern.
msg235574 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015年02月08日 22:42
Escalating to python-dev for API design feedback is the usual path forward
if we reach an impasse in the tracker comments.
I'll make one more attempt at persuading Antoine here though: the fact that
we're being tempted to add "do not use this API the way you would normally
expect to use a Python API, even though it works exactly as you might
expect" to the docs is a big red flag for me :)
There's a split between the "low level API that exposes implementation
details" (the exception state triple) and "high level API that hides them"
in the current patch, and Antoine is entirely correct that we previously
omitted the latter. It doesn't follow for me that we should also hide the
fact that the higher level convenience API is implemented in terms of the
lower level more implementation oriented one.
msg235578 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015年02月09日 00:11
You can also switch back to a single constructor, taking either an
exception instance or an exception triple. That way everyone is happy -
except perhaps if you are ideologically opposed to polymorphic
signatures :-)
Switching to python-dev would probably be overkill for this.
msg235658 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月10日 02:08
I'm idealogically opposed to polymorphic interpretation of args :)
Antoine, will you be ok with one __init__ and one classmethod?
msg235720 - (view) Author: Mahmoud Hashemi (mahmoud) * Date: 2015年02月10日 23:08
Hey all, great to see this being worked on so diligently for so long. Having worked in this area for a while (at home and at PayPal), we've got a few learnings to share:
1) linecache is textbook not-threadsafe. For example, https://hg.python.org/cpython/file/default/Lib/linecache.py#l38
For a lightweight traceback wrapper to be concurrency-friendly, we've had to catch KeyErrors, like so: https://github.com/mahmoud/boltons/blob/master/boltons/tbutils.py#L115
It's kind of a blanket approach, but maybe we could make a separate issue and help out with a linecache refresh?
2) We use something like (filename, lineno) in our DeferredLine class, but for very lightweight areas (e.g., greenlet creation) we just save a reference to the code object, as the additional attribute accesses do end up showing up in the profiles.
3) Generally we've found the APIs in TracebackInfo here to be pretty sufficient/functional: 
https://github.com/mahmoud/boltons/blob/master/boltons/tbutils.py#L134
Let me know if you've got any questions on that, and keep up the good work!
msg235872 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年02月13日 03:07
@Mahmoud thanks! I had a quick look and the structural approach we've taken is a bit different. What do you think of the current patch here (issue17911-4.patch) ?
msg237166 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月03日 22:24
Ok, all changes applied, lets see how this looks to folk.
msg237217 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015年03月04日 23:08
New changeset 73afda5a4e4c by Robert Collins in branch 'default':
Issue #17911: traceback module overhaul
https://hg.python.org/cpython/rev/73afda5a4e4c 
msg237220 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015年03月04日 23:26
New changeset 7cea10917f40 by Robert Collins in branch 'default':
Fix brownbag in issue 17911 commit
https://hg.python.org/cpython/rev/7cea10917f40 
msg237224 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2015年03月05日 01:32
With 7cea10917f40 applied, multiple tests are now failing on most buildbots. For example, on OS X 10.10, test_cgitb test_code_module test_decimal:
test_blanks (test.test_cgitb.TestCgitb) ... ok
test_fonts (test.test_cgitb.TestCgitb) ... ok
test_html (test.test_cgitb.TestCgitb) ... ERROR
test_syshook_no_logdir_default_format (test.test_cgitb.TestCgitb) ... ok
test_syshook_no_logdir_text_format (test.test_cgitb.TestCgitb) ... ok
test_text (test.test_cgitb.TestCgitb) ... ERROR
test_banner (test.test_code_module.TestInteractiveConsole) ... ok
test_cause_tb (test.test_code_module.TestInteractiveConsole) ... ERROR
test_console_stderr (test.test_code_module.TestInteractiveConsole) ... ok
test_context_tb (test.test_code_module.TestInteractiveConsole) ... ERROR
test_ps1 (test.test_code_module.TestInteractiveConsole) ... ok
test_ps2 (test.test_code_module.TestInteractiveConsole) ... ok
test_syntax_error (test.test_code_module.TestInteractiveConsole) ... ERROR
test_sysexcepthook (test.test_code_module.TestInteractiveConsole) ... ERROR
**********************************************************************
1 items had failures:
 1 of 4 in decimal.Context.create_decimal_from_float
See also, for example, the Debian buildbot where, in addition, test_fractions failed:
http://buildbot.python.org/all/builders/AMD64%20Debian%20PGO%203.x/builds/140/steps/test/logs/stdio 
msg237225 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 01:36
Thats really strange, I did a ./python -m test run before committing - the brown bag was due to me running with the patch for 22936 also applied. Looking into the failures reported now.
msg237228 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 01:53
Ok, cgitb - its passing a string instead of an exception type into format_exception - something that was never supported - it only works by accident AFAICT, because the old format code was ignoring the etype - it was deriving the type from the value. Thats easy to accomodate in the compat code.
msg237229 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 01:54
Also apologies - ned told me on IRC that python -m test -uall is needed, not just -m test. Doh.
msg237230 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015年03月05日 01:54
There are failures on buildbot.
http://buildbot.python.org/all/builders/AMD64%20Ubuntu%20LTS%203.x/builds/5651/steps/test/logs/stdio
http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5783/steps/test/logs/stdio
http://buildbot.python.org/all/builders/PPC64%20PowerLinux%203.x/builds/3109/steps/test/logs/stdio
Examples:
======================================================================
ERROR: test_cause_tb (test.test_code_module.TestInteractiveConsole)
----------------------------------------------------------------------
Traceback (most recent call last):
AttributeError
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 91, in runcode
 exec(code, self.locals)
 File "<console>", line 1, in <module>
ValueError
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
 File "/opt/python/3.x.langa-ubuntu/build/Lib/test/test_code_module.py", line 85, in test_cause_tb
 self.console.interact()
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 245, in interact
 more = self.push(line)
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 267, in push
 more = self.runsource(source, self.filename)
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 75, in runsource
 self.runcode(code)
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 95, in runcode
 self.showtraceback()
 File "/opt/python/3.x.langa-ubuntu/build/Lib/code.py", line 144, in showtraceback
 for value, tb in traceback._iter_chain(*ei[1:]):
AttributeError: module 'traceback' has no attribute '_iter_chain'
======================================================================
ERROR: test_html (test.test_cgitb.TestCgitb)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/test/test_cgitb.py", line 23, in test_html
 raise ValueError("Hello World")
ValueError: Hello World
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
 File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/test/test_cgitb.py", line 27, in test_html
 html = cgitb.html(sys.exc_info())
 File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/cgitb.py", line 189, in html
 ''.join(traceback.format_exception(etype, evalue, etb)))
 File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/traceback.py", line 109, in format_exception
 etype, value, tb, limit=limit).format(chain=chain))
 File "/home/shager/cpython-buildarea/3.x.edelsohn-powerlinux-ppc64/build/Lib/traceback.py", line 433, in __init__
 if exc_type and issubclass(exc_type, SyntaxError):
TypeError: issubclass() arg 1 must be a class
msg237231 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 02:01
code module is using a _function from within the traceback module to filter out a frame - we can do that with the new interface easily (it filters the first tem).
msg237232 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 02:04
The decimal failure is due to _format_final_exc_line now filtering out 'None' as well, because the string captured values of objects leads to None -> 'None' before we do rendering.
I think in this case its reasonable to change the behaviour (since None itself was already filtered out, its only the special case of a value of 'None' that worked previously).
msg237233 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月05日 02:20
Fixes for buildbots.
msg237237 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2015年03月05日 03:11
Much better, thanks!
msg237339 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015年03月06日 10:18
New changeset 648b35f22b91 by Berker Peksag in branch 'default':
Issue #17911: Tweak traceback documentation.
https://hg.python.org/cpython/rev/648b35f22b91 
msg237842 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015年03月11日 04:11
(correct new issue #) Idle is partially disabled in a2 (#23631), with symptoms similar to that of the first example in msg237230. Trivial test: open Idle Shell, enter '1/0'. If this is not fixed before .a3, I think it should be reverted until it is.
msg238169 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月16日 01:29
Looking at the regression now.
msg238248 - (view) Author: Robert Collins (rbcollins) * (Python committer) Date: 2015年03月16日 22:17
Regression fixed AFAICT, please re-open if not.
msg269464 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016年06月29日 05:00
Robert: in issue17911-2.patch (and the eventual commit) you added a check for value == 'None' in _format_final_exc_line(). Why was this necessary? I think it is the cause of my Issue 27348 regression:
>>> traceback.format_exception(Exception, Exception("One"), None)
['Exception: One\n']
>>> traceback.format_exception(Exception, Exception("None"), None)
['Exception\n']
History
Date User Action Args
2022年04月11日 14:57:45adminsetgithub: 62111
2016年06月29日 05:00:26martin.pantersetnosy: + martin.panter
messages: + msg269464
2015年08月17日 15:50:12vstinnerlinkissue23003 superseder
2015年03月16日 22:17:38rbcollinssetstatus: open -> closed
resolution: fixed
messages: + msg238248
2015年03月16日 01:29:06rbcollinssetmessages: + msg238169
2015年03月11日 04:11:37terry.reedysetmessages: + msg237842
2015年03月11日 04:10:45terry.reedysetmessages: - msg237840
2015年03月11日 03:57:20terry.reedysetnosy: + terry.reedy
messages: + msg237840
2015年03月06日 10:18:14python-devsetmessages: + msg237339
2015年03月06日 02:15:06ncoghlanlinkissue22936 dependencies
2015年03月05日 03:11:31ned.deilysetpriority: critical -> normal
nosy: gvanrossum, rhettinger, ncoghlan, pitrou, vstinner, rbcollins, ned.deily, python-dev, eric.snow, mahmoud, yselivanov, adaptivelogic, martius, xonatius
messages: + msg237237
2015年03月05日 02:20:55rbcollinssetfiles: + issue-19711-8.patch

messages: + msg237233
2015年03月05日 02:04:39rbcollinssetmessages: + msg237232
2015年03月05日 02:01:11rbcollinssetmessages: + msg237231
2015年03月05日 01:54:51vstinnersetmessages: + msg237230
2015年03月05日 01:54:30rbcollinssetmessages: + msg237229
2015年03月05日 01:53:04rbcollinssetmessages: + msg237228
2015年03月05日 01:36:48rbcollinssetmessages: + msg237225
2015年03月05日 01:32:46ned.deilysetpriority: normal -> critical
nosy: + ned.deily
messages: + msg237224

2015年03月04日 23:26:26python-devsetmessages: + msg237220
2015年03月04日 23:08:28python-devsetnosy: + python-dev
messages: + msg237217
2015年03月03日 22:56:40rbcollinssetfiles: + issue17911-6.patch
2015年03月03日 22:24:15rbcollinssetfiles: + issue17911-5.patch

messages: + msg237166
2015年02月13日 03:07:49rbcollinssetmessages: + msg235872
2015年02月10日 23:08:24mahmoudsetnosy: + mahmoud
messages: + msg235720
2015年02月10日 02:08:01rbcollinssetmessages: + msg235658
2015年02月09日 00:11:10pitrousetmessages: + msg235578
2015年02月08日 22:42:43ncoghlansetmessages: + msg235574
2015年02月08日 22:01:35rbcollinssetmessages: + msg235570
2015年02月08日 21:59:50rbcollinssetmessages: + msg235569
2015年02月07日 08:23:30xonatiussetfiles: + qualname_in_exceptions.patch
nosy: + xonatius
messages: + msg235517

2015年02月06日 13:31:41ncoghlansetmessages: + msg235483
2015年02月06日 12:43:39pitrousetmessages: + msg235480
2015年02月06日 08:34:55ncoghlansetmessages: + msg235472
2015年02月05日 22:19:29pitrousetmessages: + msg235456
2015年02月05日 22:18:10ncoghlansetmessages: + msg235455
2015年02月05日 19:09:23pitrousetmessages: + msg235446
2015年02月05日 18:45:49rbcollinssetmessages: + msg235444
2015年02月05日 10:49:27ncoghlansetmessages: + msg235428
2015年02月03日 23:28:49rbcollinssetfiles: + issue17911-4.patch

messages: + msg235365
2015年01月27日 13:23:04ncoghlansetmessages: + msg234820
2015年01月27日 08:33:03pitrousetmessages: + msg234812
2015年01月27日 01:05:46rbcollinssetmessages: + msg234804
2015年01月26日 23:34:01pitrousetmessages: + msg234797
2015年01月26日 23:30:57rbcollinssetmessages: + msg234796
2015年01月26日 23:22:14pitrousetmessages: + msg234795
2015年01月26日 22:46:09ncoghlansetmessages: + msg234788
2015年01月26日 22:04:09rbcollinssetmessages: + msg234782
2015年01月26日 21:48:41rbcollinssetfiles: + issue17911-3.patch

messages: + msg234781
2015年01月26日 20:44:35rbcollinssetmessages: + msg234777
2015年01月26日 18:59:32pitrousetmessages: + msg234769
2015年01月26日 18:36:18rbcollinssetmessages: + msg234765
2015年01月26日 16:30:05martiussetnosy: + martius
2015年01月26日 13:38:35pitrousetmessages: + msg234741
2015年01月26日 13:12:20ncoghlansetmessages: + msg234740
2015年01月26日 03:18:47rbcollinssetfiles: + issue17911-2.patch

messages: + msg234712
2015年01月20日 02:38:02rbcollinssetfiles: + issue17911-1.patch

messages: + msg234338
2015年01月19日 17:56:27rbcollinssetmessages: + msg234319
2015年01月19日 02:12:38rbcollinssetfiles: + tb_stats_1.patch

messages: + msg234299
2015年01月19日 02:09:59rbcollinssetfiles: + frame_1.patch

messages: + msg234298
2015年01月19日 02:09:29rbcollinssetfiles: + linecache_2.patch
2015年01月19日 01:54:50rbcollinssetmessages: + msg234297
2015年01月14日 01:37:32rbcollinssetfiles: + linecache_1.patch

messages: + msg233995
2015年01月12日 22:53:15rbcollinssetmessages: + msg233895
2014年11月28日 00:56:20rbcollinssetnosy: + rbcollins
messages: + msg231792
2014年11月27日 02:47:50ncoghlansetmessages: + msg231743
2014年06月25日 23:22:14vstinnersettitle: Extracting tracebacks does too much work -> traceback: add a new thin class storing a traceback without storing local variables
2014年06月12日 21:48:46vstinnersetmessages: + msg220390
2014年06月12日 20:11:03rhettingersetnosy: + rhettinger
messages: + msg220381
2014年06月12日 12:47:21vstinnersetnosy: + vstinner
components: + asyncio
2014年03月05日 22:17:48eric.snowsetnosy: + eric.snow
2014年01月31日 23:45:11yselivanovsetkeywords: - easy
nosy: + yselivanov

versions: + Python 3.5, - Python 3.4
2013年07月18日 23:06:16ncoghlansetmessages: + msg193333
2013年07月18日 13:45:23pitrousetmessages: + msg193296
2013年07月18日 13:35:10adaptivelogicsetmessages: + msg193294
2013年07月18日 09:42:42pitrousetmessages: + msg193272
2013年07月12日 15:13:52adaptivelogicsetfiles: + traceback2.patch
2013年07月11日 21:15:37adaptivelogicsetfiles: + traceback.patch
keywords: + patch
messages: + msg192909
2013年07月09日 08:32:07pitrousetmessages: + msg192728
2013年07月08日 20:07:58gvanrossumsetmessages: + msg192690
2013年07月08日 17:04:34adaptivelogicsetnosy: + adaptivelogic
messages: + msg192672
2013年05月08日 11:55:47pitrousetmessages: + msg188719
2013年05月07日 13:13:44pitrousetmessages: + msg188648
2013年05月07日 03:40:03gvanrossumsetkeywords: + easy
2013年05月07日 03:39:46gvanrossumsetmessages: + msg188620
2013年05月07日 01:42:05ncoghlansetmessages: + msg188616
2013年05月06日 14:53:05pitrousetmessages: + msg188556
2013年05月06日 14:46:52gvanrossumsetmessages: + msg188554
2013年05月06日 14:39:37gvanrossumsetmessages: + msg188548
2013年05月06日 13:06:02pitrousetnosy: + ncoghlan, pitrou
messages: + msg188530
2013年05月05日 21:31:26gvanrossumcreate

AltStyle によって変換されたページ (->オリジナル) /