Skip to content

Propagate IO errors from custom readers and writers - #126

Open
aleroot wants to merge 1 commit into
ml-explore:mainfrom
aleroot:fix/propagate-custom-io-errors
Open

Propagate IO errors from custom readers and writers#126
aleroot wants to merge 1 commit into
ml-explore:mainfrom
aleroot:fix/propagate-custom-io-errors

Conversation

@aleroot

@aleroot aleroot commented Aug 18, 2026

Copy link
Copy Markdown

• The mlx_io_vtable read/write callbacks return void , so good() is the only error channel and CReader / CWriter never checked it, so failures were silently dropped and the caller got garbage data.
• Now mirrors mlx::core::io::ParallelFileReader::read / FileWriter::write , which already throw on short reads/writes.

Motivation

This is the missing first link for ml-explore/mlx#3742. With that fix, an exception from Load::eval_cpu 's read task poisons the stream's events and is re-thrown at eval time but only mlx's own file reader ever threw. Custom readers (e.g. mlx-swift's, see ml-explore/mlx-swift#427) could not report a truncated/failed read at all.

The read/write functions of `mlx_io_vtable` have no return value, so a
failed IO operation can only be reported through `good()`. `CReader` and
`CWriter` forwarded the call and returned, which silently dropped the
error: the caller gets a buffer filled with garbage and no exception is
ever raised.

Check `good()` after each call and throw, mirroring what the readers and
writers implemented in mlx do (`ParallelFileReader::read` and
`FileWriter::write`).

For readers this is what makes the error propagation added in
ml-explore/mlx#3742 usable with a custom `mlx_io_reader`: the exception
thrown from the read task of `Load::eval_cpu` now poisons the pending
events of the stream and is re-thrown when the lazily loaded arrays are
evaluated, instead of the load silently succeeding with truncated data.

For writers the error surfaces directly from `mlx_save_writer()` /
`mlx_save_safetensors_writer()`.

Behavior is unchanged for vtables that never report a failure through
`good()`.
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.

1 participant