Skip to content

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

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

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

edmorley
Copy link
Member

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 ReadOptionalFileErrors 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.

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 wouldn't 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.
@edmorley edmorley added the enhancement New feature or request label Apr 28, 2025
@edmorley edmorley self-assigned this Apr 28, 2025
@edmorley edmorley marked this pull request as ready for review April 28, 2025 16:10
@edmorley edmorley requested a review from a team as a code owner April 28, 2025 16:10
@edmorley edmorley enabled auto-merge (squash) April 28, 2025 16:12
@edmorley edmorley merged commit 7d957ec into main Apr 29, 2025
8 checks passed
@edmorley edmorley deleted the improve-file-io-errors branch April 29, 2025 08:22
edmorley added a commit that referenced this pull request Apr 29, 2025
Similar to #355 but for I/O errors from spawning a `Command`
instead of from file operations.

Such Command I/O errors are only likely to happen in the case
of a bug, so the error message now says as much. The error
also now generates the program name from the command,
so avoid it getting out of sync. The command's args are
intentionally omitted, since they aren't relevant for errors
that occur when spawning a command. (ie: For command not
found, it's the program that's not found, not the args.)

GUS-W-12650236.
edmorley added a commit that referenced this pull request Apr 29, 2025
Similar to #355 but for I/O errors from spawning a `Command`
instead of from file operations.

Such Command I/O errors are only likely to happen in the case
of a bug, so the error message now says as much. The error
also now generates the program name from the command,
so avoid it getting out of sync. The command's args are
intentionally omitted, since they aren't relevant for errors
that occur when spawning a command. (ie: For command not
found, it's the program that's not found, not the args.)

GUS-W-12650236.
edmorley added a commit that referenced this pull request Apr 29, 2025
Similar to #355 but for I/O errors from spawning a `Command`
instead of from file operations.

Such Command I/O errors are only likely to happen in the case
of a bug, so the error message now says as much. The error
also now generates the program name from the command,
so avoid it getting out of sync. The command's args are
intentionally omitted, since they aren't relevant for errors
that occur when spawning a command. (ie: For command not
found, it's the program that's not found, not the args.)

GUS-W-12650236.
heroku-linguist bot added a commit that referenced this pull request May 2, 2025
## heroku/python

### Removed

- Removed support for the deprecated `runtime.txt` file. Python versions must now be specified using a `.python-version` file instead. ([#352](#352))
- Removed support for Ubuntu 20.04 (and thus Heroku-20 / `heroku/builder:20`). ([#358](#358))

### Changed

- Improved the error messages shown when `.python-version` contains an invalid Python version or stray invisible characters (such as ASCII control codes). ([#353](#353) and [#354](#354))
- Improved the error messages shown if I/O errors occur. ([#355](#355) and [#356](#356))
@heroku-linguist heroku-linguist bot mentioned this pull request May 2, 2025
heroku-linguist bot added a commit to heroku/cnb-builder-images that referenced this pull request May 2, 2025
## heroku/python

### Removed

- Removed support for the deprecated `runtime.txt` file. Python versions must now be specified using a `.python-version` file instead. ([#352](heroku/buildpacks-python#352))
- Removed support for Ubuntu 20.04 (and thus Heroku-20 / `heroku/builder:20`). ([#358](heroku/buildpacks-python#358))

### Changed

- Improved the error messages shown when `.python-version` contains an invalid Python version or stray invisible characters (such as ASCII control codes). ([#353](heroku/buildpacks-python#353) and [#354](heroku/buildpacks-python#354))
- Improved the error messages shown if I/O errors occur. ([#355](heroku/buildpacks-python#355) and [#356](heroku/buildpacks-python#356))
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request May 2, 2025
Backports the improvements made as part of:
- heroku/buildpacks-python#352
- heroku/buildpacks-python#353
- heroku/buildpacks-python#354
- heroku/buildpacks-python#355

Plus:
- applies similar changes to equivalent error messages that
  only exist in the classic buildpack (eg the Pipenv errors)
- switches to use of contractions as per the CX team's
  style guidelines

GUS-W-18225347.
GUS-W-18421778.
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request May 2, 2025
Backports the error message/build output improvements made as part of:
- heroku/buildpacks-python#352
- heroku/buildpacks-python#353
- heroku/buildpacks-python#354
- heroku/buildpacks-python#355

Plus:
- applies similar changes to equivalent error messages that
  only exist in the classic buildpack (eg the Pipenv errors)
- switches to use of contractions as per the CX team's
  style guidelines

GUS-W-18225347.
GUS-W-18421778.
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request May 2, 2025
Backports the error message/build output improvements made as part of:
- heroku/buildpacks-python#352
- heroku/buildpacks-python#353
- heroku/buildpacks-python#354
- heroku/buildpacks-python#355

Plus:
- applies similar changes to equivalent error messages that
  only exist in the classic buildpack (eg the Pipenv errors)
- switches to use of contractions as per the CX team's
  style guidelines

GUS-W-18225347.
GUS-W-18421778.
edmorley added a commit to heroku/heroku-buildpack-python that referenced this pull request May 2, 2025
Backports the error message/build output improvements made as part of:
- heroku/buildpacks-python#352
- heroku/buildpacks-python#353
- heroku/buildpacks-python#354
- heroku/buildpacks-python#355

Plus:
- applies similar changes to equivalent error messages that
  only exist in the classic buildpack (eg the Pipenv errors)
- switches to use of contractions as per the CX team's
  style guidelines

GUS-W-18225347.
GUS-W-18421778.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider using fs-err to improve I/O error messages
2 participants