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

Capture exception messages of raised NotFound and BadRequest #1210

Merged
merged 1 commit into from Jan 14, 2017

Conversation

Projects
None yet
2 participants
@mwpastore
Member

mwpastore commented Nov 29, 2016

No description provided.

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Dec 25, 2016

Member

@mwpastore Could you rebase please?

Member

zzak commented Dec 25, 2016

@mwpastore Could you rebase please?

@zzak zzak added this to the 2.0.0 milestone Dec 25, 2016

@zzak zzak added the feature label Dec 25, 2016

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Dec 25, 2016

Member

cc #1216

Member

zzak commented Dec 25, 2016

cc #1216

@mwpastore

This comment has been minimized.

Show comment
Hide comment
@mwpastore

mwpastore Dec 27, 2016

Member

@zzak I've noticed that we are reimplementing a lot of Rack::Response in Sinatra::Base::Helpers. I'm guessing this is due to the fact that Rack has improved over time to include more and more functionality. Can we use the delegation pattern to clean this up? Take a look at what I've done here. We can remove a bunch of tests, too, if we think this is a good idea.

There's a lot going on in this commit all of a sudden so let me know if you'd like me to break it up.

Member

mwpastore commented Dec 27, 2016

@zzak I've noticed that we are reimplementing a lot of Rack::Response in Sinatra::Base::Helpers. I'm guessing this is due to the fact that Rack has improved over time to include more and more functionality. Can we use the delegation pattern to clean this up? Take a look at what I've done here. We can remove a bunch of tests, too, if we think this is a good idea.

There's a lot going on in this commit all of a sudden so let me know if you'd like me to break it up.

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Dec 28, 2016

Member

@mwpastore I'm not against using delegators but this close to a release -- I want to make sure this won't affect any behavior or have performance impact.

With that said, something like this could be investigated after the release is made.

Member

zzak commented Dec 28, 2016

@mwpastore I'm not against using delegators but this close to a release -- I want to make sure this won't affect any behavior or have performance impact.

With that said, something like this could be investigated after the release is made.

@mwpastore mwpastore changed the title from Capture bodies of raised NotFound and BadRequest errors to Capture bodies of raised NotFound and BadRequest exceptions Jan 14, 2017

@mwpastore

This comment has been minimized.

Show comment
Hide comment
@mwpastore

mwpastore Jan 14, 2017

Member

Okay, I pulled the gnarly stuff out into a separate branch for later perusal and refocused this PR back to its original purpose. Let me know if you want to see any other changes!

Member

mwpastore commented Jan 14, 2017

Okay, I pulled the gnarly stuff out into a separate branch for later perusal and refocused this PR back to its original purpose. Let me know if you want to see any other changes!

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jan 14, 2017

Member

For a second, I thought "body" was the body of the request, which would probably not be a good idea to print.

So can you change the wording from "body" -> "exception message"?

Member

zzak commented Jan 14, 2017

For a second, I thought "body" was the body of the request, which would probably not be a good idea to print.

So can you change the wording from "body" -> "exception message"?

@mwpastore

This comment has been minimized.

Show comment
Hide comment
@mwpastore

mwpastore Jan 14, 2017

Member

How's that?

Member

mwpastore commented Jan 14, 2017

How's that?

@zzak zzak merged commit 9bd0d40 into sinatra:master Jan 14, 2017

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@mwpastore mwpastore changed the title from Capture bodies of raised NotFound and BadRequest exceptions to Capture exception messages of raised NotFound and BadRequest Jan 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment