You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2010/01/20 22:10:20 UTC

Re: [SHINDIG-1266] - Use correct HTTP status codes in GadgetRenderingServlet (issue186233)

http://codereview.appspot.com/186233/diff/1/7
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java
(right):

http://codereview.appspot.com/186233/diff/1/7#newcode109
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/ProxyRenderer.java:109:
throw new RenderingException("Unable to reach remote host. HTTP status "
+ code, code);
I don't think we should pass the code through verbatim. It's not an
internal server error from the perspective of Shindig Gadget Server to
find that a target server of a proxy render emits a 500. I vote
404/SC_NOT_FOUND here.

http://codereview.appspot.com/186233/diff/1/8
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java
(right):

http://codereview.appspot.com/186233/diff/1/8#newcode28
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java:28:
public int httpStatusCode;
should be final

http://codereview.appspot.com/186233/diff/1/8#newcode36
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java:36:
this.httpStatusCode = httpStatusCode;
may as well add a proper getter for this.

http://codereview.appspot.com/186233/diff/1/8#newcode40
java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingException.java:40:
super(message, t);
no default specified here (and rightly so, IMO httpStatusCode needs to
be added to this ctor)

http://codereview.appspot.com/186233/show