Minor improvement suggestions to the guide regarding cookies, json, errors and testing.

[arctic-alpaca]

Hi, while working though the guide, I found some minor things that could, in my opinion be improved. I checked the master branch to see whether the mentioned sections might have changed but as far as I can tell, my suggestions should still be applicable.

• [x] https://rocket.rs/v0.4/guide/requests/#cookies This isn't related to Rocket specifically. Cookie paths weren't clear to me so I tried to set a cookie in "/cookies/set" and print it in "/cookies/get". This doesn't work because the path set by Cookies::new() in this case is "/cookies/set", so the cookie isn't accessible from "/cookies/get". Adding a hint or warning that the path you try to read the cookie from must start with the same path you set it from might be helpful for newcomers.

• [x] https://rocket.rs/v0.4/guide/responses/#wrapping

  #[get("/")] 
  fn json() -> content::Json<&'static str> { 
      content::Json("{ 'hi': 'world' }")  
  }

  Results in "SyntaxError: JSON.parse: expected property name or '}' at line 1 column 3 of the JSON data" in Firefox. JSON seems to require double quotes according to https://www.json.org/json-en.html. replacing "{ 'hi': 'world' }" with r#"{ "hi": "world" }"# or "{ \"hi\": \"world\" }" solved the issue for me.

• [x] https://rocket.rs/v0.4/guide/responses/#errors "Responders may fail; they need not always generate a response." Is this correct? English isn't my first language. To me it seems like there something missing. "Responders may fail; they don't always need to generate a response." This seems correct to me, but I'm not sure if some meaning is lost.

• [ ] https://rocket.rs/v0.4/guide/testing/#testing let client = Client::new(rocket).expect("valid rocket instance"); Shouldn't an error description be used with expect? Something like let client = Client::new(rocket).expect("Could not create valid rocket instance");?

[jebrosen]

This isn't related to Rocket specifically. Cookie paths weren't clear to me so I tried to set a cookie in "/cookies/set" and print it in "/cookies/get". This doesn't work because the path set by Cookies::new() in this case is "/cookies/set", so the cookie isn't accessible from "/cookies/get". Adding a hint or warning that the path you try to read the cookie from must start with the same path you set it from might be helpful for newcomers.

That should not be the case - if you don't set a specific Path on the cookie, then any path should be able to read the cookie (as long as the other restrictions on the cookie are met). Do you have a minimal example of this we could test?

---

Results in "SyntaxError: JSON.parse: expected property name or '}' at line 1 column 3 of the JSON data" in Firefox.

Yes, that is a mistake in the guide. We would be happy to accept a PR fixing it, or I should be able to do it this weekend.

---

"Responders may fail; they need not always generate a response." Is this correct? English isn't my first language. To me it seems like there something missing. "Responders may fail; they don't always need to generate a response." This seems correct to me, but I'm not sure if some meaning is lost.

These two sentences actually have the same meaning in English! What do you think about this rewording, though:

"Responders may fail instead of generating a response, by returning Err with a status code. When this happens, Rocket forwards the request to the error catcher for the given status code."

---

Shouldn't an error description be used with expect?

I have always thought this was a style choice (if anyone knows of any specific guidelines, I would love to see them!) But the current messages actually would end up looking awkward if they were printed, something like thread 'main' panicked at 'valid rocket instance: LaunchError { ... }

So, perhaps we should change them. I think this one is lower priority to fix than the other issues listed, because at least for expect the filename and line number are still part of the message/stack trace.

[arctic-alpaca]

I created a PR here #1359.

Regarding the cookies, I could reproduce it with this code:

#![feature(proc_macro_hygiene, decl_macro)]
#[macro_use]
extern crate rocket;
use rocket::http::{Cookie, Cookies};

fn main() {
    rocket::ignite()
        .mount(
            "/cookies",
            routes![cookies_get, cookies_set, cookies_set_get],
        )
        .launch();
}

#[get("/get")]
pub fn cookies_get(cookies: Cookies) -> Option<String> {
    cookies
        .get("message")
        .map(|c| format!("Message: {}", c.value()))
}

#[get("/set/<message>")]
pub fn cookies_set(mut cookies: Cookies, message: String) -> String {
    cookies.add(Cookie::new("message", message));
    format!("String added")
}

#[get("/set")]
pub fn cookies_set_get(cookies: Cookies) -> Option<String> {
    cookies
        .get("message")
        .map(|c| format!("Message: {}", c.value()))
}

cookies_get doesn't "see" any cookies set by cookies_set. cookies_set_get does print the cookie value as expected. The Path visible in the developer tools in Firefox is '/cookies/set'. This is the output from the debug build when accessing localhost:8000/cookies/get:

GET /cookies/get text/html:
    => Matched: GET /cookies/get (cookies_get)
    => Warning: Response was `None`.
    => Outcome: Failure
    => Warning: Responding with 404 Not Found catcher.
    => Response succeeded.

These two sentences actually have the same meaning in English!

Good to know, thank you.

What do you think about this rewording, though: "Responders may fail instead of generating a response, by returning Err with a status code. When this happens, Rocket forwards the request to the error catcher for the given status code."

I like it, it reads more concise to me.

[jebrosen]

Ah yeah, you're right about the cookie path. I had simply forgotten this rule, and I don't see any mention of it on MDN either. It is explained in https://tools.ietf.org/html/rfc6265#section-5.1.4. By default, the cookie set by /cookies/set/x is only visible at /cookies/set/*, and by /cookies/set/y/ is only visible at /cookies/set/y/* (!) We can probably say something about this in the guide, and the way to override it: Cookie::build("name", "value").path("/").

[arctic-alpaca]

Here on MDN under Define where cookies are sent - Path attribute, there is a very short paragraph regarding the path of cookies. I think having it in the guide and especially also having the way to override this demonstrated would be very helpful.

I wasn't away of the difference between /cookies/set/y/ and /cookies/set/x, thank you for pointing that out.