Skip to content

Propagate virtual audio read OSError to callers - #507

Open
wangjie-coding wants to merge 1 commit into
bastibe:masterfrom
wangjie-coding:fix/propagate-virtual-read-oserror
Open

wangjie-coding wants to merge 1 commit into
bastibe:masterfrom
wangjie-coding:fix/propagate-virtual-read-oserror

Conversation

@wangjie-coding

Copy link
Copy Markdown

Preserve OSError raised by a virtual file's read/readinto callback and
re-raise the original exception after libsndfile returns to Python.
Previously, CFFI reported the exception as unraisable and supplied a zero
return value, so an audio read could look like normal EOF.

Keep the first read failure per SoundFile, consume it before interpreting
the native result, and close any handle returned when opening fails due
to a read callback error. Leave the existing readinto fallback behavior
unchanged. This handles read callbacks during open and sample I/O;
it does not add error propagation for write/seek/tell callbacks.

Validation on Windows, CPython 3.12 and libsndfile 1.2.2:

  • The same 30 new tests: 22 failed/8 passed before, 30 passed after.
  • Covers read, buffer_read, buffer_read_into and blocks; read/readinto
    adapters; mono/stereo PCM; header reads; non-seekable XI audio;
    successful short reads, EOF, recovery, file isolation and collection.
  • Existing runnable suite plus regressions: 330 passed, 71 deselected,
    with 16 pre-existing closed-file callback warnings.
  • git diff --check passes.
  • Pyright reports the same two pre-existing dirname(_libname) errors
    as the baseline, with no new diagnostics.

The 71 file-descriptor cases were excluded using the project's documented
Windows workaround. Linux, PyPy and other libsndfile versions have not
been validated. This uses constructed failing adapters; no production
transport failure is claimed. Prepared with AI assistance.

@bastibe

bastibe commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

I don't understand the problem, nor the solution. Also, users of this library may depend on the error types we return, so this is a breaking change, which we should only do if there's a real and significant benefit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants