Skip to content
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

Improve Error Handling #59

Merged
merged 8 commits into from
Feb 7, 2016
Merged

Improve Error Handling #59

merged 8 commits into from
Feb 7, 2016

Conversation

opalmer
Copy link
Owner

@opalmer opalmer commented Feb 7, 2016

This PR improves on the error handling by:

  • Ensuring WindowsAPIError is more consistent and better documented
  • Simplifying error_check and ensuring it's easier to tell what the original error was.

This came about while working on #57 where internal errors were sometimes obscuring the underlying API errors. There are probably further improvements to be made here but this should simplify future work.

There's a few reasons for these change:

  * The things prefixed with api_* didn't
    really describe if the thing being referenced
    was a Windows API or our own.
  * Attributes like errno and code are typical
    on other kinds of errors so people will likely
    expect these.
  * There was not any documentation for the input
    arguments before.
  * __repr__() should show the complete error.
@opalmer opalmer self-assigned this Feb 7, 2016
@opalmer opalmer added this to the 0.1.3 milestone Feb 7, 2016
@codecov-io
Copy link

Current coverage is 95.59%

Merging #59 into master will decrease coverage by -0.86% as of b875ee0

Powered by Codecov. Updated on successful CI builds.

There have been a couple of refactors since
the last release with API changes.  These will
be much more limited in the future.
opalmer added a commit that referenced this pull request Feb 7, 2016
@opalmer opalmer merged commit f216c61 into master Feb 7, 2016
@opalmer opalmer deleted the improve_error_handling branch February 7, 2016 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants