> On 2011-12-06 23:03:02, Stanton Sievers wrote: > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js, > > line 934 > > <https://reviews.apache.org/r/3037/diff/2/?file=62529#file62529line934> > > > > I'd rather be checking for a status of any 2xx code. I'd rather treat > > redirects and other 3xx codes as failures if they are going to fail at > > render time anyway. > > > > Also, what is the difference between checking response.error and > > response.status. Shouldn't they indicate the same thing? > > Ryan Baxter wrote: > I ignored the 3xx codes because I figured the browser would be able to > handle them and redirect the iframe accordingly. > > I looked into the response.error case. It looks like that will happen if > something goes wrong in the containers proxy. The response.status is the > result of actually making the http request. > > Stanton Sievers wrote: > Wouldn't we want to follow any 3xx redirects to ensure that the final > content is available? > > Another idea I thought of that might be worth evaluating... what if you > proxied the iframe urls through Shindig's proxy servlet and returned an error > page from there? Maybe add a query param flag that the proxy servlet can > read to know whether to return the original response or an error page. That > could at least solve the issue of having several http requests. > > The response.error and response.status stuff makes sense. > > Dan Dumont wrote: > I don't think that would work. I don't think the proxy would be able to > rewrite all of the content and fix images, css, links, etc... for the page to > function properly.
OK so did some digging in the BasicHttpFetcher. We set the HttpClient to follow redirects by default so we will follow all redirects when making the head request. With that being said I agree we should change the logic so we only render the URL if the status is >= 200 but < 300. - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3037/#review3681 ----------------------------------------------------------- On 2011-12-06 22:36:38, Ryan Baxter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3037/ > ----------------------------------------------------------- > > (Updated 2011-12-06 22:36:38) > > > Review request for shindig, Dan Dumont and Stanton Sievers. > > > Summary > ------- > > When you call commoncontainer.navigateUrl if the URL cannot be reached the > caller has no way of knowing if the URL was navigated successfully or not. To > solve this we make a head request to the URL we are navigating to and add a > callback to the API. > > It is important to note that we will not be caching the response of the head > request. While this could possibly give us better performance we have no way > of guaranteeing the server will still be up next time and everything may > fail. This is different from the gadget case where we have the gadget XML > cached on the server. > > > This addresses bug SHINDIG-1669. > https://issues.apache.org/jira/browse/SHINDIG-1669 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/open-views/viewenhancements-container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/embeddedexperiences/embedded_experiences_container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container/container.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/container.util/util.js > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/embeddedexperiences/PhotoList.xml > 1211103 > > http://svn.apache.org/repos/asf/shindig/trunk/features/src/test/javascript/features/container.url/container_url_test.js > 1211103 > > Diff: https://reviews.apache.org/r/3037/diff > > > Testing > ------- > > Tested in container as well as updating unit tests. > > > Thanks, > > Ryan > >
