[Python-Dev] bpo-36829: Add sys.unraisablehook()

[Andrew Svetlov]

After the detailed explanation, UnraisableHookArgs makes sense.
Forward-compatibility is important thing

On Thu, May 16, 2019 at 1:12 PM Victor Stinner <vstinner at redhat.com> wrote:
>
> Le jeu. 16 mai 2019 à 08:39, Serhiy Storchaka <storchaka at gmail.com> a écrit :
> >
> > 16.05.19 04:23, Victor Stinner пише:
> > > The first implementation of my API used sys.unraisablehook(exc_type,
> > > exc_value, exc_tb, obj). The problem is that Serhiy Storchaka asked me
> > > to add a new error message field which breaks the API: the API is not
> > > future-proof.
> > >
> > > I modified my API to create an object to pack arguments. The new API
> > > becomes sys.unraisablehook(unraisable) where unraisable has 4 fields:
> > > exc_type, exc_value, exc_tb, obj. This API is now future-proof: adding
> > > a new field will not break existing custom hooks!
> >
> > I prefer the former design, when the hook takes 5 arguments: exc_type,
> > exc_value, exc_tb, obj and msg. Any additional human readable
> > information can be encoded in msg, and machine readable information can
> > be encoded in msg or obj. Currently we have no plans for adding more
> > details, and I do not think that we will need to do this in future.
> > Packing arguments into a single extendable object just complicates the
> > code and increases the chance of raising an exception or crashing.
> >
> > Even obj and msg could be merged into a single object, and the hook
> > could check whether it is a string or callable. But passing them as
> > separate arguments is fine to me, I just think that no more complication
> > is necessary.
>
> I came to the UnraisableHookArgs structseq idea because of my bad
> experience with extension warnings "hooks". Let me tell you this story
> :-)
>
> To enhance ResourceWarning messages, I modified Python 3.6 to add a
> new "source" parameter to the warnings module. It's the object which
> caused the ResourceWarning. This object is mostly used to display the
> traceback where the object has been allocated, traceback read using
> tracemalloc.get_object_traceback() and so required tracemalloc to be
> enabled.
>
>    https://bugs.python.org/issue26604
>
> The problem is that the 2 warnings "hooks" use a fixed number of
> positional (and positional-or-keyword) parameters:
>
> def showwarning(message, category, filename, lineno, file=None, line=None): ...
> def formatwarning(message, category, filename, lineno, line=None): ...
>
> Adding a new parameter would simply break *all* custom hooks in the wild...
>
> At the same time, internally, the warnings module uses a
> WarningMessage object to "pack" all parameters into a single object. I
> was able to easily add a new "source" attribute to WarningMessage
> without breaking the code using WarningMessage.
>
> I added new hooks which accept WarningMessage:
>
> def _showwarnmsg(msg): ...
> def _formatwarnmsg(msg): ...
>
> The problem is to keep the backward compatibility: decide which hook
> should be called... How to detect that showwarning or _showwarnmsg has
> been overriden? If both are overriden, which one should be modified?
>
> The implementation is tricky, and it caused a few minor regressions:
>
> https://github.com/python/cpython/commit/be7c460fb50efe3b88a00281025d76acc62ad2fd
>
> All these problems are the consequence of the initial design choice of
> warnings hooks: using a fixed number of parameters. This API simply
> cannot be extended. IMHO it's a bad design,  and we can do better:
> WarningMessage is a better design.
>
> --
>
> I would prefer to not reproduce the same API design mistake with
> sys.unraisablehook.
>
> I modified my PR to only use 4 fields: (exc_type, exc_value, exc_tb, obj).
>
> I plan to add a 5th field "err_msg" ("error_message"?) later to
> enhance the hook (allow to pass a custom error message to give more
> context where the exception comes from). Passing a single
> UnraisableHookArgs parameter to sys.unraisablehook allows to add a 5th
> field without breaking the backward compatibility.
>
> > Any additional human readable
> > information can be encoded in msg, and machine readable information can
> > be encoded in msg or obj.
>
> IMHO it's very risky to declare the current API as "complete". In my
> experience, we always want to enhance an API at some point to pass
> more information. IMHO the warnings module with my new "source"
> parameter is a good example.
>
> For unraisable hook, it's not hard to imagine other useful parameters
> that can be passed to the hook to provide more context about the
> exception. For example, right now we only pass one object. But there
> are cases where a second object would totally makes sense.
>
> --
>
> > Packing arguments into a single extendable object just complicates the
> > code and increases the chance of raising an exception or crashing.
>
> Technically, UnraisableHookArgs is basically a tuple of 4 items. I
> consider that there is a low risk that creating the object can fail.
>
> UnraisableHookArgs creation failure *is* covered by my implementation!
> The default hook is called directly in C without using a temporary
> UnraisableHookArgs object. The exception which caused the failure is
> logged as well.
>
> IMHO the very little risk of UnraisableHookArgs creation failure is
> worth it, compared to the pain to extend an API based on a fixed
> number of parameters.
>
> Victor
> --
> Night gathers, and now my watch begins. It shall not end until my death.
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: https://mail.python.org/mailman/options/python-dev/andrew.svetlov%40gmail.com



-- 
Thanks,
Andrew Svetlov