On Tue, Apr 21, 2009 at 5:47 AM, <therealsac...@gmail.com> wrote: > Looks good to me. > > And thanks for fixing this. > > > http://codereview.appspot.com/45044/diff/1/13 > File > java/common/src/main/java/org/apache/shindig/protocol/DataServiceServlet.java > (right): > > http://codereview.appspot.com/45044/diff/1/13#newcode141 > Line 141: if (responseItem.getErrorCode() < 400) { > Should we be more strict here >= 200 && < 300? The spec allows -ev > values to be used as error code.
Good catch - I'm going to go with >= 200 and < 400, as 3xx is not an error - though I've no idea what a "redirect" means here. Also (for DataServiceServlet only), did a quick-and-dirty mapping of negative codes to HTTP error codes. > http://codereview.appspot.com/45044/diff/1/21 > File > java/common/src/main/java/org/apache/shindig/protocol/JsonRpcServlet.java > (right): > > http://codereview.appspot.com/45044/diff/1/21#newcode204 > Line 204: if (responseItem.getErrorCode() >= 400) { > ditto. Done. > http://codereview.appspot.com/45044/diff/1/21#newcode266 > Line 266: sendError(response, new > ResponseItem(HttpServletResponse.SC_BAD_REQUEST, > As per the spec. > "-32700 (Parse error) > Invalid JSON. An error occurred on the server while parsing the JSON > text." > > Should we bother to send this error code where we are sure it is json > parse error? Done. > http://codereview.appspot.com/45044 >
