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

Determine if Response body is populated #1216

Closed
bradleybeddoes opened this issue Jun 14, 2017 · 7 comments
Closed

Determine if Response body is populated #1216

bradleybeddoes opened this issue Jun 14, 2017 · 7 comments

Comments

@bradleybeddoes
Copy link
Contributor

bradleybeddoes commented Jun 14, 2017

I was looking through the api provided for Response for a way to determine if body has been populated (or not) by some other part of the application. I can't immediately see a way to do this however.

I note that body is an Option per https://github.com/hyperium/hyper/blob/master/src/http/response.rs#L15 so simply being able to access Option::is_some would suit my use case.

Alternatively Response could be extended with a bool that could be used to indicate by application code that the current response is considered to be 'committed' and probably should not be further altered.

I'd be happy to provide a PR for either (or both) but I wanted to find out if this would be palatable before proceeding.

@seanmonstar
Copy link
Member

This is definitely something that we're interested in, see #1164 for some more comments and exploration.

As for your case, are you interested in whether some user has set a body, or whether there is body coming from the socket? The former could maybe be determined today, that depends on the specific Body type that is used. The latter can be hard to tell.

@bradleybeddoes
Copy link
Contributor Author

bradleybeddoes commented Jun 20, 2017

Thanks for the link to #1164 @seanmonstar that was interesting background reading.

Currently I am most interested in determining if another part of the application has set a Body or not on the Response object which is about to be sent back over the wire (or alternatively that same part of the application has indicated that the Response is considered to be done and nothing else should touch it).

The concept is that when a Body has been set (or the Response is marked done) this will flow through unimpeded. When this is not the case however, custom code, which is invoked based on the Status of the Response, will be executed to (potentially) set a Body.

For example adding JSON to the Body with error details which have resulted from processing an invalid Request within a RESTful API so you don't need to replicate error handling/population code in multiple locations.

Hopefully that makes sense, little bit of weird one to explain, happy to provide a code example if that would help.

@seanmonstar
Copy link
Member

Ah, so would it be safe to say you have a much more localized desire, than the larger scope of that issue I linked?

It seems there is Request::body_ref(), and that a mirrored Response::body_ref() would allow you to see if it's been set, right?

@bradleybeddoes
Copy link
Contributor Author

#1164 is certainly more advanced than what I need right now.

Response::body_ref() would work out nicely - shall I create a PR?

@bradleybeddoes
Copy link
Contributor Author

Hey @seanmonstar, apologies, this fell off my radar for a short period. Are you happy for me to create a PR for the functionality discussed above or would you prefer to handle within some other work you have on the go?

@seanmonstar
Copy link
Member

I'm concerned that we want to move the Option out, and letting the Body type itself be an option or something. If so, then adding another method with the Option hardcoded feels wrong. But really, it feels like a breaking change to remove the Option anyways, and so maybe we may as well have the accessors for now, and just know they will have a type change in 0.12...

@bradleybeddoes
Copy link
Contributor Author

Thanks, have submitted #1244 noting potential change of api in the future.

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

No branches or pull requests

2 participants