8000 Human readable OSError messages by godlygeek · Pull Request #837 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

Human readable OSError messages #837

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 15, 2018
Merged

Human readable OSError messages #837

merged 3 commits into from
May 15, 2018

Conversation

godlygeek
Copy link

For several common errnos, include a human-friendly string in the exception string. For now, only enable this on boards with external SPI flash, because pulling the uerrno module in eats up an extra 1kb of flash.

Likewise, this only adds error strings for common exceptions to avoid using flash unnecessarily - others will just show up as the EXXX string, but meaningful descriptions can be added later if needed.

Tested with a Feather M0 Express REPL:

Adafruit CircuitPython 3.0.0-alpha.6-75-gcb8c159-dirty on 2018-05-14; Adafruit Feather M0 Express with samd21g18
>>> import uerrno
>>> for err in dir(uerrno):
...     if err.startswith('E'):
...         print(OSError(getattr(uerrno, err)))
...
...
...
[Errno 1] Permission denied
[Errno 2] No such file/directory
[Errno 5] Input/output error
[Errno 9] EBADF
[Errno 11] EAGAIN
[Errno 12] ENOMEM
[Errno 13] Permission denied
[Errno 17] File exists
[Errno 19] Unsupported operation
[Errno 21] EISDIR
[Errno 22] Invalid argument
[Errno 95] EOPNOTSUPP
[Errno 112] EADDRINUSE
[Errno 113] ECONNABORTED
[Errno 104] ECONNRESET
[Errno 105] ENOBUFS
[Errno 128] ENOTCONN
[Errno 116] ETIMEDOUT
[Errno 111] ECONNREFUSED
[Errno 118] EHOSTUNREACH
[Errno 120] EALREADY
[Errno 119] EINPROGRESS

Signed-off-by: Matt Wozniski mwozniski@bloomberg.net

godlygeek added 2 commits May 14, 2018 15:12
@godlygeek
Include uerrno for boards with external SPI flash
0518cc1 
Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek
Display human readable strings for common OSErrors
5833cbe 
Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@dhalbert
Copy link
Collaborator
dhalbert commented May 14, 2018 •
edited
Loading

The Traivs Unix build runs the tests - one test is showing unexpected output. May just need to modify the test, or handle that case.
https://travis-ci.org/adafruit/circuitpython/jobs/378877220

### Expected
b"<class 'int'>\n[Errno  ] EIO\n9999\n"
### Actual
b"<class 'int'>\n[Errno  error\n9999\n"
FAIL  basics/errno1.py

@godlygeek
Update errno tests to match new expected behavior
086bffb 
Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
@godlygeek godlygeek force-pushed the human_readable_oserror branch from ecc7fff to 086bffb Compare May 15, 2018 03:16
@godlygeek
Copy link
Author

The test did need to be updated; I've done so. The test was checking the output of:

str(OSError(uerrno.EIO))

Previously it would return something like '[Errno 5] EIO', but because the "5" is platform specific, the test was checking only the first 7 characters ([Errno ) and the last 5 (] EIO).

We deliberately changed the message to '[Errno 5] Input/output error' and so looking at only the last 5 was no longer correct. The test is now updated to look for everything after the first ] instead of only the last 5 characters, and to expect "Input/output error" instead of "EIO" as the description. I also added a test for one that we didn't give a human-readable description to, to show that we still stringify the error name in that case.

dhalbert
dhalbert approved these changes May 15, 2018
View reviewed changes
Copy link
Collaborator
@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much!! This is a huge improvement in clarity.

@dhalbert dhalbert merged commit 5429339 into adafruit:master May 15, 2018
@godlygeek godlygeek mentioned this pull request May 15, 2018
Closed
This was referenced May 16, 2018
Closed
Merged
@godlygeek godlygeek mentioned this pull request Jun 1, 2018
Merged
@Gigahawk Gigahawk mentioned this pull request Nov 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhalbert dhalbert dhalbert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@godlygeek @dhalbert
0