From 3735ab683f1c94c1fa7037c79f57eb7df27f45d4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 26 Aug 2026 23:38:34 +0300 Subject: [PATCH] gh-109714: Pass the file name and a message when raising OSError subclasses * Pass the file name as the filename argument instead of interpolating it into the message, so that it is available as the filename attribute: in shutil, logging.config, turtle, zipfile.Path and importlib.resources. * Give an explicit message where the one derived from errno would be misleading: "timed out" for timeouts which are not connection timeouts, and "No such resource" in importlib.resources. * In Tools, which is run with older Pythons too, pass errno explicitly. * Add _PyErr_SetOSErrorWithMessage() for raising an OSError subclass with an errno which is not the default one of the class. Co-Authored-By: Claude Opus 5 (1M context) --- Include/internal/pycore_pyerrors.h | 10 ++++++++++ Lib/asyncio/tasks.py | 4 ++-- Lib/asyncio/timeouts.py | 4 ++-- Lib/concurrent/futures/_base.py | 4 ++-- Lib/importlib/resources/abc.py | 10 +++++----- Lib/importlib/resources/readers.py | 11 ++++++++--- Lib/logging/config.py | 2 +- Lib/multiprocessing/pool.py | 4 ++-- Lib/shutil.py | 8 ++++---- Lib/test/test_turtle.py | 8 +++++--- Lib/turtle.py | 9 +++++---- Lib/zipfile/_path/__init__.py | 4 ++-- Python/errors.c | 11 +++++++++++ Python/remote_debug.h | 4 +++- Tools/build/generate-build-details.py | 4 +++- Tools/c-analyzer/c_parser/source.py | 3 ++- Tools/clinic/libclinic/utils.py | 4 +++- 17 files changed, 70 insertions(+), 34 deletions(-) diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h index c1f9d71e40077c..a05ffda5ac9e53 100644 --- a/Include/internal/pycore_pyerrors.h +++ b/Include/internal/pycore_pyerrors.h @@ -136,6 +136,16 @@ PyAPI_FUNC(void) _PyErr_SetString( PyObject *exception, const char *string); +/* + * Raise an OSError subclass with an explicit errno value, so that the + * resulting exception has a meaningful errno attribute. msg is used as + * strerror. Prefer PyErr_SetFromErrno() when the C errno is already set. + */ +PyAPI_FUNC(void) _PyErr_SetOSErrorWithMessage( + PyObject *exception, + int err, + const char *msg); + /* * Set an exception with the error message decoded from the current locale * encoding (LC_CTYPE). diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 498eec3f31b292..dfd18bc7e768b3 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -480,7 +480,7 @@ async def wait_for(fut, timeout): try: return fut.result() except exceptions.CancelledError as exc: - raise TimeoutError from exc + raise TimeoutError('timed out') from exc async with timeouts.timeout(timeout): return await fut @@ -613,7 +613,7 @@ async def _wait_for_one(self, resolve=False): f = await self._done.get() if f is None: # Dummy value from _handle_timeout(). - raise exceptions.TimeoutError + raise exceptions.TimeoutError('timed out') return f.result() if resolve else f diff --git a/Lib/asyncio/timeouts.py b/Lib/asyncio/timeouts.py index 65ddc285abd971..db69190b2568f8 100644 --- a/Lib/asyncio/timeouts.py +++ b/Lib/asyncio/timeouts.py @@ -112,7 +112,7 @@ async def __aexit__( # Since there are no new cancel requests, we're # handling this. if issubclass(exc_type, exceptions.CancelledError): - raise TimeoutError from exc_val + raise TimeoutError('timed out') from exc_val elif exc_val is not None: self._insert_timeout_error(exc_val) if isinstance(exc_val, ExceptionGroup): @@ -134,7 +134,7 @@ def _on_timeout(self) -> None: def _insert_timeout_error(exc_val: BaseException) -> None: while exc_val.__context__ is not None: if isinstance(exc_val.__context__, exceptions.CancelledError): - te = TimeoutError() + te = TimeoutError('timed out') te.__context__ = te.__cause__ = exc_val.__context__ exc_val.__context__ = te break diff --git a/Lib/concurrent/futures/_base.py b/Lib/concurrent/futures/_base.py index e728b8e0a91f74..5edd9bd0a3adee 100644 --- a/Lib/concurrent/futures/_base.py +++ b/Lib/concurrent/futures/_base.py @@ -460,7 +460,7 @@ def result(self, timeout=None): elif self._state == FINISHED: return self.__get_result() else: - raise TimeoutError() + raise TimeoutError('timed out') finally: # Break a reference cycle with the exception in self._exception self = None @@ -496,7 +496,7 @@ def exception(self, timeout=None): elif self._state == FINISHED: return self._exception else: - raise TimeoutError() + raise TimeoutError('timed out') # The following methods should only be used by Executors and in tests. def set_running_or_notify_cancel(self): diff --git a/Lib/importlib/resources/abc.py b/Lib/importlib/resources/abc.py index 0b5fdee80e8796..c1cd9e33766646 100644 --- a/Lib/importlib/resources/abc.py +++ b/Lib/importlib/resources/abc.py @@ -34,7 +34,7 @@ def open_resource(self, resource: Text) -> BinaryIO: # This deliberately raises FileNotFoundError instead of # NotImplementedError so that if this method is accidentally called, # it'll still do the right thing. - raise FileNotFoundError + raise FileNotFoundError('No such resource') @abc.abstractmethod def resource_path(self, resource: Text) -> Text: @@ -47,7 +47,7 @@ def resource_path(self, resource: Text) -> Text: # This deliberately raises FileNotFoundError instead of # NotImplementedError so that if this method is accidentally called, # it'll still do the right thing. - raise FileNotFoundError + raise FileNotFoundError('No such resource') @abc.abstractmethod def is_resource(self, path: Text) -> bool: @@ -55,12 +55,12 @@ def is_resource(self, path: Text) -> bool: Files are resources, directories are not. """ - raise FileNotFoundError + raise FileNotFoundError('No such resource') @abc.abstractmethod def contents(self) -> Iterable[str]: """Return an iterable of entries in `package`.""" - raise FileNotFoundError + raise FileNotFoundError('No such resource') class TraversalError(Exception): @@ -180,7 +180,7 @@ def open_resource(self, resource: StrPath) -> BinaryIO: return self.files().joinpath(resource).open('rb') def resource_path(self, resource: Any) -> NoReturn: - raise FileNotFoundError(resource) + raise FileNotFoundError('No such resource', filename=resource) def is_resource(self, path: StrPath) -> bool: return self.files().joinpath(path).is_file() diff --git a/Lib/importlib/resources/readers.py b/Lib/importlib/resources/readers.py index 5d0ae46d672f53..b21330731505e5 100644 --- a/Lib/importlib/resources/readers.py +++ b/Lib/importlib/resources/readers.py @@ -46,7 +46,10 @@ def open_resource(self, resource): try: return super().open_resource(resource) except KeyError as exc: - raise FileNotFoundError(exc.args[0]) + if resource == exc.args[0]: + raise FileNotFoundError('No such resource', filename=resource) + else: + raise FileNotFoundError(exc.args[0]) def is_resource(self, path): """ @@ -73,8 +76,10 @@ def __init__(self, *paths): if not self._paths: message = 'MultiplexedPath must contain at least one path' raise FileNotFoundError(message) - if not all(path.is_dir() for path in self._paths): - raise NotADirectoryError('MultiplexedPath only supports directories') + for path in self._paths: + if not path.is_dir(): + message = 'MultiplexedPath only supports directories' + raise NotADirectoryError(message, filename=path) def iterdir(self): children = (child for path in self._paths for child in path.iterdir()) diff --git a/Lib/logging/config.py b/Lib/logging/config.py index f566de5750dbf5..5d8308b7c423bd 100644 --- a/Lib/logging/config.py +++ b/Lib/logging/config.py @@ -65,7 +65,7 @@ def fileConfig(fname, defaults=None, disable_existing_loggers=True, encoding=Non if isinstance(fname, str): if not os.path.exists(fname): - raise FileNotFoundError(f"{fname} doesn't exist") + raise FileNotFoundError('No such file', filename=fname) elif not os.path.getsize(fname): raise RuntimeError(f'{fname} is an empty file') diff --git a/Lib/multiprocessing/pool.py b/Lib/multiprocessing/pool.py index f50bcbe4451bea..c382bd28922ae7 100644 --- a/Lib/multiprocessing/pool.py +++ b/Lib/multiprocessing/pool.py @@ -792,7 +792,7 @@ def wait(self, timeout=None): def get(self, timeout=None): self.wait(timeout) if not self.ready(): - raise TimeoutError + raise TimeoutError('timed out') if self._success: return self._value else: @@ -893,7 +893,7 @@ def next(self, timeout=None): except IndexError: if self._index == self._length: self._stop_iterator() - raise TimeoutError from None + raise TimeoutError('timed out') from None if self._buffersize_sema is not None: self._buffersize_sema.release() diff --git a/Lib/shutil.py b/Lib/shutil.py index ab75ba9da8894b..248c35bc47892c 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -362,7 +362,8 @@ def copyfile(src, dst, *, follow_symlinks=True): # Issue 43219, raise a less confusing exception except IsADirectoryError as e: if not os.path.exists(dst): - raise FileNotFoundError(f'Directory does not exist: {dst}') from e + raise FileNotFoundError('Directory does not exist', + filename=dst) from e else: raise @@ -948,9 +949,8 @@ def move(src, dst, copy_function=copy2): if (_is_immutable(src) or (not os.access(src, os.W_OK) and os.listdir(src) and sys.platform == 'darwin')): - raise PermissionError("Cannot move the non-empty directory " - "'%s': Lacking write permission to '%s'." - % (src, src)) + raise PermissionError("Cannot move the non-empty directory: " + "Lacking write permission", filename=src) copytree(src, real_dst, copy_function=copy_function, symlinks=True) rmtree(src) diff --git a/Lib/test/test_turtle.py b/Lib/test/test_turtle.py index c49ce9cdb6f6d9..860d242f654327 100644 --- a/Lib/test/test_turtle.py +++ b/Lib/test/test_turtle.py @@ -510,7 +510,8 @@ def test_save_raises_if_parent_not_found(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: parent = os.path.join(tmpdir, "unknown_parent") - msg = f"The directory '{parent}' does not exist. Cannot save to it" + msg = ("The directory does not exist. Cannot save to it: " + f"'{parent}'") with self.assertRaisesRegex(FileNotFoundError, re.escape(msg)): turtle.TurtleScreen.save(screen, os.path.join(parent, "a.ps")) @@ -524,8 +525,9 @@ def test_save_raises_if_file_found(self) -> None: f.write("some text") msg = ( - f"The file '{file_path}' already exists. To overwrite it use" - " the 'overwrite=True' argument of the save function." + "The file already exists. To overwrite it use" + " the 'overwrite=True' argument of the save function: " + f"'{file_path}'" ) with self.assertRaisesRegex(FileExistsError, re.escape(msg)): turtle.TurtleScreen.save(screen, file_path) diff --git a/Lib/turtle.py b/Lib/turtle.py index b49df26bd07bc5..fc5d19cb5a16df 100644 --- a/Lib/turtle.py +++ b/Lib/turtle.py @@ -1558,13 +1558,14 @@ def save(self, filename, *, overwrite=False): filename = Path(filename) if not filename.parent.exists(): raise FileNotFoundError( - f"The directory '{filename.parent}' does not exist." - " Cannot save to it." + "The directory does not exist. Cannot save to it", + filename=str(filename.parent), ) if not overwrite and filename.exists(): raise FileExistsError( - f"The file '{filename}' already exists. To overwrite it use" - " the 'overwrite=True' argument of the save function." + "The file already exists. To overwrite it use" + " the 'overwrite=True' argument of the save function", + filename=str(filename), ) if (ext := filename.suffix) not in {".ps", ".eps"}: raise ValueError( diff --git a/Lib/zipfile/_path/__init__.py b/Lib/zipfile/_path/__init__.py index faae4c84cae5ed..c5b525d76fdfaa 100644 --- a/Lib/zipfile/_path/__init__.py +++ b/Lib/zipfile/_path/__init__.py @@ -341,10 +341,10 @@ def open(self, mode='r', *args, pwd=None, **kwargs): to io.TextIOWrapper(). """ if self.is_dir(): - raise IsADirectoryError(self) + raise IsADirectoryError(filename=self) zip_mode = mode[0] if zip_mode == 'r' and not self.exists(): - raise FileNotFoundError(self) + raise FileNotFoundError('No such file', filename=self) stream = self.root.open(self.at, zip_mode, pwd=pwd) if 'b' in mode: if args or kwargs: diff --git a/Python/errors.c b/Python/errors.c index 48b03e5fd714b1..231d489b8e390b 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -933,6 +933,17 @@ PyErr_SetFromErrno(PyObject *exc) return PyErr_SetFromErrnoWithFilenameObjects(exc, NULL, NULL); } +void +_PyErr_SetOSErrorWithMessage(PyObject *exc, int err, const char *msg) +{ + PyObject *args = Py_BuildValue("(is)", err, msg); + if (args == NULL) { + return; + } + PyErr_SetObject(exc, args); + Py_DECREF(args); +} + #ifdef MS_WINDOWS /* Windows specific error code handling */ PyObject *PyErr_SetExcFromWindowsErrWithFilenameObject( diff --git a/Python/remote_debug.h b/Python/remote_debug.h index 12c5963f97c0e6..7122be934a2f52 100644 --- a/Python/remote_debug.h +++ b/Python/remote_debug.h @@ -29,6 +29,7 @@ extern "C" { #include "pyconfig.h" #include "internal/pycore_ceval.h" +#include "internal/pycore_pyerrors.h" #ifdef __linux__ # include @@ -1577,7 +1578,8 @@ _Py_RemoteDebug_WriteRemoteMemory(proc_handle_t *handle, uintptr_t remote_addres PyErr_SetString(PyExc_PermissionError, "Not enough permissions to write memory"); break; case KERN_INVALID_ARGUMENT: - PyErr_SetString(PyExc_PermissionError, "Invalid argument to mach_vm_write"); + _PyErr_SetOSErrorWithMessage(PyExc_PermissionError, EINVAL, + "Invalid argument to mach_vm_write"); break; default: PyErr_Format(PyExc_RuntimeError, "Unknown error writing memory: %d", (int)kr); diff --git a/Tools/build/generate-build-details.py b/Tools/build/generate-build-details.py index 8272635bc627d6..a098fa9f8089b9 100644 --- a/Tools/build/generate-build-details.py +++ b/Tools/build/generate-build-details.py @@ -7,6 +7,7 @@ import argparse import collections +import errno import importlib.machinery import json import os @@ -93,7 +94,8 @@ def generate_data(schema_version: str) -> collections.defaultdict[str, Any]: has_dynamic_library = hasattr(sys, 'dllhandle') has_static_library = not has_dynamic_library else: - raise NotADirectoryError(f'Unknown platform: {os.name}') + raise NotADirectoryError(errno.ENOTDIR, + f'Unknown platform: {os.name}') # On POSIX, EXT_SUFFIX is set regardless if extension modules are supported # or not, and on Windows older versions of CPython only set EXT_SUFFIX when diff --git a/Tools/c-analyzer/c_parser/source.py b/Tools/c-analyzer/c_parser/source.py index 30a09eeb56a1f7..44261e30ce3ad4 100644 --- a/Tools/c-analyzer/c_parser/source.py +++ b/Tools/c-analyzer/c_parser/source.py @@ -1,4 +1,5 @@ import contextlib +import errno import os.path @@ -28,7 +29,7 @@ def good_file(filename, alt=None): yield filename except Exception: if not os.path.exists(filename): - raise FileNotFoundError(f'file not found: {filename}') + raise FileNotFoundError(errno.ENOENT, 'file not found', filename) raise # re-raise diff --git a/Tools/clinic/libclinic/utils.py b/Tools/clinic/libclinic/utils.py index 01015ff1237656..6a0085fb429041 100644 --- a/Tools/clinic/libclinic/utils.py +++ b/Tools/clinic/libclinic/utils.py @@ -1,5 +1,6 @@ import collections import dataclasses as dc +import errno import enum import hashlib import os @@ -64,7 +65,8 @@ def makedirs(self, dirname: str) -> None: elif os.path.exists(dirname): # Create nothing, but fail as os.makedirs() does, so that # the caller can report an existing non-directory. - raise FileExistsError(dirname) + raise FileExistsError(errno.EEXIST, os.strerror(errno.EEXIST), + dirname) def write(self, filename: str, new_contents: str) -> None: if not self.dry_run: