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

Minor adjustments to exception handling #153

Merged
merged 6 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
- name: Build sdist
run: python setup.py sdist

- uses: actions/upload-artifact@v4
- uses: actions/upload-artifact@v4.4.3
with:
name: sdist
path: ./dist/*.tar.gz
Expand All @@ -49,7 +49,7 @@ jobs:
- name: Build wheels
run: bash ./.ci/build_wheels.sh

- uses: actions/upload-artifact@v4
- uses: actions/upload-artifact@v4.4.3
with:
name: wheels
path: ./dist/*.whl
Expand All @@ -76,7 +76,7 @@ jobs:
run: bash ./.ci/download_zlib.sh
- name: Build wheels
run: bash ./.ci/build_wheels.sh
- uses: actions/upload-artifact@v4
- uses: actions/upload-artifact@v4.4.3
with:
name: wheels
path: ./dist/*.whl
Expand Down Expand Up @@ -105,7 +105,7 @@ jobs:
uses: docker/setup-qemu-action@v3
- name: Build wheels
run: bash ./.ci/build_wheels.sh
- uses: actions/upload-artifact@v4
- uses: actions/upload-artifact@v4.4.3
with:
name: wheels
path: ./dist/*.whl
Expand Down
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# `indexed_gzip` changelog


## 1.9.0 (November 15th 2024)


* Preserve exception information when reading from a Python file-like (#152).


## 1.8.8 (November 7th 2024)


Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ Many thanks to the following contributors (listed chronologically):
- Maximilian Knespel (@mxmlnkn) Change default read buffer size to
improve performance (#90).
- Ben Beasley (@musicinmybrain) Python 3.12 compatibility (#126).
- @camillol: Preserve exceptions raised by Python file-likes (#152).


## License
Expand Down
2 changes: 1 addition & 1 deletion indexed_gzip/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@
"""


__version__ = '1.8.8'
__version__ = '1.9.0'
79 changes: 52 additions & 27 deletions indexed_gzip/indexed_gzip.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -9,36 +9,44 @@ random access to gzip files.
"""


from libc.stdio cimport (SEEK_SET,
SEEK_CUR,
SEEK_END,
FILE,
fopen,
fdopen,
fclose)

from libc.stdint cimport (uint8_t,
uint32_t,
uint64_t,
int64_t)

from cpython.mem cimport (PyMem_Malloc,
PyMem_Realloc,
PyMem_Free)

from cpython.buffer cimport (PyObject_GetBuffer,
PyBuffer_Release,
PyBUF_ANY_CONTIGUOUS,
PyBUF_SIMPLE)

from cpython.ref cimport PyObject, Py_XDECREF
from cpython.exc cimport PyErr_Occurred, PyErr_Fetch, PyErr_NormalizeException
from libc.stdio cimport (SEEK_SET,
SEEK_CUR,
SEEK_END,
FILE,
fopen,
fdopen,
fclose)

from libc.stdint cimport (uint8_t,
uint32_t,
uint64_t,
int64_t)

from cpython.mem cimport (PyMem_Malloc,
PyMem_Realloc,
PyMem_Free)

from cpython.buffer cimport (PyObject_GetBuffer,
PyBuffer_Release,
PyBUF_ANY_CONTIGUOUS,
PyBUF_SIMPLE)

from cpython.ref cimport (PyObject,
Py_XDECREF)
from cpython.exc cimport (PyErr_Fetch,
PyErr_Fetch,
PyErr_NormalizeException,
PyErr_Occurred)
from indexed_gzip.set_traceback cimport PyException_SetTraceback


cimport indexed_gzip.zran as zran


import io
import os
import os.path as op
import sys
import pickle
import logging
import warnings
Expand All @@ -55,6 +63,9 @@ instead.
"""


PY3 = sys.version_info[0] >= 3


log = logging.getLogger(__name__)


Expand All @@ -69,14 +80,28 @@ def open(filename=None, fileobj=None, *args, **kwargs):
return IndexedGzipFile(filename, fileobj, **kwargs)


cdef _get_python_exception():
cdef get_python_exception():
"""Checks to see if a Python exception has occurred. If so, returns a
reference to the exception object. Returns None otherwise.

This function is used so that, if an IndexedGzipFile object is reading
from a Python file-like, and the file-like raises an error, the
information about that error is not lost.
"""
cdef PyObject *ptype
cdef PyObject *pvalue
cdef PyObject *ptraceback
if PyErr_Occurred():
PyErr_Fetch(&ptype, &pvalue, &ptraceback)
PyErr_NormalizeException(&ptype, &pvalue, &ptraceback)

# Populate traceback info for the original exception
if PY3 and (ptraceback != NULL):
PyException_SetTraceback(pvalue, ptraceback)

exc = <object>pvalue
if PY3 and (ptraceback != NULL):
PyException_SetTraceback(pvalue, NULL)
Py_XDECREF(ptype)
Py_XDECREF(pvalue)
Py_XDECREF(ptraceback)
Expand Down Expand Up @@ -759,7 +784,7 @@ cdef class _IndexedGzipFile:

# Unknown error
elif ret < 0:
exc = _get_python_exception()
exc = get_python_exception()
raise ZranError('zran_read returned error: {} (file: '
'{})'.format(ZRAN_ERRORS.ZRAN_READ[ret],
self.errname)) from exc
Expand Down Expand Up @@ -818,7 +843,7 @@ cdef class _IndexedGzipFile:

# see how the read went
if ret == zran.ZRAN_READ_FAIL:
exc = _get_python_exception()
exc = get_python_exception()
raise ZranError('zran_read returned error: {} (file: {})'
.format(ZRAN_ERRORS.ZRAN_READ[ret], self.errname)) from exc

Expand Down
10 changes: 10 additions & 0 deletions indexed_gzip/set_traceback.pxd
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#
# Cython declaration for the PyException_SetTraceback function.
# This function is in the Python C API, but is not in the built-in
# Cython declarations.
#

from cpython.ref cimport PyObject

cdef extern from "Python.h":
PyObject* PyException_SetTraceback(PyObject* ex, PyObject* tb)
22 changes: 14 additions & 8 deletions indexed_gzip/tests/test_indexed_gzip.py
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,8 @@ def test_read_beyond_end(concat, drop):


def test_read_exception(testfile, nelems):
"""When wrapping a python file object, if it raises an exception it should be preserved.
"""When wrapping a python file object, if it raises an exception
it should be preserved.
"""
if sys.version_info[0] < 3:
# We can't set the .read attribute in Python 2
Expand All @@ -521,15 +522,15 @@ def test_read_exception(testfile, nelems):
gzf = None

MY_ERROR = "This error should be preserved"
# We'll use a weakref to check that we are handling reference counting correctly,
# and you cannot weakref an Exception, so we need a subclass.

# We'll use a weakref to check that we are handling reference counting
# correctly, and you cannot weakref an Exception, so we need a subclass.
class MyException(Exception):
pass
my_err_weak = None
my_err_weak = [None]
def my_error_fn(*args, **kwargs):
err = MyException(MY_ERROR)
nonlocal my_err_weak
my_err_weak = weakref.ref(err)
my_err_weak[0] = weakref.ref(err)
raise err

try:
Expand All @@ -540,15 +541,20 @@ def my_error_fn(*args, **kwargs):
try:
gzf.read(1)
except Exception as e:
assert MY_ERROR in str(e) or MY_ERROR in str(e.__cause__), f"Exception was not preserved; got {e}"
assert (MY_ERROR in str(e) or MY_ERROR in str(e.__cause__),
"Exception was not preserved; got {}".format(e))
del e
else:
assert False, "Expected an exception to be raised"


finally:
if gzf is not None: gzf.close()
if f is not None: f .close()
del f
del gzf
assert my_err_weak is None or my_err_weak() is None, "Exception was not garbage collected"
assert (my_err_weak[0] is None or my_err_weak[0]() is None,
"Exception was not garbage collected")


def test_seek(concat):
Expand Down