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

csv.writer.writerows masks exceptions from __iter__ #70595

Closed
IljaEveril mannequin opened this issue Feb 22, 2016 · 11 comments
Closed

csv.writer.writerows masks exceptions from __iter__ #70595

IljaEveril mannequin opened this issue Feb 22, 2016 · 11 comments
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@IljaEveril
Copy link
Mannequin

IljaEveril mannequin commented Feb 22, 2016

BPO 26407
Nosy @terryjreedy, @serhiy-storchaka, @miss-islington
PRs
  • bpo-26407: Do not mask errors in csv. #20536
  • [3.9] bpo-26407: Do not mask errors in csv. (GH-20536) #21047
  • [3.8] bpo-26407: Do not mask errors in csv. (GH-20536) #24021
  • 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 = <Date 2021-01-01.20:47:08.361>
    created_at = <Date 2016-02-22.12:38:53.796>
    labels = ['3.8', 'type-bug', 'library', '3.9', '3.10']
    title = 'csv.writer.writerows masks exceptions from __iter__'
    updated_at = <Date 2021-01-01.20:47:08.360>
    user = 'https://bugs.python.org/IljaEveril'

    bugs.python.org fields:

    activity = <Date 2021-01-01.20:47:08.360>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-01-01.20:47:08.361>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2016-02-22.12:38:53.796>
    creator = 'Ilja Everil\xc3\xa4'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 26407
    keywords = ['patch']
    message_count = 11.0
    messages = ['260673', '260712', '260936', '261018', '261019', '261026', '370374', '370375', '372046', '372052', '384191']
    nosy_count = 4.0
    nosy_names = ['terry.reedy', 'serhiy.storchaka', 'Ilja Everil\xc3\xa4', 'miss-islington']
    pr_nums = ['20536', '21047', '24021']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue26407'
    versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

    @IljaEveril
    Copy link
    Mannequin Author

    IljaEveril mannequin commented Feb 22, 2016

    When passing a class implementing the dunder __iter__ that raises to csv.writer.writerows it hides the original exception and raises a TypeError instead.

    In the raised TypeError the __context__ and __cause__ both are None.

    For example:

    >>> class X:
    ...  def __iter__(self):
    ...   raise RuntimeError("I'm hidden")
    ... 
    >>> x = X()
    >>> list(x)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 3, in __iter__
    RuntimeError: I'm hidden
    >>> import csv
    >>> csv.writer(open('foo.csv', 'w', newline='')).writerows(x)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: writerows() argument must be iterable
    >>> try:
    ...  csv.writer(open('foo.csv', 'w', newline='')).writerows(x)
    ... except TypeError as e:
    ...  e_ = e
    >>> e_.__context__ is None
    True
    >>> e_.__cause__ is None
    True

    It would seem that for reasons unknown to me either PyObject_GetIter loses the exception context or the call to PyErr_SetString in csv_writerows hides it.

    @IljaEveril IljaEveril mannequin added the type-bug An unexpected behavior, bug, or error label Feb 22, 2016
    @IljaEveril
    Copy link
    Mannequin Author

    IljaEveril mannequin commented Feb 23, 2016

    After doing some reading on https://docs.python.org/dev/c-api/exceptions.html it seems that this is possibly "as designed" or such, since csv_writerows explicitly calls PyErr_SetString on receiving NULL from PyObject_GetIter.

    Still, it feels like this could either let the original exception fall through (since it has nothing in the way of handling it) or form the chain in PY3 for easier debugging of the real cause.

    To give this thing some context we ran in to this while passing SQLAlchemy Query objects to csv_writerows. The Query object is compiled during call to __iter__ and the current behaviour masks possible SQL errors etc.

    @terryjreedy
    Copy link
    Member

    The TypeError is correct. You passed a non-iterable. I agree that adding information, *if possible*, when 'non-iterable' is determined by an exception being raised, would be nice. I don't know how easy this would be.

    @terryjreedy terryjreedy added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Feb 27, 2016
    @IljaEveril
    Copy link
    Mannequin Author

    IljaEveril mannequin commented Feb 29, 2016

    I have 0 beef with it being the TypeError as long as the exception chain is kept intact, especially since PY3 makes it possible. With current behaviour heisenbugs would produce some serious hair pulling, until one figures out that it is actually the __iter__ raising an exception.

    With that in mind, take an object implementing the __iter__ dunder correctly 999,999 times out of a million. Is it not iterable?

    Unfortunately I lack the experience with CPython C API to do something about this. Tests on the other hand I suppose I could manage, if a consensus on the behaviour can be reached.

    @IljaEveril
    Copy link
    Mannequin Author

    IljaEveril mannequin commented Feb 29, 2016

    As a side note PyObject_GetIter actually raises a TypeError already for non-iterables (classes without __iter__ etc.), so csv_writerows duplicates the effort at the moment.

    @IljaEveril
    Copy link
    Mannequin Author

    IljaEveril mannequin commented Feb 29, 2016

    I'm starting to think my initial example code was too simplified and misled from the issue at hand. It very explicitly showed what happens when some class with __iter__ raises and is passed to csv_writerows. Even then:

    >>> from collections.abc import Iterable
    >>> class X:
    ...  def __iter__(self):
    ...   raise RuntimeError
    ... 
    >>> x = X()
    >>> issubclass(X, Iterable)
    True
    >>> isinstance(x, Iterable)
    True

    The glossary entry for iterable has nothing to say about exceptions. It only requires that they are able to return their members "one at a time". In a moderately complicated class this might be true most of the time, but that's not interesting; that 1 time when it can't is.

    Now imagine you have a class with __iter__ using 1..N foreign libraries that all may blow up at the wrong phase of the moon (humor me). With the current implementation you get "TypeError: writerows() argument must be iterable", which goes out of its way to actually mislead you. Just letting the original exception through would be the obviously helpful thing to do. Which is what PyObject_GetIter does, before csv_writerows overwrites it.

    @serhiy-storchaka
    Copy link
    Member

    It is definitely a bug. It can mask exceptions out of the control of the programmer like MemoryError and KeyboardInterrupt.

    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir 3.7 (EOL) end of life 3.8 only security fixes 3.9 only security fixes 3.10 only security fixes type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels May 30, 2020
    @serhiy-storchaka
    Copy link
    Member

    This bug occurred not only in writerows(), but also in writerow() and csv.reader(). See also more general bpo-40824.

    @serhiy-storchaka
    Copy link
    Member

    New changeset c88239f by Serhiy Storchaka in branch 'master':
    bpo-26407: Do not mask errors in csv. (GH-20536)
    c88239f

    @miss-islington
    Copy link
    Contributor

    New changeset 5606d55 by Miss Islington (bot) in branch '3.9':
    bpo-26407: Do not mask errors in csv. (GH-20536)
    5606d55

    @terryjreedy terryjreedy removed the 3.7 (EOL) end of life label Dec 31, 2020
    @serhiy-storchaka
    Copy link
    Member

    New changeset 6dffa67 by Serhiy Storchaka in branch '3.8':
    [3.8] bpo-26407: Do not mask errors in csv. (GH-20536) (GH-24021)
    6dffa67

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes 3.10 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants