Skip to content

Raise NotFoundError rather than panic on non-existent file #90

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

Closed
maxrjones opened this issue Apr 22, 2025 · 3 comments · Fixed by #93
Closed

Raise NotFoundError rather than panic on non-existent file #90

maxrjones opened this issue Apr 22, 2025 · 3 comments · Fixed by #93

Comments

@maxrjones
Copy link
Member

A RustPanic occurs when passing a non-existent file to TIFF.open(). Would it be possible to instead raise a FileNotFoundError because this type of traceback would be unfamiliar to many Python users?

from async_tiff import TIFF
from async_tiff.store import LocalStore
import asyncio

file = "/Users/max/Documents/Code/maxrjones/virtual_tiff/tests/dvc/github/imaginary_file.tif" # Doesn't exist
store = LocalStore()
async def open_tiff(*, store, path):
    return await TIFF.open(path, store=store)

tiff = asyncio.run(open_tiff(store=store, path=file))
print(tiff)
(base) max@Maxs-DevSeed-MacBook python % uv run .vscode/no-tiff.py
thread 'tokio-runtime-worker' panicked at src/tiff.rs:35:86:
called `Result::unwrap()` on an `Err` value: ObjectStore(NotFound { path: "/Users/max/Documents/Code/maxrjones/virtual_tiff/tests/dvc/github/imaginary_file.tif", source: Os { code: 2, kind: NotFound, message: "No such file or directory" } })
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Traceback (most recent call last):
  File "/Users/max/Documents/Code/developmentseed/async-tiff/python/.vscode/no-tiff.py", line 11, in <module>
    tiff = asyncio.run(open_tiff(store=store, path=file))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/max/.local/share/uv/python/cpython-3.11.10-macos-aarch64-none/lib/python3.11/asyncio/runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "/Users/max/.local/share/uv/python/cpython-3.11.10-macos-aarch64-none/lib/python3.11/asyncio/runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/max/.local/share/uv/python/cpython-3.11.10-macos-aarch64-none/lib/python3.11/asyncio/base_events.py", line 654, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "/Users/max/Documents/Code/developmentseed/async-tiff/python/.vscode/no-tiff.py", line 9, in open_tiff
    return await TIFF.open(path, store=store)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
pyo3_async_runtimes.RustPanic: rust future panicked: unknown error
@kylebarron
Copy link
Member

Agreed it should be impossible for the Python user to see a rust panic.

@maxrjones
Copy link
Member Author

Agreed it should be impossible for the Python user to see a rust panic.

FYI I'm running tests against all of the TIFFs that GDAL uses in its test suite and these are the files that generate rust panics (unrelated to non-existent files)- https://github.com/maxrjones/virtual-tiff/blob/afa8440dc776e79aec93aaaab043ac1d323ad3bf/tests/test_gdal_examples.py#L210-L235. I could organize these into other async-tiff issues if helpful.

@kylebarron kylebarron changed the title Raise NotFoundError rather than panic Raise NotFoundError rather than panic on non-existent file Apr 22, 2025
@weiji14
Copy link
Member

weiji14 commented May 6, 2025

FYI I'm running tests against all of the TIFFs that GDAL uses in its test suite and these are the files that generate rust panics (unrelated to non-existent files)- https://github.com/maxrjones/virtual-tiff/blob/afa8440dc776e79aec93aaaab043ac1d323ad3bf/tests/test_gdal_examples.py#L210-L235. I could organize these into other async-tiff issues if helpful.

Yes, would be great to have a list of issues on what needs to be fixed/added still. As a suggestion, could you start with some of the predictor related tests, so that we can properly test #86?

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 a pull request may close this issue.

3 participants