1363: API: /api/json/reviewrequests/<review_request_id>/reviews/ -- Non-existent reviews return 404 not found
- NotABug
- Review Board
| cybr****@gmai***** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| Oct. 22, 2009 |
What version are you running? ReviewBoard-1.1alpha1.dev_r2026 What's the URL of the page containing the problem? http://Reviewboard/api/json/reviewrequests/<review_request_id>/ What steps will reproduce the problem? 1. Create a new review (i.e. Review ID: 123) 2. Delete the review completely. 3. Request the review through the API What is the expected output? What do you see instead? I get a 404 Not Found error. If the review looks like a valid review ID, then instead of 404 I should get a JSON formatted error indicating that the review id was not found. The 404 exception doesn't tell me if the review was not found. It simply tells me there was some server error. Maybe RB is mis-configured. Maybe I'm querying the wrong server. Maybe you will disagree that this is a defect, but I thought I should mention it anyway. The workaround I'm using now is catching 404 exceptions and *assuming* that this means the requested review doesn't exist. What operating system are you using? What browser? N/A Please provide any additional information below.
This is how a proper REST API should behave. In fact, we should be doing this more than we do. 404 means "not found" and in this case, that's exactly the correct response. The idea of REST is that you make use of HTTP's built-in methods and responses as much as possible, since they're universally supported. If you want to get something without any side-effects on the server, you use a GET. If you want to post something, you use a POST. And so on. Likewise, built-in HTTP statuses should be used, since clients all understand them. Now, we don't do nearly as good a job as we should do. We don't use PUT, DELETE, etc., and we don't always use the response codes 100% correctly, but in this case, it is correct. Maybe down the road we could supply a JSON payload along with the 404 (as most error codes can also return page content). This is a nice to have, but won't happen for 1.1.
-
+ NotABug -
+ Component-API -
+ chipx86