mozilla / embedly-proxy

Mozilla Public License 2.0
2 stars 7 forks source link

Modify server timeout #42

Closed pdehaan closed 8 years ago

pdehaan commented 8 years ago

I filed https://github.com/mozilla/activity-streams/issues/251 earlier but it may be easier to handle request timeouts from the proxy directly.

I was stuck waiting on a embedly proxy response for ~30 seconds (which ultimately came back as a 502). Not sure if we want to tighten that to fail after 5-10s if we can't get a response, or ...

jaredlockhart commented 8 years ago

I'm not sure we should do anything about this. There's many options. We could, as you suggest lower the request time so the client fails faster. We could fail very fast for any uncached results and return a 202 to the client and then the client could poll again for results that have been precomputed by the server, but this requires additional logic on the server and client side. What do other people think? @k88hudson @oyiptong?

oyiptong commented 8 years ago

What are the embedly latencies? If embedly doesn't have a URL stored locally, it will go out on the network and obtain the summaries.

  1. One way to fix this would be to time out the client's request and/or return partial results found in the cache while continuing to wait for embedly.
  2. Another way would be to only respond to clients with cached results, moving the embedly requests to a task/asynchronous job every time we encounter a URL that's was a cache miss.

I'm of two minds about this. We don't want to spend too much engineering effort on this I want to say, but if this is a frequent occurrence, we might be forced to, to deliver a decent enough user experience.

Option #2 sounds like the easiest way to to get this rolling for now. In the mean time, @jaredkerim, can we get a log of embedly latencies?

pdehaan commented 8 years ago

Another case where I was waiting 15 and 30 seconds for embedly proxy responses (one of which resulted in a 502). But this seems to block page rendering, which means I was staring at a mostly blank page for 30s.

new_tab

502-ing URL in question was:

https://embedly-proxy.dev.mozaws.net/extract?urls=http%3A%2F%2Fimgur.com%2FaHtkEzD&urls=https%3A%2F%2Fmadeupandprobablydoesnotexist.com%2Ftaskbar%2F&urls=https%3A%2F%2Fwww.reddit.com%2Fr%2Faskscience%2Fcomments%2F4anry6%2Fdoes_light_lose_energy_when_reflected%2F&urls=http%3A%2F%2Flotsol.com%2F2016%2F03%2Fone-of-worlds-oldest-childrens-books-found-in-north-staffordshire%2F&urls=http%3A%2F%2Fwww.theguardian.com%2Fmusic%2F2016%2Fmar%2F16%2Fkeith-emerson-death-suicide-emerson-lake-and-palmer&urls=http%3A%2F%2Fphys.org%2Fnews%2F2016-03-nasa-space-unmanned-orbiting-craft.html&urls=http%3A%2F%2Fi.imgur.com%2F6SnZNUo.jpg&urls=https%3A%2F%2Ftwitter.com%2FDavidBrentMovie%2Fstatus%2F710017247973130240&urls=https%3A%2F%2Fwww.reddit.com%2Fr%2FLifeProTips%2Fcomments%2F4aoft8%2Flpt_if_you_ever_get_mail_from_nielsen_the_tv%2F&urls=https%3A%2F%2Fwww.reddit.com%2F&urls=http%3A%2F%2Fwww.politico.com%2Fblogs%2F2016-gop-primary-live-updates-and-results%2F2016%2F03%2Ftrump-foreign-policy-adviser-220853&urls=https%3A%2F%2Fmedium.com%2Fbasic-income%2Fdeep-learning-is-going-to-teach-us-all-the-lesson-of-our-lives-jobs-are-for-machines-7c6442e37a49&urls=https%3A%2F%2Fwww.youtube.com%2Fwatch&urls=http%3A%2F%2Fi.imgur.com%2Fx5wf1Q2.jpg&urls=https%3A%2F%2Fwww.reddit.com%2F


This looks fishy in my DevTools console:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://embedly-proxy.dev.mozaws.net/extract?urls=http%3A%2F%2Fimgur.com%2FaHtkEzD&urls=https%3A%2F%2Fmadeupandprobablydoesnotexist.com%2Ftaskbar%2F&urls=https%3A%2F%2Fwww.reddit.com%2Fr%2Faskscience%2Fcomments%2F4anry6%2Fdoes_light_lose_energy_when_reflected%2F&urls=http%3A%2F%2Flotsol.com%2F2016%2F03%2Fone-of-worlds-oldest-childrens-books-found-in-north-staffordshire%2F&urls=http%3A%2F%2Fwww.theguardian.com%2Fmusic%2F2016%2Fmar%2F16%2Fkeith-emerson-death-suicide-emerson-lake-and-palmer&urls=http%3A%2F%2Fphys.org%2Fnews%2F2016-03-nasa-space-unmanned-orbiting-craft.html&urls=http%3A%2F%2Fi.imgur.com%2F6SnZNUo.jpg&urls=https%3A%2F%2Ftwitter.com%2FDavidBrentMovie%2Fstatus%2F710017247973130240&urls=https%3A%2F%2Fwww.reddit.com%2Fr%2FLifeProTips%2Fcomments%2F4aoft8%2Flpt_if_you_ever_get_mail_from_nielsen_the_tv%2F&urls=https%3A%2F%2Fwww.reddit.com%2F&urls=http%3A%2F%2Fwww.politico.com%2Fblogs%2F2016-gop-primary-live-updates-and-results%2F2016%2F03%2Ftrump-foreign-policy-adviser-220853&urls=https%3A%2F%2Fmedium.com%2Fbasic-income%2Fdeep-learning-is-going-to-teach-us-all-the-lesson-of-our-lives-jobs-are-for-machines-7c6442e37a49&urls=https%3A%2F%2Fwww.youtube.com%2Fwatch&urls=http%3A%2F%2Fi.imgur.com%2Fx5wf1Q2.jpg&urls=https%3A%2F%2Fwww.reddit.com%2F. (Reason: CORS header 'Access-Control-Allow-Origin' missing).


banners_and_alerts_and_new_tab_and_1__npm_run_once__sh_

jaredlockhart commented 8 years ago

This is handled by moving the embedly calls to the asynchronous workers #73