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 >>> > >>> > >>> > >>> >> >> >
