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

Don't catch SomeException #411

Closed
wants to merge 1 commit into from
Closed

Conversation

eborden
Copy link
Collaborator

@eborden eborden commented Jun 10, 2019

Exception handling was overly broad in some places. We only need to
catch IOException.

Control.Exception can be used to catch IOException, which requires
ScopedTypeVariables or we can use catchIOError. This commit chooses
catchIOError for its simplicity.

Closes #376

Exception handling was overly broad in some places. We only need to
catch `IOException`.

`Control.Exception` can be used to catch `IOException`, which requires
`ScopedTypeVariables` or we can use `catchIOError`. This commit chooses
`catchIOError` for its simplicity.
@eborden eborden requested a review from kazu-yamamoto June 10, 2019 00:37
@eborden eborden self-assigned this Jun 10, 2019
Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

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

LGTM.

kazu-yamamoto added a commit to kazu-yamamoto/network that referenced this pull request Jun 10, 2019
@kazu-yamamoto
Copy link
Collaborator

Rebased, added two other commits and merged.

@kazu-yamamoto kazu-yamamoto deleted the eborden/only-catch-io-exceptions branch June 10, 2019 01:59
@maoe maoe mentioned this pull request Jun 20, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overly broad exception handlers catching asynchronous exceptions
2 participants