[
https://issues.apache.org/jira/browse/SHINDIG-1770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13272298#comment-13272298
]
[email protected] commented on SHINDIG-1770:
--------------------------------------------------------
bq. On 2012-05-08 19:39:58, Dan Dumont wrote:
bq. > 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?
bq. >
bq. > Did we before your change?
bq.
bq. Stanton Sievers wrote:
bq. Did some extra testing. I introduced a compile error in
embedded-experiences feature and hit this url in my browser:
http://localhost:8080/gadgets/js/embedded-experiences.js?c=1&container=default
bq.
bq. I set a breakpoint in JsServlet.doGet() and regardless of whether my
patch was applied or not, we were doing the processing every time. The only
difference before my patch was that on subsequent requests the browser would
actually get a 304 Not Modified back instead of the 404. Is that because of
the ETag filter or the cache control headers?
bq.
bq. Response headers before my patch:
bq. Cache-Control: public,max-age=3600
bq. Content-Length: 0
bq. Content-Type: text/html;charset=utf-8
bq. Date: Wed, 09 May 2012 13:52:15 GMT
bq. Etag: "d41d8cd98f00b204e9800998ecf8427e"
bq. Expires: Wed, 09 May 2012 14:52:15 GMT
bq. Server: Apache-Coyote/1.1
bq.
bq. Response headers after my patch:
bq. Content-Length: 1636
bq. Content-Type: text/html;charset=utf-8
bq. Date: Wed, 09 May 2012 13:59:59 GMT
bq. Server: Apache-Coyote/1.1
bq.
bq. Henry Saputra wrote:
bq. Looks like the ClosureJsCompiler cache the compiled result based on
key of hash value of the non-compiled JS content.
bq.
bq. Stanton Sievers wrote:
bq. Good call Henry. I never noticed the
ClosureJsCompiler.cacheAndReturnErrorResult() method before, which will be
called when the compiler has errors during processing. So on subsequent calls,
we will go through the JS processor again, however, the compiler processor will
have cached the errors and will return those instead of running the compiler
again.
bq.
bq. And it does appear that it's the ETagFilter that is causing the 304 to
come back in the case where my patch is not applied.
bq.
bq. Ryan Baxter wrote:
bq. So we are in fact caching the error response and not recompiling bad
JS every time?
bq.
bq. Stanton Sievers wrote:
bq. Yes, but to be clear, it's being cached in the ClosureJsCompiler, not
at the JsServlet level.
So the difference being that the request will still go through all of the
processors every time where as if it were cached at the JsServlet level it
wouldn't correct?
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5070/#review7695
-----------------------------------------------------------
On 2012-05-08 19:17:32, Stanton Sievers wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/5070/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-05-08 19:17:32)
bq.
bq.
bq. Review request for shindig.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. 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.
bq.
bq. Jira text:
bq. 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.
bq.
bq.
bq. This addresses bug SHINDIG-1770.
bq. https://issues.apache.org/jira/browse/SHINDIG-1770
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/DefaultJsServingPipeline.java
1335416
bq.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsServletTest.java
1335416
bq.
bq. Diff: https://reviews.apache.org/r/5070/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Added a new test in JsServlet to ensure the error makes it through.
bq.
bq. 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.
bq.
bq.
bq. Thanks,
bq.
bq. Stanton
bq.
bq.
> Improve JsServlet to respond with an error and error message if there was a
> problem
> -----------------------------------------------------------------------------------
>
> Key: SHINDIG-1770
> URL: https://issues.apache.org/jira/browse/SHINDIG-1770
> Project: Shindig
> Issue Type: Improvement
> Components: Java
> Affects Versions: 2.5.0
> Reporter: Stanton Sievers
> Assignee: Stanton Sievers
> Fix For: 2.5.0
>
>
> 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 message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
