PEP 678: Enriching Exceptions with Notes

PEPs
41

Though with_traceback only overwrites the traceback field, it never appends. So it wouldn’t really be very consistent.

42

Sure, let’s go with .add_note(note: str) then :slightly_smiling_face:

Are you OK to implement in CPython, and I’ll handle the PEP and backport?

1 Like
43

Here’s a PR. I added one detail: The note passed to add_note can be None, and then you can clear all notes with add_note(None, replace=True).

2 Likes
44

Why default to None?
A default of “” (empty string) would remove a lot of special case code.

ex.__note__ = s if ex.__note__ is None else ex.__note__ + s
becomes
ex.__note__ += s

Also, the PEP uses the term “cast to a string”. It should be “convert to a string”.

2 Likes
45

More bikeshedding: consider dropping the replace keyword and instead add clear_note() and replace_note() APIs.

2 Likes
46

I like the new add_note() spelling, but I’m not sure of the motivation for the more complicated signature in Irit’s PR. It seems safer to start with just add_note(self, note: str) -> None, and add more features once we find a compelling use case.

What are the use cases for clearing or replacing notes?

1 Like
47

@markshannon, each note is printed starting on a new line, so we want to distinguish between “empty note” and “no note”. str | None was the obvious way to do that before we changed to the .add_note() and tuple[str, ...] plan.

@erlendaasland @Jelle @iritkatriel, I can at least imagine wanting to clear verbose notes before logging if I knew they were not relevant in my application. Perhaps .add_note(note) and .clear_notes() methods? Replacement is considerably rarer, and easy to implement by clearing notes and adding back those you wanted to keep.

3 Likes
48

I think clearing will not be done often either, and I’m not sure it’s worth another entry in BaseException’s dict.

We could make it possible to clear the notes with e.__notes__ = None, while still forbidding assignment of any other value (though the value of __notes__ after that will be (), which may be weird).

49

Or allowing del e.__notes__ to clear it?

3 Likes
50

del does seem like a more natural fit, although still kinda weird in that the attribute is changed rather than removed.

51

There’s prior art (at least similar) in invalidating the cache of a functools.cached_property

3 Likes
52

I’ve updated PR 31317 with that.

1 Like
53

Yes, it’s no more mysterious than any other descriptor with a deleter at the end of the day. +1 from me.

54

My apologies for not getting to this sooner – I’m only getting ideas as I re-read the PEP & look at the big picture.

Let me get back to this rejected idea:

Just make notes mutable

If __notes__ was mutable (e.g. a list) or even assignable, there would be no need for explicit methods to add and remove notes separate from e.g. ex.__notes__.append(note) or ex.__notes__.clear(). While we like the simplicity of this approach, it cannot guarantee that __notes__ is a sequence of strings, and thus risks failing at traceback-display time like the proposal above.

What if the type-checking was done at the traceback printing time, rather than when the attribute/items are set?
That is:

  • When the traceback printing code finds a non-tuple in __notes__ it prints “<bad notes>” rather than the list of notes.
  • When it encounters a non-string note, it prints “<non-string note>” rather than the note itself.

With that, it seems that a lot of the complexity could be avoided – no need to disallow mutation and carefully allow only add_note (plus maybe clear_notes) – at the cost of a bit of complexity in the traceback printing code (which is, IMO, complex enough already that a few type checks won’t make much difference).

The docs can simply say that the __notes__ attribute should, if set, be a list of strings. Users who set it to something else they may get unexpected behavior, including exceptions (e.g. in other code that uses __notes__).

Usage:

  • To add a note:
    try:
    exc.__notes__.append('...')
except AttributeError:
    exc.__notes__ = ['...']
  • To clear notes:
    try:
    del exc.__notes__
except AttributeError:
    pass

Since there would be no change to exception objects themselves: in older versions of Python, third-party traceback printers would work seamlessly together with code that sets __notes__.

(It would also be possible to add a __notes__ descriptor that auto-creates an empty list on access, to turn those usage examples into one-liners. But on second thought it seems like too much magic. The kind of libraries that handle __notes__ don’t need sugar.)

55

The main disadvantage seems to be that it would be difficult to debug where non-string notes are being added, both because the incorrect code mutating __notes__ is very distant from the indication of error, and also because (although I’m not sure about this), it would be difficult to inspect __notes__ at the time the non-string notes were discovered by the traceback code.

56

+100. Which is why I am still supporting a single __note__ string with \n-separated notes.

1 Like
57

IIUC, the problem with this is that notes can be multi-line, so you would not be able to know what the individual notes were, and this would make translations of notes not work, because you need to look them up as the exact text.

58

It is I who first mentioned (in a different context) the desirability of translations but it is clear that the intent of __note__ (or __notes__) is for specialized applications for which direct translations will not be required or useful.

59

That’s a variant of a common “problem” in Python – types aren’t checked until an object is used. I don’t think it’s much of an issue here – it’ll “just” make a helpful note turn unhelpful. (Plus, typing tools should be able to catch this.)

I don’t know about translations, but anything that needs to display the notes on something that’s not a text-based terminal should have access to separate notes. A single string with a \n\n separators (which can appear in individual notes) sounds too “stringly typed” to me.
It’s well possible that I’m overthinking this, of course.

60

My position is indeed that everyone is overthinking this. Do we even have a use case for having \n inside what is to be considered a single note? And is there a use case (apart from translation, which Andre Roberge says is not relevant) that requires picking apart notes once they have been added to an exception object?

IMO the use cases are:

  • Add a note (may happen multiple times)
  • Get the notes to print them
  • Whatever’s needed to unittest notes behavior

A single read-write __note__ or __notes__ attribute that’s limited to a string or None can satisfy all these.

1 Like