Skip to content

Improve error handling#162

Merged
carllerche merged 5 commits intocarllerche:masterfrom
manifest:feature/errors
Nov 30, 2018
Merged

Improve error handling#162
carllerche merged 5 commits intocarllerche:masterfrom
manifest:feature/errors

Conversation

@manifest
Copy link
Contributor

@manifest manifest commented Nov 29, 2018

The PR for the issue.

@manifest manifest changed the title [WIP] Improve error handling Improve error handling Nov 29, 2018
Copy link
Owner

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. The strategy looks good. We need to maintain backwards compat for now though. Commented inline

}

/// Returns `true` if `self` represents a 400 -- bad request error
pub fn is_bad_request(&self) -> bool {
Copy link
Owner

Choose a reason for hiding this comment

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

These functions need to stay as they are part of the public API. They can be considered for removal in the next breaking release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I see.

Copy link
Owner

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

LGTM. Happy to merge whenever you are ready (I still see WIP in the title).

@carllerche
Copy link
Owner

Woops, the WIP is gone, it was a relic of my email...

@carllerche carllerche merged commit 3d83764 into carllerche:master Nov 30, 2018
@manifest
Copy link
Contributor Author

Thanks!

@manifest manifest deleted the feature/errors branch November 30, 2018 19:01
kornholi pushed a commit to kornholi/tower-web that referenced this pull request Dec 3, 2019
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.

2 participants