Improve the error messages for file I/O errors (#355)

This improves the error messages shown for file related I/O errors,
by:
- ensuring that the path is always included
- adding additional suggestions to the error based on the context
  (which is now known, since specialised types are used for each of
  the common scenarios like checking if a file exists or reading a
  file to a string)

We're able to make the errors richer than those provided by `fs-err`,
since we know the context in which the errors have occurred.

For example, in the error message for `ReadOptionalFileError`s we know
not to say to check for a missing file or that the file is a directory,
since `read_optional_file()` explicitly excludes those cases by design.
If we used `io::Error` (or `fs-err`'s equivalent) we would have to rely
on the user not using the wrong logging function for the wrong I/O error
case, rather than relying on the type system to do that for us.

These changes leave `log_io_error()` as being used only for the
`Command` I/O cases. A later PR will switch adjust those usages too
(but this PR is already large enough).

Closes #270.
GUS-W-12650236.

Author: edmorley

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Improved the error messages shown when `.python-version` contains an invalid Python version or stray invisible characters (such as ASCII control codes). ([#353](https://github.com/heroku/buildpacks-python/pull/353) and [#354](https://github.com/heroku/buildpacks-python/pull/354))
+- Improved the error messages shown if I/O errors occur. ([#355](https://github.com/heroku/buildpacks-python/pull/355))
 
 ## [0.26.1] - 2025-04-08
 

src/detect.rs

@@ -1,4 +1,4 @@
-use std::io;
+use crate::utils::{self, FileExistsError};
 use std::path::Path;
 
 /// Filenames that if found in a project mean it should be treated as a Python project,
@@ -40,11 +40,10 @@ const KNOWN_PYTHON_PROJECT_FILES: [&str; 23] = [
 
 /// Returns whether the specified project directory is that of a Python project, and so
 /// should pass buildpack detection.
-pub(crate) fn is_python_project_directory(app_dir: &Path) -> io::Result<bool> {
+pub(crate) fn is_python_project_directory(app_dir: &Path) -> Result<bool, FileExistsError> {
     // Until `Iterator::try_find` is stabilised, this is cleaner as a for loop.
     for filename in KNOWN_PYTHON_PROJECT_FILES {
-        let path = app_dir.join(filename);
-        if path.try_exists()? {
+        if utils::file_exists(&app_dir.join(filename))? {
             return Ok(true);
         }
     }
@@ -71,7 +70,9 @@ mod tests {
 
     #[test]
     fn is_python_project_directory_io_error() {
-        assert!(is_python_project_directory(Path::new("tests/fixtures/empty/.gitkeep")).is_err());
+        // We pass a path containing a NUL byte as an easy way to trigger an I/O error.
+        let err = is_python_project_directory(Path::new("\0/invalid")).unwrap_err();
+        assert_eq!(err.path, Path::new("\0/invalid/.python-version"));
     }
 
     #[test]

src/django.rs

@@ -1,15 +1,17 @@
+use crate::FileExistsError;
 use crate::utils::{self, CapturedCommandError, StreamedCommandError};
 use indoc::indoc;
 use libcnb::Env;
 use libherokubuildpack::log::log_info;
-use std::io;
 use std::path::Path;
 use std::process::Command;
 
 const MANAGEMENT_SCRIPT_NAME: &str = "manage.py";
 
-pub(crate) fn is_django_installed(dependencies_layer_dir: &Path) -> io::Result<bool> {
-    dependencies_layer_dir.join("bin/django-admin").try_exists()
+pub(crate) fn is_django_installed(dependencies_layer_dir: &Path) -> Result<bool, FileExistsError> {
+    // The Django package includes a `django-admin` entrypoint script, which we can use
+    // as a simple/fast way to check if Django is installed.
+    utils::file_exists(&dependencies_layer_dir.join("bin/django-admin"))
 }
 
 pub(crate) fn run_django_collectstatic(
@@ -54,8 +56,8 @@ pub(crate) fn run_django_collectstatic(
     .map_err(DjangoCollectstaticError::CollectstaticCommand)
 }
 
-fn has_management_script(app_dir: &Path) -> io::Result<bool> {
-    app_dir.join(MANAGEMENT_SCRIPT_NAME).try_exists()
+fn has_management_script(app_dir: &Path) -> Result<bool, FileExistsError> {
+    utils::file_exists(&app_dir.join(MANAGEMENT_SCRIPT_NAME))
 }
 
 fn has_collectstatic_command(app_dir: &Path, env: &Env) -> Result<bool, CapturedCommandError> {
@@ -87,7 +89,7 @@ fn has_collectstatic_command(app_dir: &Path, env: &Env) -> Result<bool, Captured
 #[derive(Debug)]
 pub(crate) enum DjangoCollectstaticError {
     CheckCollectstaticCommandExists(CapturedCommandError),
-    CheckManagementScriptExists(io::Error),
+    CheckManagementScriptExists(FileExistsError),
     CollectstaticCommand(StreamedCommandError),
 }
 
@@ -110,6 +112,8 @@ mod tests {
 
     #[test]
     fn has_management_script_io_error() {
-        assert!(has_management_script(Path::new("tests/fixtures/empty/.gitkeep")).is_err());
+        // We pass a path containing a NUL byte as an easy way to trigger an I/O error.
+        let err = has_management_script(Path::new("\0/invalid")).unwrap_err();
+        assert_eq!(err.path, Path::new("\0/invalid/manage.py"));
     }
 }

src/errors.rs

@@ -13,7 +13,10 @@ use crate::python_version::{
     ResolvePythonVersionError,
 };
 use crate::python_version_file::ParsePythonVersionFileError;
-use crate::utils::{CapturedCommandError, DownloadUnpackArchiveError, StreamedCommandError};
+use crate::utils::{
+    CapturedCommandError, DownloadUnpackArchiveError, FileExistsError, FindBundledPipError,
+    ReadOptionalFileError, StreamedCommandError,
+};
 use indoc::{formatdoc, indoc};
 use libherokubuildpack::log::log_error;
 use std::io;
@@ -49,11 +52,12 @@ pub(crate) fn on_error(error: libcnb::Error<BuildpackError>) {
 
 fn on_buildpack_error(error: BuildpackError) {
     match error {
-        BuildpackError::BuildpackDetection(error) => on_buildpack_detection_error(&error),
+        BuildpackError::BuildpackDetection(error) | BuildpackError::DjangoDetection(error) => {
+            log_file_exists_error(error);
+        }
         BuildpackError::Checks(error) => on_buildpack_checks_error(error),
         BuildpackError::DeterminePackageManager(error) => on_determine_package_manager_error(error),
         BuildpackError::DjangoCollectstatic(error) => on_django_collectstatic_error(error),
-        BuildpackError::DjangoDetection(error) => on_django_detection_error(&error),
         BuildpackError::PipDependenciesLayer(error) => on_pip_dependencies_layer_error(error),
         BuildpackError::PipLayer(error) => on_pip_layer_error(error),
         BuildpackError::PoetryDependenciesLayer(error) => on_poetry_dependencies_layer_error(error),
@@ -64,14 +68,6 @@ fn on_buildpack_error(error: BuildpackError) {
     }
 }
 
-fn on_buildpack_detection_error(error: &io::Error) {
-    log_io_error(
-        "Unable to complete buildpack detection",
-        "determining if the Python buildpack should be run for this application",
-        error,
-    );
-}
-
 fn on_buildpack_checks_error(error: ChecksError) {
     match error {
         ChecksError::ForbiddenEnvVar(name) => log_error(
@@ -89,11 +85,7 @@ fn on_buildpack_checks_error(error: ChecksError) {
 
 fn on_determine_package_manager_error(error: DeterminePackageManagerError) {
     match error {
-        DeterminePackageManagerError::CheckFileExists(io_error) => log_io_error(
-            "Unable to determine the package manager",
-            "determining which Python package manager to use for this project",
-            &io_error,
-        ),
+        DeterminePackageManagerError::CheckFileExists(error) => log_file_exists_error(error),
         DeterminePackageManagerError::MultipleFound(package_managers) => {
             let files_found = package_managers
                 .into_iter()
@@ -141,16 +133,8 @@ fn on_determine_package_manager_error(error: DeterminePackageManagerError) {
 
 fn on_requested_python_version_error(error: RequestedPythonVersionError) {
     match error {
-        RequestedPythonVersionError::CheckRuntimeTxtExists(io_error) => log_io_error(
-            "Unable to check for runtime.txt",
-            "checking if a runtime.txt file exists",
-            &io_error,
-        ),
-        RequestedPythonVersionError::ReadPythonVersionFile(io_error) => log_io_error(
-            "Unable to read .python-version",
-            "reading the .python-version file",
-            &io_error,
-        ),
+        RequestedPythonVersionError::CheckRuntimeTxtExists(error) => log_file_exists_error(error),
+        RequestedPythonVersionError::ReadPythonVersionFile(error) => log_read_file_error(error),
         RequestedPythonVersionError::ParsePythonVersionFile(error) => match error {
             ParsePythonVersionFileError::InvalidVersion(version) => log_error(
                 "Invalid Python version in .python-version",
@@ -384,11 +368,7 @@ fn on_pip_layer_error(error: PipLayerError) {
                 "},
             ),
         },
-        PipLayerError::LocateBundledPip(io_error) => log_io_error(
-            "Unable to locate the bundled copy of pip",
-            "locating the pip wheel file bundled inside the Python 'ensurepip' module",
-            &io_error,
-        ),
+        PipLayerError::LocateBundledPip(error) => log_find_bundled_pip_error(error),
     }
 }
 
@@ -455,11 +435,7 @@ fn on_poetry_layer_error(error: PoetryLayerError) {
                 "},
             ),
         },
-        PoetryLayerError::LocateBundledPip(io_error) => log_io_error(
-            "Unable to locate the bundled copy of pip",
-            "locating the pip wheel file bundled inside the Python 'ensurepip' module",
-            &io_error,
-        ),
+        PoetryLayerError::LocateBundledPip(error) => log_find_bundled_pip_error(error),
     }
 }
 
@@ -501,14 +477,6 @@ fn on_poetry_dependencies_layer_error(error: PoetryDependenciesLayerError) {
     }
 }
 
-fn on_django_detection_error(error: &io::Error) {
-    log_io_error(
-        "Unable to determine if this is a Django-based app",
-        "checking if the 'django-admin' command exists",
-        error,
-    );
-}
-
 fn on_django_collectstatic_error(error: DjangoCollectstaticError) {
     match error {
         DjangoCollectstaticError::CheckCollectstaticCommandExists(error) => match error {
@@ -537,11 +505,9 @@ fn on_django_collectstatic_error(error: DjangoCollectstaticError) {
                 },
             ),
         },
-        DjangoCollectstaticError::CheckManagementScriptExists(io_error) => log_io_error(
-            "Unable to inspect Django configuration",
-            "checking if the 'manage.py' script exists",
-            &io_error,
-        ),
+        DjangoCollectstaticError::CheckManagementScriptExists(error) => {
+            log_file_exists_error(error);
+        }
         DjangoCollectstaticError::CollectstaticCommand(error) => match error {
             StreamedCommandError::Io(io_error) => log_io_error(
                 "Unable to generate Django static files",
@@ -571,6 +537,8 @@ fn on_django_collectstatic_error(error: DjangoCollectstaticError) {
     }
 }
 
+// This is now only used for Command I/O errors.
+// TODO: Replace this with specialised handling for Command I/O errors.
 fn log_io_error(header: &str, occurred_whilst: &str, io_error: &io::Error) {
     // We don't suggest opening a support ticket, since a subset of I/O errors can be caused
     // by issues in the application. In the future, perhaps we should try and split these out?
@@ -583,3 +551,70 @@ fn log_io_error(header: &str, occurred_whilst: &str, io_error: &io::Error) {
         "},
     );
 }
+
+fn log_file_exists_error(FileExistsError { path, io_error }: FileExistsError) {
+    let filepath = path.to_string_lossy();
+    let filename = path
+        .file_name()
+        .unwrap_or(path.as_os_str())
+        .to_string_lossy();
+
+    log_error(
+        format!("Unable to check if {filename} exists"),
+        formatdoc! {"
+            An I/O error occurred while checking if this file exists:
+            {filepath}
+
+            Details: {io_error}
+
+            Try building again to see if the error resolves itself.
+        "},
+    );
+}
+
+fn log_read_file_error(ReadOptionalFileError { path, io_error }: ReadOptionalFileError) {
+    let filepath = path.to_string_lossy();
+    let filename = path
+        .file_name()
+        .unwrap_or(path.as_os_str())
+        .to_string_lossy();
+
+    log_error(
+        format!("Unable to read {filename}"),
+        formatdoc! {"
+            An I/O error occurred while reading the file:
+            {filepath}
+
+            Details: {io_error}
+
+            Check the file's permissions and that it contains valid UTF-8.
+
+            Then try building again.
+        "},
+    );
+}
+
+fn log_find_bundled_pip_error(
+    FindBundledPipError {
+        bundled_wheels_dir,
+        io_error,
+    }: FindBundledPipError,
+) {
+    let bundled_wheels_dir = bundled_wheels_dir.to_string_lossy();
+
+    // TODO: Decide whether we want users to file a bug or open a support ticket.
+    log_error(
+        "Unable to locate the Python stdlib's bundled pip",
+        formatdoc! {"
+            Couldn't find the pip wheel file bundled inside the Python
+            stdlib's `ensurepip` module, at:
+            {bundled_wheels_dir}
+
+            Details: {io_error}
+
+            This is an internal error. Please report it as a bug, here:
+            https://github.com/heroku/buildpacks-python/issues
+        "
+        },
+    );
+}

src/layers/pip.rs

@@ -1,6 +1,6 @@
 use crate::packaging_tool_versions::PIP_VERSION;
 use crate::python_version::PythonVersion;
-use crate::utils::StreamedCommandError;
+use crate::utils::{FindBundledPipError, StreamedCommandError};
 use crate::{BuildpackError, PythonBuildpack, utils};
 use libcnb::Env;
 use libcnb::build::BuildContext;
@@ -11,7 +11,6 @@ use libcnb::layer::{
 use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope};
 use libherokubuildpack::log::log_info;
 use serde::{Deserialize, Serialize};
-use std::io;
 use std::path::Path;
 use std::process::Command;
 
@@ -133,7 +132,7 @@ struct PipLayerMetadata {
 #[derive(Debug)]
 pub(crate) enum PipLayerError {
     InstallPipCommand(StreamedCommandError),
-    LocateBundledPip(io::Error),
+    LocateBundledPip(FindBundledPipError),
 }
 
 impl From<PipLayerError> for libcnb::Error<BuildpackError> {

src/layers/poetry.rs

@@ -1,6 +1,6 @@
 use crate::packaging_tool_versions::POETRY_VERSION;
 use crate::python_version::PythonVersion;
-use crate::utils::StreamedCommandError;
+use crate::utils::{FindBundledPipError, StreamedCommandError};
 use crate::{BuildpackError, PythonBuildpack, utils};
 use libcnb::Env;
 use libcnb::build::BuildContext;
@@ -11,7 +11,6 @@ use libcnb::layer::{
 use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope};
 use libherokubuildpack::log::log_info;
 use serde::{Deserialize, Serialize};
-use std::io;
 use std::path::Path;
 use std::process::Command;
 
@@ -134,7 +133,7 @@ struct PoetryLayerMetadata {
 #[derive(Debug)]
 pub(crate) enum PoetryLayerError {
     InstallPoetryCommand(StreamedCommandError),
-    LocateBundledPip(io::Error),
+    LocateBundledPip(FindBundledPipError),
 }
 
 impl From<PoetryLayerError> for libcnb::Error<BuildpackError> {

src/main.rs

@@ -22,13 +22,13 @@ use crate::python_version::{
     PythonVersionOrigin, RequestedPythonVersion, RequestedPythonVersionError,
     ResolvePythonVersionError,
 };
+use crate::utils::FileExistsError;
 use indoc::formatdoc;
 use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder};
 use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder};
 use libcnb::generic::{GenericMetadata, GenericPlatform};
 use libcnb::{Buildpack, Env, buildpack_main};
 use libherokubuildpack::log::{log_header, log_info, log_warning};
-use std::io;
 
 struct PythonBuildpack;
 
@@ -157,15 +157,15 @@ impl Buildpack for PythonBuildpack {
 #[derive(Debug)]
 pub(crate) enum BuildpackError {
     /// I/O errors when performing buildpack detection.
-    BuildpackDetection(io::Error),
+    BuildpackDetection(FileExistsError),
     /// Errors due to one of the environment checks failing.
     Checks(ChecksError),
     /// Errors determining which Python package manager to use for a project.
     DeterminePackageManager(DeterminePackageManagerError),
     /// Errors running the Django collectstatic command.
     DjangoCollectstatic(DjangoCollectstaticError),
     /// I/O errors when detecting whether Django is installed.
-    DjangoDetection(io::Error),
+    DjangoDetection(FileExistsError),
     /// Errors installing the project's dependencies into a layer using pip.
     PipDependenciesLayer(PipDependenciesLayerError),
     /// Errors installing pip into a layer.