Re: Allow containers to specify their own http error codes if needed.
The whole ResponseError class isn't really necessary. ResponseError.parse() is unused. The String version of the error code is used in exactly one place, JsonRpcServlet.getJSONResponse(), and there just to pre-seed the error message. The simpler solution is just deleting the entire class and using integer response codes end-to-end. On Fri, Apr 17, 2009 at 8:29 PM, Sachin Shenoy therealsac...@gmail.com wrote: A client may want to let user know very clear error message why an operation failed. For example, LIMIT_EXCEEDED defined currently only tells that some limit was exceeded. If you take the case of photo upload, we are not sure if it was number of photos in an album, or number of total photos uploaded by the user, or even the size of the photo that was exceeded. If we use the text message attached with the error to convey this information, we run into i18n issues. It seems it is lot cleaner for apps to be able to use the impl. defined error codes, ALBUM_PHOTO_LIMIT_EXCEEDED = -32000; USER_PHOTO_LIMIT_EXCEEDED = -32001; PHOTO_SIZE_EXCEEDED = -32002; etc. Any thoughts? Thanks, Sachin On Sat, Apr 18, 2009 at 8:39 AM, Kevin Brown e...@google.com wrote: Why not just use actual HTTP error codes? I'll personally guarantee that they never change. On Fri, Apr 17, 2009 at 4:05 PM, Adam Winer awi...@gmail.com wrote: Why is it necessary to make it a class? If we need a few more error codes, we can add some extra enums. If there really is a need to support any HTTP error code and this has to be a class, there should be some assurance that the built-in HTTP status codes and jsonValues are not reused - this keeps parsing and comparisons easy. E.g., UNAUTHORIZED, unauthorized, and HttpServletResponse.SC_UNAUTHORIZED all are guaranteed to match up in all cases. On Mon, Apr 13, 2009 at 12:58 PM, therealsac...@gmail.com wrote: Reviewers: awiner, Description: Changed ResponseError from enum to a class. https://issues.apache.org/jira/browse/SHINDIG-1012 Please review this at http://codereview.appspot.com/40076 Affected files: java/common/src/main/java/org/apache/shindig/protocol/ResponseError.java java/common/src/test/java/org/apache/shindig/protocol/ResponseErrorTest.java
Re: Allow containers to specify their own http error codes if needed.
Why is it necessary to make it a class? If we need a few more error codes, we can add some extra enums. If there really is a need to support any HTTP error code and this has to be a class, there should be some assurance that the built-in HTTP status codes and jsonValues are not reused - this keeps parsing and comparisons easy. E.g., UNAUTHORIZED, unauthorized, and HttpServletResponse.SC_UNAUTHORIZED all are guaranteed to match up in all cases. On Mon, Apr 13, 2009 at 12:58 PM, therealsac...@gmail.com wrote: Reviewers: awiner, Description: Changed ResponseError from enum to a class. https://issues.apache.org/jira/browse/SHINDIG-1012 Please review this at http://codereview.appspot.com/40076 Affected files: java/common/src/main/java/org/apache/shindig/protocol/ResponseError.java java/common/src/test/java/org/apache/shindig/protocol/ResponseErrorTest.java
Re: Allow containers to specify their own http error codes if needed.
Why not just use actual HTTP error codes? I'll personally guarantee that they never change. On Fri, Apr 17, 2009 at 4:05 PM, Adam Winer awi...@gmail.com wrote: Why is it necessary to make it a class? If we need a few more error codes, we can add some extra enums. If there really is a need to support any HTTP error code and this has to be a class, there should be some assurance that the built-in HTTP status codes and jsonValues are not reused - this keeps parsing and comparisons easy. E.g., UNAUTHORIZED, unauthorized, and HttpServletResponse.SC_UNAUTHORIZED all are guaranteed to match up in all cases. On Mon, Apr 13, 2009 at 12:58 PM, therealsac...@gmail.com wrote: Reviewers: awiner, Description: Changed ResponseError from enum to a class. https://issues.apache.org/jira/browse/SHINDIG-1012 Please review this at http://codereview.appspot.com/40076 Affected files: java/common/src/main/java/org/apache/shindig/protocol/ResponseError.java java/common/src/test/java/org/apache/shindig/protocol/ResponseErrorTest.java
Re: Allow containers to specify their own http error codes if needed.
As per the Json-RPC spec http://groups.google.com/group/json-rpc/web/json-rpc-1-2-proposal?pli=1#error-object and http://www.opensocial.org/Technical-Resources/opensocial-spec-v081/rpc-protocol (Section Standard Error) a server can use error codes from -32099..-32000 for implementation-defined server-errors. I wanted shindig to allow them to be used by a server. One way is to make this a class (as I have done here). Another way I can think of is to define 100 (or at least 20) enums IMPL_DEFINED_ERROR1 ... IMPL_DEFINED_ERROR100, and then let the implementation map them to more readable value in their code. ResponseError converToResponseError(ServerError serverError); convertToResponseError(MY_SERVER_SPECIFIC_ERROR); I don't have strict preference. I could go with the predefined enum way too? Should I do predefined enum way, or is there something else we can do? Thanks Sachin On Sat, Apr 18, 2009 at 4:35 AM, Adam Winer awi...@gmail.com wrote: Why is it necessary to make it a class? If we need a few more error codes, we can add some extra enums. If there really is a need to support any HTTP error code and this has to be a class, there should be some assurance that the built-in HTTP status codes and jsonValues are not reused - this keeps parsing and comparisons easy. E.g., UNAUTHORIZED, unauthorized, and HttpServletResponse.SC_UNAUTHORIZED all are guaranteed to match up in all cases. On Mon, Apr 13, 2009 at 12:58 PM, therealsac...@gmail.com wrote: Reviewers: awiner, Description: Changed ResponseError from enum to a class. https://issues.apache.org/jira/browse/SHINDIG-1012 Please review this at http://codereview.appspot.com/40076 Affected files: java/common/src/main/java/org/apache/shindig/protocol/ResponseError.java java/common/src/test/java/org/apache/shindig/protocol/ResponseErrorTest.java
Re: Allow containers to specify their own http error codes if needed.
A client may want to let user know very clear error message why an operation failed. For example, LIMIT_EXCEEDED defined currently only tells that some limit was exceeded. If you take the case of photo upload, we are not sure if it was number of photos in an album, or number of total photos uploaded by the user, or even the size of the photo that was exceeded. If we use the text message attached with the error to convey this information, we run into i18n issues. It seems it is lot cleaner for apps to be able to use the impl. defined error codes, ALBUM_PHOTO_LIMIT_EXCEEDED = -32000; USER_PHOTO_LIMIT_EXCEEDED = -32001; PHOTO_SIZE_EXCEEDED = -32002; etc. Any thoughts? Thanks, Sachin On Sat, Apr 18, 2009 at 8:39 AM, Kevin Brown e...@google.com wrote: Why not just use actual HTTP error codes? I'll personally guarantee that they never change. On Fri, Apr 17, 2009 at 4:05 PM, Adam Winer awi...@gmail.com wrote: Why is it necessary to make it a class? If we need a few more error codes, we can add some extra enums. If there really is a need to support any HTTP error code and this has to be a class, there should be some assurance that the built-in HTTP status codes and jsonValues are not reused - this keeps parsing and comparisons easy. E.g., UNAUTHORIZED, unauthorized, and HttpServletResponse.SC_UNAUTHORIZED all are guaranteed to match up in all cases. On Mon, Apr 13, 2009 at 12:58 PM, therealsac...@gmail.com wrote: Reviewers: awiner, Description: Changed ResponseError from enum to a class. https://issues.apache.org/jira/browse/SHINDIG-1012 Please review this at http://codereview.appspot.com/40076 Affected files: java/common/src/main/java/org/apache/shindig/protocol/ResponseError.java java/common/src/test/java/org/apache/shindig/protocol/ResponseErrorTest.java
