Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C/API PyErr_AsUnicode() #50533

Open
ideasman42 mannequin opened this issue Jun 14, 2009 · 11 comments
Open

C/API PyErr_AsUnicode() #50533

ideasman42 mannequin opened this issue Jun 14, 2009 · 11 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ideasman42
Copy link
Mannequin

ideasman42 mannequin commented Jun 14, 2009

BPO 6284
Nosy @amauryfa, @vstinner, @Trundle, @ideasman42
Files
  • PyErr_AsUnicode_r73429.diff: updated patch
  • PyErr_AsUnicode_r73443.diff: patch that wont override sys.stderr as advaised.
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2009-06-14.17:03:37.492>
    labels = ['interpreter-core', 'type-feature']
    title = 'C/API PyErr_AsUnicode()'
    updated_at = <Date 2013-07-06.00:36:07.586>
    user = 'https://github.com/ideasman42'

    bugs.python.org fields:

    activity = <Date 2013-07-06.00:36:07.586>
    actor = 'christian.heimes'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2009-06-14.17:03:37.492>
    creator = 'ideasman42'
    dependencies = []
    files = ['14301', '14308']
    hgrepos = []
    issue_num = 6284
    keywords = ['patch']
    message_count = 8.0
    messages = ['89357', '89358', '89372', '89429', '89430', '125723', '125724', '125726']
    nosy_count = 4.0
    nosy_names = ['amaury.forgeotdarc', 'vstinner', 'Trundle', 'ideasman42']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue6284'
    versions = ['Python 3.4']

    @ideasman42
    Copy link
    Mannequin Author

    ideasman42 mannequin commented Jun 14, 2009

    This function returns the output of PyErr_Print as a unicode string.

    We needed this in Blender3D because it has its own error logging system
    which does not use the console.

    The patch is made against python3k r73429.

    @ideasman42
    Copy link
    Mannequin Author

    ideasman42 mannequin commented Jun 14, 2009

    last patch was bad heres a new one.

    @amauryfa
    Copy link
    Member

    PyErr_Print seems a too high-level function for this usage:

    • it uses sys.excepthook
    • it exits the process if the exception is SystemExit (!)

    I'd prefer a function similar to PyErr_Display. And sys.stderr should
    not be redirected to a temporary stream: this change is not thread safe.
    For example, A new function PyErr_DisplayEx could take an additional
    (PyObject *fp) argument.

    As for the Blender use case: isn't it appropriate for Blender to change
    sys.stderr globally, since the console is not used?

    @ideasman42
    Copy link
    Mannequin Author

    ideasman42 mannequin commented Jun 16, 2009

    Thanks for the feedback, I wasnt aware of PyErr_Display, its not
    documented as far as I know.

    http://docs.python.org/dev/py3k/genindex-all.html

    For blender, its development id still in progress so we don't yet have
    an internal console replacement.

    however we use a number of python apis in blender (game engine api, UI
    api, wrapping our own data) - So there are a number of places where this
    function could come in handy, and completely replacing the stdout isn't
    always an option - when running the game engine for instance.

    Ill look into using a PyErr_DisplayEx

    @ideasman42
    Copy link
    Mannequin Author

    ideasman42 mannequin commented Jun 16, 2009

    Updated with an PyErr_DisplayEx function that accepts a file argument.

    @pitrou pitrou added the type-feature A feature request or enhancement label Jan 6, 2011
    @vstinner
    Copy link
    Member

    vstinner commented Jan 7, 2011

    + PyErr_Fetch(&error_type, &error_value, &error_traceback);
    +
    + PyErr_Clear();

    I think that the call to PyErr_Clear() is useless, PyErr_Fetch() already cleared the exception.

    + /* clear the error */
    + PyErr_Restore(error_type, error_value, error_traceback);
    + PyErr_Clear();

    Why do you restore the error if you clear it directly? I think that at this position, there is no current exception. So it may be simplified as:

    Py_DECREF(error_type);
    Py_DECREF(error_value);
    Py_DECREF(error_traceback);

    I suppose here that all these 3 variables are not NULL.

    + if(! (string_io_mod= PyImport_ImportModule("io")) ) {
    + PyErr_Clear();
    + return NULL;
    + }
    + else if (! (string_io= PyObject_CallMethod(string_io_mod, "StringIO", NULL))) {
    + PyErr_Clear();
    + Py_DECREF(string_io_mod);
    + return NULL;
    + }
    + else if (! (string_io_getvalue= PyObject_GetAttrString(string_io, "getvalue"))) {
    + PyErr_Clear();
    + Py_DECREF(string_io_mod);
    + Py_DECREF(string_io);
    + return NULL;
    + }

    Minor style remark: you can remove the "else" keywords here.

    You should factorize the error handling at the end using "goto error;" with a error handler looking like:
    ------------
    ...
    goto finally;

    error:
       PyErr_Clear();
       Py_CLEAR(stringio_buf);
    finally:
       Py_XDECREF(string_io_mod);
       Py_XDECREF(string_io);
       Py_XDECREF(string_io_getvalue);
       return NULL;

    In my opinion, it's easier to maintain it, and more readable.

    @vstinner
    Copy link
    Member

    vstinner commented Jan 7, 2011

    /* Returns the exception string as a new
    PyUnicode object or NULL if the conversion failed */

    NULL is not very useful to analyze the error :-/ Why don't keep errors if the conversion failed? The caller will be responsible to use the new error, or to clear it.

    If PyErr_AsUnicode() raises a new error on conversion error, you should raise an error on:

    + if (!PyErr_Occurred())
    + return NULL;

    On keep PyImport_ImportModule(), PyObject_CallMethod() and PyObject_GetAttrString() error.

    Oh, by the way, PyErr_AsUnicode returns NULL and raise an error if PyObject_CallObject() failed (if StringIO().getvalue() failed).

    @Trundle
    Copy link
    Mannequin

    Trundle mannequin commented Jan 7, 2011

    There should also be a call to PyErr_NormalizeException(), as PyErr_Fetch() can return unnormalized exceptions (see e.g. issue bpo-10756).

    @tiran tiran added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jul 6, 2013
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @iritkatriel
    Copy link
    Member

    @ideasman42 This issue is quite old. Do you still need this feature?
    The patch needs to be converted to a GitHub PR, and can be simpler now since exception handling APIs have been improved.

    Marking as pending and will close this issue if there is no movement on it soon.

    @iritkatriel iritkatriel added the pending The issue will be closed if no feedback is provided label Feb 28, 2023
    @ideasman42
    Copy link
    Contributor

    @iritkatriel yes, I see there have been replies to the patch I must have missed, so I'll attempt to update the patch based on feedback.

    @arhadthedev

    This comment was marked as off-topic.

    @iritkatriel iritkatriel removed the pending The issue will be closed if no feedback is provided label Jul 29, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants