classification
Title: csv.writer.writerows masks exceptions from __iter__
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ilja Everilä, miss-islington, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2016-02-22 12:38 by Ilja Everilä, last changed 2021-01-01 20:47 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 20536 merged serhiy.storchaka, 2020-05-30 11:40
PR 21047 merged miss-islington, 2020-06-22 08:22
PR 24021 merged serhiy.storchaka, 2020-12-31 12:32
Messages (11)
msg260673 - (view) Author: Ilja Everilä (Ilja Everilä) Date: 2016-02-22 12:38
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.
msg260712 - (view) Author: Ilja Everilä (Ilja Everilä) Date: 2016-02-23 09:07
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.
msg260936 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-02-27 09:52
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.
msg261018 - (view) Author: Ilja Everilä (Ilja Everilä) Date: 2016-02-29 20:15
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.
msg261019 - (view) Author: Ilja Everilä (Ilja Everilä) Date: 2016-02-29 20:31
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.
msg261026 - (view) Author: Ilja Everilä (Ilja Everilä) Date: 2016-02-29 22:30
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.
msg370374 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-05-30 11:45
It is definitely a bug. It can mask exceptions out of the control of the programmer like MemoryError and KeyboardInterrupt.
msg370375 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-05-30 11:56
This bug occurred not only in writerows(), but also in writerow() and csv.reader(). See also more general issue40824.
msg372046 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-06-22 08:22
New changeset c88239f864a27f673c0f0a9e62d2488563f9d081 by Serhiy Storchaka in branch 'master':
bpo-26407: Do not mask errors in csv. (GH-20536)
https://github.com/python/cpython/commit/c88239f864a27f673c0f0a9e62d2488563f9d081
msg372052 - (view) Author: miss-islington (miss-islington) Date: 2020-06-22 08:40
New changeset 5606d555b6e797625be9b0368472a86d8ad5252e by Miss Islington (bot) in branch '3.9':
bpo-26407: Do not mask errors in csv. (GH-20536)
https://github.com/python/cpython/commit/5606d555b6e797625be9b0368472a86d8ad5252e
msg384191 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2021-01-01 17:42
New changeset 6dffa67b98f78ae41b596f84478f3379f55d4d03 by Serhiy Storchaka in branch '3.8':
[3.8] bpo-26407: Do not mask errors in csv. (GH-20536) (GH-24021)
https://github.com/python/cpython/commit/6dffa67b98f78ae41b596f84478f3379f55d4d03
History
Date User Action Args
2021-01-01 20:47:08serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2021-01-01 17:42:27serhiy.storchakasetmessages: + msg384191
2020-12-31 21:13:36terry.reedysetstage: patch review -> commit review
versions: - Python 3.6, Python 3.7
2020-12-31 12:32:33serhiy.storchakasetpull_requests: + pull_request22861
2020-06-22 08:40:59miss-islingtonsetmessages: + msg372052
2020-06-22 08:22:15miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request20218
2020-06-22 08:22:07serhiy.storchakasetmessages: + msg372046
2020-05-30 11:56:29serhiy.storchakasetmessages: + msg370375
2020-05-30 11:45:12serhiy.storchakasettype: enhancement -> behavior
messages: + msg370374
components: + Library (Lib)
versions: + Python 3.7, Python 3.8, Python 3.9, Python 3.10
2020-05-30 11:40:43serhiy.storchakasetkeywords: + patch
nosy: + serhiy.storchaka

pull_requests: + pull_request19779
stage: test needed -> patch review
2016-02-29 22:30:33Ilja Everiläsetmessages: + msg261026
2016-02-29 20:31:18Ilja Everiläsetmessages: + msg261019
2016-02-29 20:15:17Ilja Everiläsetmessages: + msg261018
2016-02-27 09:52:50terry.reedysetversions: + Python 3.6
nosy: + terry.reedy

messages: + msg260936

type: behavior -> enhancement
stage: test needed
2016-02-23 09:07:38Ilja Everiläsetmessages: + msg260712
2016-02-22 12:38:53Ilja Everiläcreate