search

PokemonGoers / Catch-em-all

Now that we have tons of data about Pokemon (what they are, where they are, what’s their relationship, what they can transform into, which attacks they can perform, aso) we want to integrate it all into a comprehensive website. This website should contain sections about each Pokemon and its details. Additionally, the website should register the user’s location and tell the user how close is that the predicted pokemon to him/her. Additionally you will be incorporating the apps that were created by project B,C and D into the website. Your group will need to create automated builds and testing for this apps and use continuous integration to pull in new changes in the code repositories. Apps from projects B-D should be packaged and made available on NPM. Ideally when you completed these tasks the webapp component would integrate the apps by “requiring’ them. Here is a possible user story: when a user opens the website or the app the current location of the user will be shown. Additionally, the website/app will show automatically where the pokemons that are currently active are and where the pokemons that we predict to active in the nearest future (i.e. within half a day) will be located (all of this will be available from the app developed in project D). Hopefully, the website will be somewhat crowded by that data. Then, there needs to be a menu bar or something available (e.g. above the map or on the right side to it) that will list currently active or predicted pokemons. Clicking on one of them will make other pokemons on the map disappear, except of this clicked one. Separate web pages would allow the search and presentation of individual Pokemons and the information we gathered about them, including third party data (project A) and twitter analysis (project C)
9 stars 7 forks source link

Too many redirects #139

Open sacdallago opened 7 years ago

sacdallago commented 7 years ago

Hey guys,

still working on the final deployment. Apart from it being hard to judge if stuff is not working because it's not working or it's not working because something is wrong on the deployment, I have one issue which I'm pretty sure is a mixture of how the app needs to be deployed and how you have implemented things.

If you try to go to the pokedex on htpps://predictemall.online and you open up your debug console, you'll notice the: core.edb342e….bundle.js:2234 GET https://predictemall.online/api/pokemon/name/bul net::ERR_TOO_MANY_REDIRECTS error. This is bad. If you actually try to go to https://predictemall.online/api/pokemon/name/bul, you will have three redirects (in my opinion, didn't CURL fact-check it):

  1. to httpS://predictemall.online/api/pokemon/name/bul
  2. To http://api.predictemall.online/api/pokemon/name/bul
  3. to httpS://api.predictemall.online/api/pokemon/name/bul

Apart from the fact that there is no data coming back from the API (but I have complained about that as well, don't worry https://github.com/PokemonGoers/PokeData/issues/191 ), this is something that needs to get fixed ASAP. Maybe assuming the protocol to be https instead of http will lower the number of redirects and chrome won't complain anymore. This should be an ENV var, if you are wondering: cos otherwise it's gonna fuck up development (no one develops in https)

johartl commented 7 years ago

@sacdallago We already have environment variables for all API endpoints. See the defaults in the Dockerfile

// Dockerfile
ENV API_ENDPOINT=http://pokedata.c4e3f8c7.svc.dockerapp.io:65014
ENV WEBSOCKET_ENDPOINT=http://pokedata.c4e3f8c7.svc.dockerapp.io:65024

My first guess would be to set them as follows

ENV API_ENDPOINT=https://api.predictemall.online
ENV WEBSOCKET_ENDPOINT=https://api.predictemall.online:[websocket port?]

Edit: Btw, I don't see the errors in my console. Have you solved the issue already?

sacdallago commented 7 years ago

What you are saying is very different from what I am complaining about :D I'm changing the env vars already through my docker-compose file ( read https://github.com/PokemonGoers/PokeData/issues/191 ). My suggestion was to have another env var, which would avoid one redirect: the redirect from http to https as far as the protocol for the API goes. Your calls do the following: I call http://api.predictemall.online/api/somethingSomething, but nginx doesn't like when it get's the http request and answers with a 301 permanently moved, saying that the resource is now at https://api.predictemall.online/api/somethingSomething. Having that one redirect less, is already a great improvement!

Anyway, as I mentioned in a very eloquent and high english here, all problems seem to have been solved?! Magically!? Like.. No idea!!?

johartl commented 7 years ago

@sacdallago In your docker-compose file from https://github.com/PokemonGoers/PokeData/issues/191#issue-184668019 you have

  environment:
    - API_ENDPOINT=api.predictemall.online
    - WEBSOCKET_ENDPOINT=api.predictemall.online:65024

My suggestion was to to add https:// to both API_ENDPOINT and WEBSOCKET_ENDPOINT.

But I guess since everything is working now (?) it's fine :D

sacdallago commented 7 years ago

wouldn't adding the protocol overwrite something somewhere? I imagine you use those variables either to:

  1. http.get(API_ENDPOINT)
  2. get("http://" + API_ENDPOINT)

So I fear that adding the protocol will either break the thing or be completely pointless(in case 1). It works, yes. But this is an issue anyway :)

johartl commented 7 years ago

We are using the variable consistently in the first way and so far nothing has been broken yet ;) And can you explain how the first way of using it is completely pointless?

MajorBreakfast commented 7 years ago

Just a side note: There are protocol relative URLs, e.g. //example.com (double slash at the beginning) https://www.paulirish.com/2010/the-protocol-relative-url/

sacdallago commented 7 years ago

ok, I'll try out the suggestions on a test instance when I'm back from SFO in a week :)

But since this is working, I'll close for now, and sorry for talking to the dead :D

sacdallago commented 7 years ago

Yay, it was too good to be true!

It now happens consistently :) And I have no idea of how to fix this, tried:

API_ENDPOINT=https://api.predictemall.online
WEBSOCKET_ENDPOINT=https://stream.predictemall.online
API_ENDPOINT=https://api.predictemall.online:443
WEBSOCKET_ENDPOINT=https://stream.predictemall.online:443
API_ENDPOINT=http://api.predictemall.online
WEBSOCKET_ENDPOINT=http://stream.predictemall.online
API_ENDPOINT=http://api.predictemall.online:80
WEBSOCKET_ENDPOINT=http://stream.predictemall.online:80
API_ENDPOINT=api.predictemall.online
WEBSOCKET_ENDPOINT=stream.predictemall.online

The last one didn't work at all :) the other ones give all sorts of problems, but mainly the TOO_MANY_REDIRECTS.

Don't worry about WEBSOCKET_ENDPOINT, for me what is important is that API_ENDPOINT works correctly.

johartl commented 7 years ago

@sacdallago Where do you see the TOO_MANY_REDIRECTS error? In the browser console or in the node server log of the CatchEmAll container? Is there some way for me to reproduce it? For example using curl?

In theory, it should work like this: (This is just my understanding of the proxy chain, please correct me where I'm wrong)

  1. The browser sends a request to window.location.origin + '/api/pokemon/name/bul'which should evaluate to https://predictemall.online/api/pokemon/name/bul.
  2. The nginx proxy server should proxy this request to http://catchemall.svc.dockerapp.io/api/pokemon/name/bul (domain made up) where the catchemall docker container runs at.
  3. The catchemall node server proxies the request to API_ENDPOINT=https://api.predictemall.online
  4. The nginx proxy server proxies the request to http://pokedata.svc.dockerapp.io/api/pokemon/name/bul where the pokedata container runs at (again, domain made up).
  5. The pokedata container receives the request, sends out the response and it goes all the way back.

To me this is a lot of proxying but I don't see any redirects here (as in 301 Moved Permanently). To the browser this should all be transparent, right? Also, it would help my understanding to know where the pokedata and catchemall containers are running. Is this the same host as the nginx server? Maybe you could also provide the nginx configuration. It's a bit difficult to debug from my side without being able to reproduce the error.

johartl commented 7 years ago

@sacdallago Can you please redeploy as soon as https://hub.docker.com/r/pokemongoers/catch-em-all/builds/bmhxzjysukghvxqaomvgxvn/ is done building to get the changes from #158.

sacdallago commented 7 years ago

@johartl sorry for the delay: everything redeploys automatically (so as soon as stuff builds on hub, give it like 5 min?).

The errors happen in the chrome console @ predictemall.online . I will be a bit more active starting tomorrow (am currently at a Google conference)

johartl commented 7 years ago

This afternoon when I checked there was still an old container active (from Oct 27th). So either something is wrong with the automated deployment or with my cache :/ Anyway, I don't see any redirect errors in the console right now for predictemall.online (and I think I've never seen any in the past).

sacdallago commented 7 years ago

Yup, not happening on my machine anymore either. I'm not happy that this thing is not happening consistently. Need to stress test it once I'm back from SFO, btw check your email

johartl commented 7 years ago

this thing is not happening consistently

I suspect this had something to do with the service worker cache not being flushed. I tackled this issue in #162.

Also, I found that some request were send to http://pokedata.c4e3f8c7.svc.dockerapp.io:65014 directly. I fixed the issue in #164, however, I don't think that this caused any redirects.

Thanks for that mail! I'm going to investigate further once #162 #163 and #164 are merged and deployed. Especially the broken service worker makes it really frustrating to debug :/