----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5070/#review7695 -----------------------------------------------------------
Quick question before I dig in. Do we cache the compile error response so that we don't get hammered trying to compile it over and over again? Did we before your change? - Dan On 2012-05-08 19:17:32, Stanton Sievers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5070/ > ----------------------------------------------------------- > > (Updated 2012-05-08 19:17:32) > > > Review request for shindig. > > > Summary > ------- > > I had initially made the change in JsServlet itself to check for errors > before calling emitJsResponse, however, DefaultJsServingPipeline.process() > seemed like a better place. JsServingPipeline.process' javadoc indicates the > method should throw an exception if any of the processing steps resulted in > an error, so it seems reasonable to check for errors and throw before > returning. > > Jira text: > Currently if JsServlet encounters an error processing a response, such as a > closure compile error, it will simply respond with the error status code but > won't respond with an error message. To improve this behavior, I'd like to > have the servlet send a proper error response with the error status code and > the error string. > > > This addresses bug SHINDIG-1770. > https://issues.apache.org/jira/browse/SHINDIG-1770 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsServingPipeline.java > 1335416 > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java > 1335416 > > Diff: https://reviews.apache.org/r/5070/diff > > > Testing > ------- > > Added a new test in JsServlet to ensure the error makes it through. > > Manually introduced JavaScript errors that would cause Closure to complain > and verified that the error text and status code made it through when hitting > the JsServlet in the browser. > > > Thanks, > > Stanton > >
