Switching service error response to JSON #92
Conversation
| @@ -95,10 +95,11 @@ impl Controller for FoxBox { | |||
| let services = self.services.lock().unwrap(); | |||
| match services.get(&id) { | |||
| None => { | |||
| let mut response = Response::with(format!("No Such Service: {}", id)); | |||
| let mut response = Response::with(format!( | |||
| "{{\"result\": \"error\", \"details\": \"No Such Service: {}\"}}", id)); | |||
There was a problem hiding this comment.
Move to the new json macros, and do something like : { error: "NoSuchService", id: id }
|
FWIW we are using this response format for the user related endpoints https://github.com/fxbox/users/blob/master/doc/API.md#response-format |
|
@ferjm, that error spec is a great goal aim for. I'd prefer to be switching to your format in all the places at once in a separate PR, coordinating with gmarty who is already consuming API errors. |
|
The fxbox/users repo uses numeric error codes. That's fine when you just mirror the http ones, but please don't add new custom ones. Instead, use strings like most DOM apis do, that's much more understandable. |
|
So are we going to land this? |
|
This needs to be rebased |
b83d7bb to
70b0fa7
Compare
|
Looks like a few json! switches were already landed in other patches. |
| @@ -147,7 +147,7 @@ describe! service_router { | |||
| &service_router).unwrap(); | |||
|
|
|||
| let result = response::extract_body_to_string(response); | |||
| assert_eq!(result, "No Such Service: unknown-id"); | |||
| assert_eq!(result, "{\"error\":\"NoSuchService\",\"id\":\"unknown-id\"}"); | |||
There was a problem hiding this comment.
There's a nicer string litteral syntax: r#{"error": "NoSuchService" ... }#
There was a problem hiding this comment.
It doesn't work in this context:
error: found invalid character; only `#` is allowed in raw string delimitation: {
assert_eq!(result, r#{"error":"NoSuchService","id":"unknown-id"}#);
^~
There was a problem hiding this comment.
right, I got it wrong. See https://github.com/fxbox/foxbox/blob/master/src/utils.rs#L72
There was a problem hiding this comment.
Done. Also fixed the indent.
|
Also, please squash your commits |
|
Ok, @fabricedesre ? |
Switching service error response to JSON
Seems wrong to deliver plaintext errors there.