Repository: seanmonstar/reqwest (language: rust)

Issue:
Add Response::text()?
Currently, you can run into a (somewhat) surprising condition when you read the body of a Response twice.

* `resp.read_to_string(&mut s);
* `resp.json()`

or just

* `resp.read_to_string(&mut s);
* `resp.read_to_string(&mut s);

While to many it will occur quickly that the problem is reading the response twice, I think this can be improved.

`.json()` already consumes the response, but the `Read` trait methods do not.

Maybe there could be a `body()` method which returns a `Read` and also consumes the response.


Discussion / hints:
The returned `Read` would have the same "issue", that you can try to read from it multiple times. This is just the design in Rust. For things where you can rewind, there is the `Seek` trait, and `Response` doesn't implement it, because you can't rewind.

----

Alternatively, since it seems to be so common to read the body to a string, I wonder if there is value in providing `response.string()`...
> The returned Read would have the same "issue", that you can try to read from it multiple times.

Good point.
`response.text()` would be my choice. In fact that is what I have in `requests`
+1 for a `.string()` or similar.
I can add this in
I currently have it here in question form as I'd like to hear if it's worth having, or to continue to recommend the `io::Read` way of `read_to_string(&mut s)`...
I see. I think it's worth having since it's probably the most likely case other than deserializing into json. The convenience and the consumption of the response is certainly a plus.
One argument in favor of adding the method: without this, you have to `use std::io::Read;`, `let mut s = String::new();`, and only then can you call `read_to_string(&mut s)`. That's much less convenient than `.text()`.

Fix patch:
diff --git a/src/response.rs b/src/response.rs
index a8f81e314..f2defe918 100644
--- a/src/response.rs
+++ b/src/response.rs
@@ -165,6 +165,26 @@ impl Response {
         serde_json::from_reader(self).map_err(::error::from)
     }
 
+    /// Get the response text.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// # extern crate reqwest;
+    /// # fn run() -> Result<(), Box<::std::error::Error>> {
+    /// let content = reqwest::get("http://httpbin.org/range/26")?.text()?;
+    /// # Ok(())
+    /// # }
+    /// ```
+    pub fn text(&mut self) -> ::Result<String> {
+        let len = self.headers().get::<::header::ContentLength>()
+            .map(|ct_len| **ct_len)
+            .unwrap_or(0);
+        let mut content = String::with_capacity(len as usize);
+        self.read_to_string(&mut content).map_err(::error::from)?;
+        Ok(content)
+    }
+
     /// Copy the response body into a writer.
     ///
     /// This function internally uses [`std::io::copy`] and hence will continuously read data from
@@ -185,10 +205,10 @@ impl Response {
     /// # }
     /// ```
     #[inline]
-    pub fn copy_to<W: ?Sized>(&mut self, w: &mut W) -> io::Result<u64>
+    pub fn copy_to<W: ?Sized>(&mut self, w: &mut W) -> ::Result<u64>
         where W: io::Write
     {
-        io::copy(self, w)
+        io::copy(self, w).map_err(::error::from)
     }
 
     /// Turn a response into an error if the server returned an error.


Test patch:
diff --git a/tests/client.rs b/tests/client.rs
index a302a2a29..65731e935 100644
--- a/tests/client.rs
+++ b/tests/client.rs
@@ -5,6 +5,39 @@ mod support;
 
 use std::io::Read;
 
+#[test]
+fn test_response_text() {
+    let server = server! {
+        request: b"\
+            GET /text HTTP/1.1\r\n\
+            Host: $HOST\r\n\
+            User-Agent: $USERAGENT\r\n\
+            Accept: */*\r\n\
+            Accept-Encoding: gzip\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 200 OK\r\n\
+            Server: test\r\n\
+            Content-Length: 5\r\n\
+            \r\n\
+            Hello\
+            "
+    };
+
+    let url = format!("http://{}/text", server.addr());
+    let mut res = reqwest::get(&url).unwrap();
+    assert_eq!(res.url().as_str(), &url);
+    assert_eq!(res.status(), reqwest::StatusCode::Ok);
+    assert_eq!(res.headers().get(),
+               Some(&reqwest::header::Server::new("test".to_string())));
+    assert_eq!(res.headers().get(),
+               Some(&reqwest::header::ContentLength(5)));
+
+    let body = res.text().unwrap();
+    assert_eq!(b"Hello", body.as_bytes());
+}
+
 #[test]
 fn test_response_copy_to() {
     let server = server! {